search

esphome / issues

Issue Tracker for ESPHome
https://esphome.io/
291 stars 34 forks source link

sml component convert value wrong #4858

Closed drakesoft closed 8 months ago

drakesoft commented 10 months ago

The problem

The reading 1-0:1.8.0 from meter marked with (x) is wrongly converted to a very very large number. Another meter with identical configuration does not show this issue. Maybe it has something to do with leading zeros? The meter with no issues shows leading zeros in hex value.

Which version of ESPHome has the issue?

2023.8.3

What type of installation are you using?

Home Assistant Add-on

Which version of Home Assistant has the issue?

No response

What platform are you using?

ESP32

Board

Lolin32-lite

Component causing the issue

sml

Example YAML snippet

- platform: sml
  sml_id: sml_home
  name: "ESP Bezug Energie"
  obis_code: "1-0:1.8.0"
  unit_of_measurement: kWh
  accuracy_decimals: 0
  device_class: energy
  state_class: total_increasing
  filters:
    - multiply: 0.0001
    - throttle: 300s

- platform: sml
  sml_id: sml_pv
  name: "ESP PV Bezug Energie"
  obis_code: "1-0:1.8.0"
  unit_of_measurement: kWh
  accuracy_decimals: 0
  device_class: energy
  state_class: total_increasing
  filters:
    - multiply: 0.0001
    - throttle: 300s

Anything in the logs that might be useful for us?

[10:06:07][D][sml:066]: OBIS info:
[10:06:07][D][sml:072]:   (x) 1-0:96.50.1 [0x454652]
[10:06:07][D][sml:072]:   (x) 1-0:96.1.0 [0xx]
[10:06:07][D][sml:072]:   (x) 1-0:1.8.0 [0xcd51e2]
[10:06:07][D][sml:072]:   (x) 1-0:2.8.0 [0x1c7c8aca]
[10:06:07][D][sml:072]:   (x) 1-0:16.7.0 [0x00]
[10:06:07][D][sml:072]:   (x) 1-0:32.7.0 [0x0914]
[10:06:07][D][sml:072]:   (x) 1-0:52.7.0 [0x08e3]
[10:06:07][D][sml:072]:   (x) 1-0:72.7.0 [0x08dd]
[10:06:07][D][sml:072]:   (x) 1-0:31.7.0 [0x00]
[10:06:07][D][sml:072]:   (x) 1-0:51.7.0 [0x00]
[10:06:07][D][sml:072]:   (x) 1-0:71.7.0 [0x00]
[10:06:07][D][sml:072]:   (x) 1-0:81.7.1 [0x79]
[10:06:07][D][sml:072]:   (x) 1-0:81.7.2 [0x00ee]
[10:06:07][D][sml:072]:   (x) 1-0:81.7.4 [0x00]
[10:06:07][D][sml:072]:   (x) 1-0:81.7.15 [0x00]
[10:06:07][D][sml:072]:   (x) 1-0:81.7.26 [0x00]

[10:06:07][D][sml:066]: OBIS info:
[10:06:07][D][sml:072]:   (y) 1-0:96.50.1 [0x484c59]
[10:06:07][D][sml:072]:   (y) 1-0:96.1.0 [0xy]
[10:06:07][D][sml:072]:   (y) 1-0:1.8.0 [0x0003b882]
[10:06:07][D][sml:072]:   (y) 1-0:2.8.0 [0x03a79e60]
[10:06:07][D][sml:072]:   (y) 1-0:16.7.0 [0xd641]
[10:06:07][D][sml:072]:   (y) 1-0:36.7.0 [0xf20f]
[10:06:07][D][sml:072]:   (y) 1-0:56.7.0 [0xf20e]
[10:06:07][D][sml:072]:   (y) 1-0:76.7.0 [0xf226]
[10:06:07][D][sml:072]:   (y) 1-0:32.7.0 [0x0911]
[10:06:07][D][sml:072]:   (y) 1-0:52.7.0 [0x08e3]
[10:06:07][D][sml:072]:   (y) 1-0:72.7.0 [0x08dd]
[10:06:07][D][sml:072]:   (y) 1-0:31.7.0 [0x067d]
[10:06:07][D][sml:072]:   (y) 1-0:51.7.0 [0x06bf]
[10:06:07][D][sml:072]:   (y) 1-0:71.7.0 [0x06c2]
[10:06:07][D][sml:072]:   (y) 1-0:81.7.1 [0x77]

Additional information

Screenshot_20230906_104001

gitspm commented 10 months ago

same here

fblaese commented 9 months ago

I have the same issue. I haven't verified this yet, but it seems fairly likely that this is related to the following change: https://github.com/esphome/esphome/commit/6089526975ee74b73d5b4975b7b481f37d50ac58

The meter model I am reading from is a "eBZD-W1E3-0L-HL0-D4-000001-F50/Q2". The value sent by the smart meter is 0xcad3a9 and gets converted to 18446744073709551616 (0x10000000000000000) in the SML sensor entry.

(0xca & 0x80) is true.

The issue probably appears because the only 3 bytes are transmitted when the number is small enough, so buffer.size() is 3. The issue neither appears for a smaller (0x69275d) number nor a bigger number (0x0133fb06)

Additional information:

[..]
[17:15:16][D][sensor:094]: 'Stromzähler Debug': Sending state 18446744073709551616.00000  with 0 decimals of accuracy
[..]
[17:15:16][D][sml:066]: OBIS info:
[17:15:16][D][sml:072]:   (xx) 1-0:96.50.1 [xx]
[17:15:16][D][sml:072]:   (xx) 1-0:96.1.0 [xx]
[17:15:16][D][sml:072]:   (xx) 1-0:1.8.0 [0x0133fb06]
[17:15:16][D][sml:072]:   (xx) 1-0:1.8.1 [0x69275d]    <<--- this one is fine
[17:15:16][D][sml:072]:   (xx) 1-0:1.8.2 [0xcad3a9]    <<--- this is the measurement resulting in wrong values
[17:15:16][D][sml:072]:   (xx) 1-0:2.8.0 [0x0aad]      <<--- this one as well
[17:15:16][D][sml:072]:   (xx) 1-0:16.7.0 [xx]
drakesoft commented 9 months ago

Hi fblaese, I can confirm your suspicion. The change https://github.com/esphome/esphome/commit/6089526975ee74b73d5b4975b7b481f37d50ac58 cause this issue. I just fixed it by removing the few lines of code.

fblaese commented 8 months ago

@mulder-fbi @jesserockz

Is this extension of 24 bit signed integers part of the SML standard or is this a workaround for broken devices? Or is it my meter that is broken? Is there a way to differentiate between signed and unsigned values, so the extension is only applied to signed integers?

mulder-fbi commented 8 months ago

I have seen the 24 bit sigend integer behaviour with a DWS7612.2 smart meter. The protocol definition (unfortunately in german) I referenced when debugging the issue only talks about 8, 16, 32 and 64 bit integers (see 6.2.2). But it also allows for "full bytes with leading zeros or ones" to be omitted when the data is being transferred, if this does not change the value on the receiving side. So sending a 24 bit signed integer doesn't seem to violate the data type definition in general. 6.2.3. than goes on and allows omitting leading zeros for unsigned values. So from my point of view there is no way to implicitly derive the datatype on the receiving side. As far as I know the actual data type is not transmitted inside the message, so the only way to I could think of would be to add a signed/unsigned config flag in the yaml.

fblaese commented 8 months ago

According to 6.2.3, leading-zero bytes can be ommited for unsigned data types without any restrictions, so the sent value is ambigous, sadly. The mentioned restriction only seems to apply for the signed data type.

I have found the following: "Aus der OBIS Kennzahl leitet sich implizit der konkrete Datentyp und der Wertebereich für das zu liefernde Ergebnis ab" (5.1.9) which means something like "The OBIS code implicitly defines the data type and its range". So it should be possible to derive the data type from the OBIS code. The full list of OBIS codes is defined in IEC 62056 and can be found here: https://www.promotic.eu/en/pmdoc/Subsystems/Comm/PmDrivers/IEC62056_OBIS.htm

I am not sure if vendor-specific extensions are allowed.

A manual flag for signed/unsigned extension might be a good first step to fix the regression, with proper handling of the OBIS codes (if possible) later on.

fblaese commented 8 months ago

Actually, after skimming through the protocol definition a second time, it looks like it should be possible to derive the data type from the type-length (TL) field. A TL-value matching (0b01010000 & 0b01110000) should represent a signed integer while a TL-value matching (0b01100000 & 0b01110000) should represent an unsigend integer. (See 6.1 and 6.2f.)

fblaese commented 8 months ago

It looks like the code already properly represents this as well. bytes_to_uint is only called for unsigned integers. However, its code has been reused in the bytes_to_int function.

The sign-extension should probably be placed in bytes_to_int instead. I will do a PR with the required changes.

mulder-fbi commented 8 months ago

Ok, good catch. Thanks :)

fblaese commented 8 months ago

I have made a PR, but I currently do not have a testing environment available. @drakesoft it would be great if you could test if the changes in https://github.com/esphome/esphome/pull/5544 fix the issue.

mulder-fbi commented 8 months ago

I have tested your changes (on ESP8266) and can confirm, that negative signed integers still work as expected. But I have no way to test the other failure mode reported here.

fblaese commented 8 months ago

I have quickly set up a development and testing environment myself. It looks like my code does work as intended and does fix the issue of the original issue creator as well as maintaining correct sign extension for abbreviated transmissions.

I have therefore removed the draft tag from the PR.

drakesoft commented 8 months ago

@fblaese I tested the changes. Everything seems fine. :-) Great work. Thanks a lot