Skip to content

Important features for driver - #42

Closed
Dexter9532 wants to merge 37 commits into
mainfrom
important_features_for_driver
Closed

Important features for driver#42
Dexter9532 wants to merge 37 commits into
mainfrom
important_features_for_driver

Conversation

@Dexter9532

Copy link
Copy Markdown
Contributor

Draft to add the remaining of the most important functions for the driver.

Also added tests to test function

Signed-off-by: Bobo Bäck Engström <bobo@id8-engineering.io>
Also added valid tests for the functio.

Signed-off-by: Bobo Bäck Engström <bobo@id8-engineering.io>
Also added valid test for the function

Signed-off-by: Bobo Bäck Engström <bobo@id8-engineering.io>
@Dexter9532
Dexter9532 force-pushed the important_features_for_driver branch from 6120d7d to 158beeb Compare October 28, 2025 13:25
@MiladMakdesi
MiladMakdesi force-pushed the important_features_for_driver branch from cd09ef3 to 8406e9e Compare October 28, 2025 15:33
Comment thread src/em511/em511.py Outdated
Comment thread src/em511/em511.py Outdated
Comment thread src/em511/em511.py
Comment thread src/em511/em511.py Outdated
Comment thread src/em511/test_em511.py Outdated
MiladMakdesi and others added 4 commits October 28, 2025 14:35
Also added valid tests for function

Signed-off-by: Milad Makdesi <milad@id8-engineering.io>
also added valid tests.

Signed-off-by: Milad Makdesi <milad@id8-engineering.io>
also added valid tests

Signed-off-by: Milad Makdesi <milad@id8-engineering.io>
Also added valid tests for the function

Signed-off-by: Bobo Bäck Engström <bobo@id8-engineering.io>
@MiladMakdesi
MiladMakdesi force-pushed the important_features_for_driver branch 4 times, most recently from 74750e1 to 654f98c Compare October 29, 2025 07:25
@Dexter9532
Dexter9532 force-pushed the important_features_for_driver branch from 0f0f592 to dd85392 Compare October 29, 2025 07:38
Also added valid tests

Signed-off-by: Bobo Bäck Engström <bobo@id8-engineering.io>
Also added valid tests

Signed-off-by: Bobo Bäck Engström <bobo@id8-engineering.io>
Also added valid tests

Signed-off-by: Bobo Bäck Engström <bobo@id8-engineering.io>
Also added valid tests

Signed-off-by: Bobo Bäck Engström <bobo@id8-engineering.io>
Also added valid tests

Signed-off-by: Bobo Bäck Engström <bobo@id8-engineering.io>
@MiladMakdesi
MiladMakdesi force-pushed the important_features_for_driver branch from 28a6120 to 5b8d57c Compare October 29, 2025 08:40
@Dexter9532
Dexter9532 force-pushed the important_features_for_driver branch from 073dfcb to 2e113f7 Compare October 29, 2025 08:55
Also added valid tests

Signed-off-by: Bobo Bäck Engström <bobo@id8-engineering.io>
@Dexter9532
Dexter9532 force-pushed the important_features_for_driver branch from 2e113f7 to 81e4707 Compare October 29, 2025 09:00
Also added valid tests

Signed-off-by: Bobo Bäck Engström <bobo@id8-engineering.io>
@Dexter9532
Dexter9532 force-pushed the important_features_for_driver branch from d7270a5 to 0ec908d Compare October 29, 2025 09:10
@Dexter9532
Dexter9532 requested a review from mirzak October 29, 2025 09:22
…tion raises

Did this because all setters claim that they will raise a ModbusException if failed to write to single regiser

Signed-off-by: Bobo Bäck Engström <bobo@id8-engineering.io>
@Dexter9532
Dexter9532 force-pushed the important_features_for_driver branch from 8a271d5 to 2b2edb2 Compare October 29, 2025 09:23
@MiladMakdesi
MiladMakdesi force-pushed the important_features_for_driver branch from 0693b67 to dbd2e5c Compare October 29, 2025 10:02
@Dexter9532
Dexter9532 force-pushed the important_features_for_driver branch from ad9ce94 to 69c7a8d Compare October 29, 2025 10:53
MiladMakdesi and others added 2 commits October 29, 2025 08:25
also added valid tests

Signed-off-by: Milad Makdesi <milad@id8-engineering.io>
Also added valid tests

Signed-off-by: Bobo Bäck Engström <bobo@id8-engineering.io>

@mirzak mirzak left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did a review of just the two first commits, will leave it at that for now.

Regarding your commit messages,

feat(hz): Add getter to read Hertz 

I would write something along the lines of:

feat: Add Hz property

Comment thread src/em511/em511.py Outdated

EM511_REGISTER_V = 0x0000
EM511_REGISTER_A = 0x0002
EM511_REGISTER_HZ = 0xF

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be 0x000F

Comment thread src/em511/test_em511.py
Comment on lines -23 to +24
assert value == 230
assert value == Decimal("230.0")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please put these changes in a separate commit as they are not related to adding Hz property.

Comment thread src/em511/test_em511.py
Comment on lines +96 to +106
"""Test 3: Should raise exception due to more registers in use than allowed."""
mock_result.registers = [0x1860, 0x0023, 0x5743]
client.read_input_registers.return_value = mock_result
with pytest.raises(ValueError, match="Unexpected register count:"):
_ = meter.Hz

"""Test 6: Should raise exception if input value exceeds maximum value, display shows 'EEE', 16-bit register."""
mock_result.registers = [0x7FFF]
client.read_input_registers.return_value = mock_result
with pytest.raises(ValueError, match="Input overflow EEE for 16-bit register: "):
_ = meter.Hz

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Testing this once is enough, and there is no need to repeat it for every property.

Comment thread src/em511/em511.py Outdated
Comment on lines +29 to +30
EM511_REGISTER_V = 0x0000
EM511_REGISTER_A = 0x0002
EM511_REGISTER_V = 0x0
EM511_REGISTER_A = 0x2

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The original was intended, i.e printing out the full address with zeroes. Also this change is not related to adding Hz property.

Dexter9532 and others added 4 commits October 29, 2025 13:56
…actory settings

Aslo add valid tests

Signed-off-by: Bobo Bäck Engström <bobo@id8-engineering.io>
…ax demand

Also add valid tests

Signed-off-by: Bobo Bäck Engström <bobo@id8-engineering.io>
also added valid tests

Signed-off-by: Milad Makdesi <milad@id8-engineering.io>
Also added valid tests

Signed-off-by: Bobo Bäck Engström <bobo@id8-engineering.io>
@MiladMakdesi
MiladMakdesi force-pushed the important_features_for_driver branch from d1b7dc9 to b95db6d Compare October 29, 2025 13:50
@Dexter9532
Dexter9532 force-pushed the important_features_for_driver branch 2 times, most recently from 4b9fac3 to 0f1a74e Compare October 29, 2025 13:57
Dexter9532 and others added 4 commits October 29, 2025 15:08
Signed-off-by: Bobo Bäck Engström <bobo@id8-engineering.io>
did this because thests where in worng order

Signed-off-by: Bobo Bäck Engström <bobo@id8-engineering.io>
also added valid tests

Signed-off-by: Milad Makdesi <milad@id8-engineering.io>
Did this to be more consistent

Signed-off-by: Bobo Bäck Engström <bobo@id8-engineering.io>
@Dexter9532
Dexter9532 requested a review from mirzak October 29, 2025 14:39
MiladMakdesi and others added 11 commits October 29, 2025 11:01
also added valid tests

Signed-off-by: Milad Makdesi <milad@id8-engineering.io>
also added valid tests

Signed-off-by: Milad Makdesi <milad@id8-engineering.io>
…l energy run hour counter

Also add valid tests

Signed-off-by: Bobo Bäck Engström <bobo@id8-engineering.io>
…e setting

also added valid tests

Signed-off-by: Milad Makdesi <milad@id8-engineering.io>
Also add valid tests

Signed-off-by: Bobo Bäck Engström <bobo@id8-engineering.io>
also added valid tests

Signed-off-by: Milad Makdesi <milad@id8-engineering.io>
Also add valid tests

Signed-off-by: Bobo Bäck Engström <bobo@id8-engineering.io>
also added valid tests

Signed-off-by: Milad Makdesi <milad@id8-engineering.io>
Also add valid tests

Signed-off-by: Bobo Bäck Engström <bobo@id8-engineering.io>
also added valid tests

Signed-off-by: Milad Makdesi <milad@id8-engineering.io>
Also add valid tests

Signed-off-by: Bobo Bäck Engström <bobo@id8-engineering.io>
Comment thread src/em511/em511.py
Comment on lines +207 to +209
regs = self._read_input_registers(self.EM511_REGISTER_A_DMD, self.INT32_REG_COUNT)
value = Decimal(self._unpack(regs, self.EM511_REGISTER_A_DMD)) / self.SCALE_1000
return round(value, 3)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a very common structure, could be worth simplifying to something like:

diff --git a/src/em511/em511.py b/src/em511/em511.py
index 4da7c7e..dbdaf99 100644
--- a/src/em511/em511.py
+++ b/src/em511/em511.py
@@ -121,6 +121,11 @@ class Em511:
         msg = f"Unexpected register count: {len(regs)}."
         raise ValueError(msg)
 
+    def _read_register(self, address: int, count: int, scale: int, decimals: int) -> Decimal:^M
+        regs = self._read_input_registers(address, count)^M
+        value = Decimal(self._unpack(regs, address)) / scale^M
+        return round(value, decimals)^M
+^M
     @property
     def V(self) -> Decimal:
         """Voltage (V).
@@ -132,9 +137,9 @@ class Em511:
             ValueError: If input is at max value or above.
             ModbusException: If failed to read input register.
         """
-        regs = self._read_input_registers(self.EM511_REGISTER_V, self.INT32_REG_COUNT)
-        value = Decimal(self._unpack(regs, self.EM511_REGISTER_V)) / self.SCALE_10
-        return round(value, 1)
+        return self._read_register(^M
+            address=self.EM511_REGISTER_V, count=self.INT32_REG_COUNT, scale=self.SCALE_10, decimals=1^M
+        )^M
 
     @property
     def A(self) -> Decimal:

Another alternative,

diff --git a/src/em511/em511.py b/src/em511/em511.py
index 4da7c7e..424a27c 100644
--- a/src/em511/em511.py
+++ b/src/em511/em511.py
@@ -1,12 +1,22 @@
 # ruff: noqa: N802
 """Driver class for EM511."""
 
+from dataclasses import dataclass^M
 from decimal import Decimal
+from typing import Dict^M
 
 from pymodbus.client import ModbusSerialClient
 from pymodbus.exceptions import ModbusException
 
 
+@dataclass(frozen=True)^M
+class RegisterSpec:^M
+    address: int^M
+    count: int^M
+    scale: int^M
+    decimals: int^M
+^M
+^M
 class Em511:
     """Driver for Carlo Gavazzi EM511 series energy meters.
 
@@ -26,13 +36,10 @@ class Em511:
     INPUT_MAX_VALUE_32 = 0x7FFFFFFF
     INPUT_MAX_VALUE_16 = 0x7FFF
 
-    EM511_REGISTER_V = 0x0000
-    EM511_REGISTER_A = 0x0002
-    EM511_REGISTER_PASSWORD = 0x1000
-
-    SCALE_10 = 10
-    SCALE_100 = 100
-    SCALE_1000 = 1000
+    _register_specs: Dict = {^M
+        "V": RegisterSpec(address=0x0000, count=INT32_REG_COUNT, scale=10, decimals=1),^M
+        "A": RegisterSpec(address=0x0002, count=INT32_REG_COUNT, scale=10, decimals=1),^M
+    }^M
 
     def __init__(self, device_address: int, client: ModbusSerialClient) -> None:
         """Initialize an Em511 driver instance with an existing Modbus client.
@@ -121,6 +128,12 @@ class Em511:
         msg = f"Unexpected register count: {len(regs)}."
         raise ValueError(msg)
 
+    def _read_register(self, register_name: str) -> Decimal:^M
+        spec = self._register_specs[register_name]^M
+        regs = self._read_input_registers(spec.address, spec.count)^M
+        value = Decimal(self._unpack(regs, spec.address)) / spec.scale^M
+        return round(value, spec.decimals)^M
+^M
     @property
     def V(self) -> Decimal:
         """Voltage (V).
@@ -131,10 +144,9 @@ class Em511:
         Raises:
             ValueError: If input is at max value or above.
             ModbusException: If failed to read input register.
+            KeyError: If register name is invalid.^M
         """
-        regs = self._read_input_registers(self.EM511_REGISTER_V, self.INT32_REG_COUNT)
-        value = Decimal(self._unpack(regs, self.EM511_REGISTER_V)) / self.SCALE_10
-        return round(value, 1)
+        return self._read_register("V")^M
 
     @property
     def A(self) -> Decimal:

@mirzak mirzak Oct 29, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one is interesting (fun exercise to figure out how it works :)):

diff --git a/src/em511/em511.py b/src/em511/em511.py
index 4da7c7e..9493832 100644
--- a/src/em511/em511.py
+++ b/src/em511/em511.py
@@ -1,12 +1,37 @@
 # ruff: noqa: N802
 """Driver class for EM511."""
 
+from dataclasses import dataclass^M
 from decimal import Decimal
+from typing import Dict^M
 
 from pymodbus.client import ModbusSerialClient
 from pymodbus.exceptions import ModbusException
 
 
+def register_properties(cls):^M
+    """Class decorator that auto-generates @property accessors for registers."""^M
+    for name in cls._register_specs:^M
+^M
+        def prop(self, _name=name):  # default arg binds current name^M
+            """Auto-generated register reader."""^M
+            return self._read_register(_name)^M
+^M
+        prop.__name__ = name^M
+        prop.__doc__ = f"{name} measurement."^M
+        setattr(cls, name, property(prop))^M
+    return cls^M
+^M
+^M
+@dataclass(frozen=True)^M
+class RegisterSpec:^M
+    address: int^M
+    count: int^M
+    scale: int^M
+    decimals: int^M
+^M
+^M
+@register_properties^M
 class Em511:
     """Driver for Carlo Gavazzi EM511 series energy meters.
 
@@ -26,13 +51,10 @@ class Em511:
     INPUT_MAX_VALUE_32 = 0x7FFFFFFF
     INPUT_MAX_VALUE_16 = 0x7FFF
 
-    EM511_REGISTER_V = 0x0000
-    EM511_REGISTER_A = 0x0002
-    EM511_REGISTER_PASSWORD = 0x1000
-
-    SCALE_10 = 10
-    SCALE_100 = 100
-    SCALE_1000 = 1000
+    _register_specs: Dict = {^M
+        "V": RegisterSpec(address=0x0000, count=INT32_REG_COUNT, scale=10, decimals=1),^M
+        "A": RegisterSpec(address=0x0002, count=INT32_REG_COUNT, scale=1000, decimals=1),^M
+    }^M
 
     def __init__(self, device_address: int, client: ModbusSerialClient) -> None:
         """Initialize an Em511 driver instance with an existing Modbus client.
@@ -121,70 +143,8 @@ class Em511:
         msg = f"Unexpected register count: {len(regs)}."
         raise ValueError(msg)
 
-    @property
-    def V(self) -> Decimal:
-        """Voltage (V).
-
-        Returns:
-            Decimal: Current voltage value.
-
-        Raises:
-            ValueError: If input is at max value or above.
-            ModbusException: If failed to read input register.
-        """
-        regs = self._read_input_registers(self.EM511_REGISTER_V, self.INT32_REG_COUNT)
-        value = Decimal(self._unpack(regs, self.EM511_REGISTER_V)) / self.SCALE_10
-        return round(value, 1)
-
-    @property
-    def A(self) -> Decimal:
-        """Current (A).
-
-        Returns:
-            Decimal: Current ampere value.
-
-        Raises:
-            ValueError: If input is at max value or above.
-            ModbusException: If failed to read input register.
-        """
-        regs = self._read_input_registers(self.EM511_REGISTER_A, self.INT32_REG_COUNT)
-        value = Decimal(self._unpack(regs, self.EM511_REGISTER_A)) / self.SCALE_1000
-        return round(value, 3)
-
-    @property
-    def password(self) -> int:
-        """Password.
-
-        Returns:
-            int: Current password value.
-
-        Raises:
-            ValueError: If input is at max value or above.
-            ValueError: If password is out of range.
-            ModbusException: If failed to read input register.
-        """
-        regs = self._read_input_registers(self.EM511_REGISTER_PASSWORD, self.INT16_REG_COUNT)
-        value = self._unpack(regs, self.EM511_REGISTER_PASSWORD)
-        if not (self.PASSWORD_MIN_VALUE <= value <= self.PASSWORD_MAX_VALUE):
-            msg = f"Invalid password value: {value}. Must be between 0 and 9999."
-            raise ValueError(msg)
-        return value
-
-    @password.setter
-    def password(self, value: int) -> None:
-        """Password.
-
-        Min value: 0 (no password).
-        Max value: 9999.
-
-        Args:
-            value (int): Set Password.
-
-        Raises:
-            ModbusException: If failed to write to single register.
-            ValueError: If password value is out of range.
-        """
-        if not (self.PASSWORD_MIN_VALUE <= value <= self.PASSWORD_MAX_VALUE):
-            msg = f"Invalid password value: {value}. Must be between 0 and 9999."
-            raise ValueError(msg)
-        self._write_register(self.EM511_REGISTER_PASSWORD, value)
+    def _read_register(self, register_name: str) -> Decimal:^M
+        spec = self._register_specs[register_name]^M
+        regs = self._read_input_registers(spec.address, spec.count)^M
+        value = Decimal(self._unpack(regs, spec.address)) / spec.scale^M
+        return round(value, spec.decimals)^M

@mirzak

mirzak commented Oct 31, 2025

Copy link
Copy Markdown
Contributor

Superseded by #43

@mirzak mirzak closed this Oct 31, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants