unlobito / ha-hildebrandglow

PRE-RELEASE. HomeAssistant integration for the Hildebrand Glow smart meter HAN for UK SMETS meters.
MIT License
54 stars 31 forks source link

_hex_twos_complement_to_decimal error #43

Closed rit001 closed 2 years ago

rit001 commented 3 years ago

Sadly things are never easy....

The latest update that deals with the correcting of the electricity value for anyone with local power generation now generates the following error within the Home Assistant log

2021-10-14 18:47:50 ERROR (Thread-4) [root] Uncaught thread exception Traceback (most recent call last): File "/usr/local/lib/python3.9/threading.py", line 973, in _bootstrap_inner self.run() File "/usr/local/lib/python3.9/threading.py", line 910, in run self._target(*self._args, **self._kwargs) File "/usr/local/lib/python3.9/site-packages/paho/mqtt/client.py", line 3452, in _thread_main self.loop_forever(retry_first_connection=True) File "/usr/local/lib/python3.9/site-packages/paho/mqtt/client.py", line 1779, in loop_forever rc = self.loop(timeout, max_packets) File "/usr/local/lib/python3.9/site-packages/paho/mqtt/client.py", line 1181, in loop rc = self.loop_read(max_packets) File "/usr/local/lib/python3.9/site-packages/paho/mqtt/client.py", line 1572, in loop_read rc = self._packet_read() File "/usr/local/lib/python3.9/site-packages/paho/mqtt/client.py", line 2310, in _packet_read rc = self._packet_handle() File "/usr/local/lib/python3.9/site-packages/paho/mqtt/client.py", line 2936, in _packet_handle return self._handle_publish() File "/usr/local/lib/python3.9/site-packages/paho/mqtt/client.py", line 3216, in _handle_publish self._handle_on_message(message) File "/usr/local/lib/python3.9/site-packages/paho/mqtt/client.py", line 3444, in _handle_on_message self.on_message(self, self._userdata, message) File "/config/custom_components/hildebrandglow/glow.py", line 131, in _cb_on_message payload = MQTTPayload(msg.payload) File "/config/custom_components/hildebrandglow/mqttpayload.py", line 250, in init Meter(payload["elecMtr"]) if "03" in payload["elecMtr"]["0702"] else None File "/config/custom_components/hildebrandglow/mqttpayload.py", line 225, in init self.historical_consumption = self.HistoricalConsumption(payload) File "/config/custom_components/hildebrandglow/mqttpayload.py", line 162, in init self._hex_twos_complement_to_decimal(historical_consumption["00"]) File "/config/custom_components/hildebrandglow/mqttpayload.py", line 184, in _hex_twos_complement_to_decimal return int(struct.unpack(">i", bytes.fromhex(hex_value))[0]) struct.error: unpack requires a buffer of 4 bytes

My system is a new install (as of yesterday) using 2021.10.4 in supervised mode and the deployment platform is a Pi 4 running Pi OS in 32 bit mode.

danstreeter commented 3 years ago

Agh =( I’m yet to update myself to 2021.10 due to others experiencing issues with the energy functionality.

I’ll try and spin up a dev env on it and see if I can replicate.

rit001 commented 3 years ago

The error seems to be at the Python level, a language I've spend more time on today than I have since it was created :)

The best I can do is provide the following system info - while the Pi uses Python 2.x by default the HA container is running a 3.x release

System Health

version core-2021.10.4
installation_type Home Assistant Supervised
dev false
hassio true
docker true
user root
virtualenv false
python_version 3.9.7
os_name Linux
os_version 5.10.63-v7l+
arch armv7l
timezone Europe/London
Home Assistant Community Store GitHub API | ok -- | -- Github API Calls Remaining | 5000 Installed Version | 1.15.2 Stage | running Available Repositories | 878 Installed Repositories | 3
Home Assistant Cloud logged_in | false -- | -- can_reach_cert_server | ok can_reach_cloud_auth | ok can_reach_cloud | ok
Home Assistant Supervisor host_os | Raspbian GNU/Linux 10 (buster) -- | -- update_channel | stable supervisor_version | supervisor-2021.10.0 docker_version | 20.10.9 disk_total | 233.5 GB disk_used | 5.6 GB healthy | true supported | failed to load: Unsupported supervisor_api | ok version_api | ok installed_addons | Mosquitto broker (6.0.1), Terminal & SSH (9.2.1)
Lovelace dashboards | 1 -- | -- resources | 0 mode | auto-gen
danstreeter commented 3 years ago

The error is stating that one of the functions used to calculate a human version of the value isn’t getting what it wants/needs to do so… I’ll make change to log that value out reliably and figure out what your instance is getting that mine isnt, can then figure out how to handle that.

rit001 commented 3 years ago

OK, one thing to consider is a value less than 255 as that would be possible at this time of day for my system and result in a single byte return.

No scratch this idea as I just increased my load to over 255 and did a reboot, with the same error as the module loaded. I'm not going to try 65536 :)

danstreeter commented 3 years ago

I believe the actual raw value within the payload has already been converted to a signed 24 bit integer or something like that (working off memory and on phone, can’t search the spec pdf) so whatever you are receiving whatever the value should be pre converted to the four bytes needed

rit001 commented 3 years ago

I've no real idea if it will help, but below is the raw mqtt data from glowmqtt.energyhive.com for the electricity meter is formatted as

  "04":{"01":"000802","00":"0000AF"},"02":{"00":"00"}

From the little, I understand the key part is "00":"0000AF" as that relates to the meter reading in watts.

danstreeter commented 3 years ago

Well thats odd.. Yes that is the target attribute that corresponds to 'Instantatious Demand' - the value we are looking for in Watts that is consumption/generation.

Mine right now:

{
    "elecMtr": {
        "0702": {
            "03": {"truncated"},
            "00": {"truncated"},
            "04": {
                "01": "001B4B",
                "40": "023939",
                "30": "00B1E4",
                "00": "00000992"
            },
            "continues"

So my 0702.04.00 value is 00000992. Putting this through https://www.rapidtables.com/convert/number/hex-to-decimal.html gives me a value of 2450 in the "Decimal from signed 2's complement" box - which is correct as my dishwasher is currently running.

Yours - gives a N/A in that box... which is probably why when the python function runs and it tries to unpack what it is expecting as a four byte value - it fails as this is not a signed 24 bit ineger (*ref 1)

Hrm... I wonder why you are getting an un-convertable number...


Update - I can confirm that a 2021.11.0.dev0 locally running instance, with the mqtt branch of this code works still as expected, reporting the correct data to Home Assistant.

Maybe reach out to the guys on https://forum.glowmarkt.com/index.php?p=/discussions and see if you can diagnose why the Hex value you are getting as your 0702.04.00 value is not two's compliment 'decodable' (not sure thats the right word)

rit001 commented 3 years ago

I think the bigger question is why you are getting back an 8 digit value (32 bit) for a 24bit wide value.

I'm going to hook up a 3kw heater and see what numbers I get back from the mqtt.

danstreeter commented 3 years ago

Its rather late for me to be organising my bits and bytes...lol and I'm NO mathematician at any time of the day - but because its a "signed twos complimented" value maybe?

There seems to be refereence all over the place about shifting bits and adding them in here: https://en.wikipedia.org/wiki/Two%27s_complement

The two's complement is calculated by inverting the bits and adding one.

rit001 commented 3 years ago

OK, after adding a load to my environment I get the following mqtt reading

  "04":{"01":"000A8C","00":"000803"},"02":{"00":"00"}}}

So I am staying at a 24 bit returned value, which is also what the example sent out by hildebrandglow shows, but with the following statement

"There is a small level of variation between SMETS1 and SMETS2 as to what data is presented but broadly speaking this is what you get."

By any chance do you have a SMETS2 meter as I have a SMETS1 version?

Unless you are building logic boards 2's complement is not that hard - if the top-most bit is 1 the number is negative in nature, if it is 0 it is positive. This then allows you to do the complement if you need the negative value for something.

The problem here is more than when using standard patterns in Python it seems you need to know the exact size of what you are dealing with, the mqtt output is not consistent and the function/method does not seem designed to cope with a 6 digit string as it does not map directly to a 4 byte int.

As I do not know Python I can not propose a sensible solution, just a hack - if the string is 6 characters in length convert it to 8 by (note this is not python code, just an indication of what is needed)

   newstring = string[0]+string[0]+string

I'm sure there is something better, but that would handle any value between -2^20 and 2^20 and I would hope that no one is using these tools to monitor +1MW loads.

danstreeter commented 3 years ago

I am indeed on a SMETS2 meter.

If this is the case, and SMETS1 is spitting out a 24 bit, and SMETS2 a 32 bit - and can be relied upon always doing so, (it may be worth checking the spec to see if there is a identified on SMETS version or not?) then surely converting it into a common form like you say is a decent enough option to ensure that the conversion works out - or it does a SMETS1 conversion and a SMETS2 conversion based on the inbound data / SMETS type.

rit001 commented 3 years ago

The spec states that the value is 24 bits in size, the problem is that is just a problematic size to select - no processor has 24 bit wide registers, so they selected that size to remove bytes from the data packets. This then just causes possible issues at every conversion point - and becomes a prime example of why test driven development works so well for interfaces :)

So someone somewhere in the data exchange path has some code that is doing the same type of casting as your Python code, but handerling the 24 bit to 32 bit conversion without any issues - the developer then continues to use the 32 bit value without any thought to the issue when they then output it as a text string again, resulting in an 8 character string. The chances are that there is indepedent code for SMETS2 and SMETS1 devices due to the way that the data exchange network was coded.

This topic may lead to a better way to handle the conversion of a 24 bit value.

     https://stackoverflow.com/questions/3783677/how-to-read-integers-from-a-file-that-are-24bit-and-little-endian-using-python

The nice thing about 2's complement is that you can disregard it for this issue, I think you just need to check the length of the string and then select a conversion based on its length - 6 or 8 characters.

rit001 commented 3 years ago

I just exchanged some emails with a member of staff at Hildebrand and queried this issue. They just relay what they retrieve from the meter, so the string length difference is down to the implementation of the standards document in the meter's firmware.

Good to know that the meter industry can not even follow its own standards.

unlobito commented 3 years ago

Hey @rit001 and @danstreeter,

I've pushed up ab265bdb22c73ae7ceab8a550dfc34e38b3bccd7 which implements @rit001's suggestion of checking the string's length.

I have a SMETS1 meter at home as well, so had been limited in my understanding of where the different data types originate from. Really appreciate the details you've both shared toward investigating this issue.