powermeter: add support for SML over serial#229
Conversation
This commit adds support to read the power reading directly via the serial port and the SML protocol from energy meters that support outputing via the IR port.
WalkthroughThis PR introduces SML (Smart Meter Language) powermeter support by adding dependencies, implementing a serial-based power meter reader, extending the configuration system to recognize and instantiate SML powermeters, and documenting the feature with examples. Changes
Sequence Diagram(s)sequenceDiagram
participant Config as Config Loader
participant Sml as Sml Instance
participant Serial as Serial Device
participant Parser as smllib
participant Stats as EnergyStats
Config->>Sml: create_sml_powermeter(config)
Sml->>Sml: __init__(serial_device)
Note over Sml: Later: Power Reading Request
Sml->>Sml: get_powermeter_watts()
Sml->>Sml: read_serial()
Sml->>Serial: acquire lock & open serial
Serial-->>Sml: serial data stream
Sml->>Parser: SmlStreamReader.feed(data)
Parser-->>Sml: parsed SmlFrame
Sml->>Stats: from_sml_frame(frame)
Stats-->>Sml: EnergyStats with power values
Sml->>Sml: return [current_power_watts]
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@config/config_loader.py`:
- Around line 170-174: create_sml_powermeter currently passes an empty string as
fallback so Sml(config.get(..., fallback="")) overrides Sml's default
"/dev/ttyUSB0" and can cause serial-open failures; change create_sml_powermeter
to read the SERIAL value (e.g. via config.get with fallback=None or similar) and
if the retrieved value is falsy/empty call Sml() without arguments (or
explicitly pass "/dev/ttyUSB0"), otherwise pass the retrieved serial string to
Sml; update the logic inside create_sml_powermeter (reference function name
create_sml_powermeter and class Sml) so the Sml constructor default remains
reachable.
In `@powermeter/sml.py`:
- Around line 75-91: In read_serial, the logger.debug call in function
read_serial (and its message string) contains a typo "attemps"; update the log
message in the logger.debug invocation inside read_serial (where it logs "got
sml frame: %s after %s attemps") to use the correct word "attempts" so the
message reads "... after %s attempts"; ensure only the string is changed and
formatting/arguments (self._current, i) remain unchanged.
- Around line 28-41: The from_sml_frame method in EnergyStats uses assert for
unit checks which can be disabled; replace those asserts with explicit runtime
validation that raises a clear exception (e.g., ValueError) when UNITS[ov.unit]
is not the expected unit for the given OBIS code (_OBIS_POWER_CURRENT -> "W",
_OBIS_POWER_IN_TOTAL/_OBIS_POWER_OUT_TOTAL -> "Wh"); validate each ov from
sml_frame.get_obis() before assigning stats.current_power, stats.total_power_in,
and stats.total_power_out and include the actual ov.unit and ov.value in the
error message to aid debugging.
🧹 Nitpick comments (2)
powermeter/sml.py (2)
58-73:_try_read_framereturnsNoneeven after a successfulstream.add— consider documenting or restructuring.When
sml_frameisNoneandser.readreturns data, the method adds data to the stream but still returnsNone. The frame will only be retrieved on the next call to_try_read_frame. This works because of the retry loop inread_serial, but the method name_try_read_frameis slightly misleading since it may add data without attempting to parse again.Also, the bare
except Exception(line 64) is flagged by Ruff (BLE001). Consider narrowing tosmllib.errors.SmlErroror whatever the library's base exception is, if one exists, to avoid masking unrelated bugs.
75-91: Silent fallback to stale/zero data when all read attempts fail.If all 10 iterations exhaust without a valid frame,
read_seriallogs an error butget_powermeter_wattsstill returnsself._current.current_power, which is either the previous reading or the default0. The caller has no indication the reading is stale or failed.This may be acceptable for resilience (the author mentions stable behavior over weeks), but worth noting that downstream consumers will silently receive outdated values.
| def create_sml_powermeter( | ||
| section: str, config: configparser.ConfigParser | ||
| ) -> Powermeter: | ||
| return Sml(config.get(section, "SERIAL", fallback="")) | ||
|
|
There was a problem hiding this comment.
Fallback "" overrides the Sml constructor's default of "/dev/ttyUSB0".
If a user adds an [SML] section without specifying SERIAL, the fallback="" here will pass an empty string to Sml(), which will fail when opening the serial port. The Sml.__init__ default of "/dev/ttyUSB0" is effectively unreachable. Consider aligning the fallback:
Proposed fix
- return Sml(config.get(section, "SERIAL", fallback=""))
+ return Sml(config.get(section, "SERIAL", fallback="/dev/ttyUSB0"))📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def create_sml_powermeter( | |
| section: str, config: configparser.ConfigParser | |
| ) -> Powermeter: | |
| return Sml(config.get(section, "SERIAL", fallback="")) | |
| def create_sml_powermeter( | |
| section: str, config: configparser.ConfigParser | |
| ) -> Powermeter: | |
| return Sml(config.get(section, "SERIAL", fallback="/dev/ttyUSB0")) |
🤖 Prompt for AI Agents
In `@config/config_loader.py` around lines 170 - 174, create_sml_powermeter
currently passes an empty string as fallback so Sml(config.get(...,
fallback="")) overrides Sml's default "/dev/ttyUSB0" and can cause serial-open
failures; change create_sml_powermeter to read the SERIAL value (e.g. via
config.get with fallback=None or similar) and if the retrieved value is
falsy/empty call Sml() without arguments (or explicitly pass "/dev/ttyUSB0"),
otherwise pass the retrieved serial string to Sml; update the logic inside
create_sml_powermeter (reference function name create_sml_powermeter and class
Sml) so the Sml constructor default remains reachable.
| def from_sml_frame(cls, sml_frame: SmlFrame) -> "EnergyStats": | ||
| stats = cls() | ||
| for ov in sml_frame.get_obis(): | ||
| if ov.obis == _OBIS_POWER_CURRENT: | ||
| stats.current_power = ov.value | ||
| assert UNITS[ov.unit] == "W" | ||
| elif ov.obis == _OBIS_POWER_IN_TOTAL: | ||
| stats.total_power_in = ov.value | ||
| assert UNITS[ov.unit] == "Wh" | ||
| elif ov.obis == _OBIS_POWER_OUT_TOTAL: | ||
| stats.total_power_out = ov.value | ||
| assert UNITS[ov.unit] == "Wh" | ||
| stats.when = datetime.datetime.now() | ||
| return stats |
There was a problem hiding this comment.
Replace assert with explicit validation — assertions are stripped under python -O.
Using assert for runtime validation (lines 33, 36, 39) is unsafe in production: running Python with -O (optimize) disables all assertions silently, allowing incorrect unit data to be accepted without any error.
🛡️ Proposed fix
if ov.obis == _OBIS_POWER_CURRENT:
stats.current_power = ov.value
- assert UNITS[ov.unit] == "W"
+ if UNITS.get(ov.unit) != "W":
+ raise ValueError(f"Unexpected unit for current power: {UNITS.get(ov.unit)}")
elif ov.obis == _OBIS_POWER_IN_TOTAL:
stats.total_power_in = ov.value
- assert UNITS[ov.unit] == "Wh"
+ if UNITS.get(ov.unit) != "Wh":
+ raise ValueError(f"Unexpected unit for total power in: {UNITS.get(ov.unit)}")
elif ov.obis == _OBIS_POWER_OUT_TOTAL:
stats.total_power_out = ov.value
- assert UNITS[ov.unit] == "Wh"
+ if UNITS.get(ov.unit) != "Wh":
+ raise ValueError(f"Unexpected unit for total power out: {UNITS.get(ov.unit)}")🤖 Prompt for AI Agents
In `@powermeter/sml.py` around lines 28 - 41, The from_sml_frame method in
EnergyStats uses assert for unit checks which can be disabled; replace those
asserts with explicit runtime validation that raises a clear exception (e.g.,
ValueError) when UNITS[ov.unit] is not the expected unit for the given OBIS code
(_OBIS_POWER_CURRENT -> "W", _OBIS_POWER_IN_TOTAL/_OBIS_POWER_OUT_TOTAL ->
"Wh"); validate each ov from sml_frame.get_obis() before assigning
stats.current_power, stats.total_power_in, and stats.total_power_out and include
the actual ov.unit and ov.value in the error message to aid debugging.
| def read_serial(self) -> None: | ||
| if not self._lock.acquire(blocking=False): | ||
| return | ||
| try: | ||
| stream = SmlStreamReader() | ||
| with serial.Serial(self._serial_device, 9600, timeout=10) as ser: | ||
| data = ser.read(512) | ||
| stream.add(data) | ||
| for i in range(10): | ||
| sml_frame = self._try_read_frame(ser, stream) | ||
| if sml_frame is not None: | ||
| self._current = EnergyStats.from_sml_frame(sml_frame) | ||
| logger.debug("got sml frame: %s after %s attemps", self._current, i) | ||
| return | ||
| logger.error("failed to read SML frame after 10 attempts") | ||
| finally: | ||
| self._lock.release() |
There was a problem hiding this comment.
Typo: "attemps" → "attempts" on line 87.
📝 Fix
- logger.debug("got sml frame: %s after %s attemps", self._current, i)
+ logger.debug("got sml frame: %s after %s attempts", self._current, i)🤖 Prompt for AI Agents
In `@powermeter/sml.py` around lines 75 - 91, In read_serial, the logger.debug
call in function read_serial (and its message string) contains a typo "attemps";
update the log message in the logger.debug invocation inside read_serial (where
it logs "got sml frame: %s after %s attemps") to use the correct word "attempts"
so the message reads "... after %s attempts"; ensure only the string is changed
and formatting/arguments (self._current, i) remain unchanged.
1 participant
This commit adds support to read the power reading directly via the serial port and the SML protocol from energy meters that support outputing via the IR port.
(I'm using this since a couple of weeks and it seems to be stable for me)
Summary by CodeRabbit
New Features
Documentation