denpamusic/PyPlumIO

Error in parsin device parameters

svenlange2 opened this issue · 16 comments

I get contantly this error:
Traceback (most recent call last):
File "/usr/local/lib/python3.9/site-packages/pyplumio/connection.py", line 107, in _process
device.set_data(frame.data)
File "/usr/local/lib/python3.9/site-packages/pyplumio/frame.py", line 99, in data
self.parse_message(self.message)
File "/usr/local/lib/python3.9/site-packages/pyplumio/responses.py", line 218, in parse_message
param_type.unpack(message[offset:])
File "/usr/local/lib/python3.9/site-packages/pyplumio/data_types.py", line 45, in unpack
self._data = data[0 : self.size]
File "/usr/local/lib/python3.9/site-packages/pyplumio/data_types.py", line 321, in size
return len(self.value) + 1
File "/usr/local/lib/python3.9/site-packages/pyplumio/data_types.py", line 312, in value
while not self._data[offset] == "\x00":
AttributeError: 'String' object has no attribute '_data'

It appears in library and affects HASS as well. It seems that for my device Ecomax920 the parameters are incorrectly detected and the libarary tries to unpack the parameters from messages wrong. In this case it the last parameter in the schema and message only has left three zero bytes while it tries to decode string. This requires according tho the code thou at least four bytes. Interestingly sometimes i dont see it. But thats only occasionally. It seems the messages from the device change somehow.

Hmm... It's more of a chicken-egg problem than anything.

  • To extract the bytes for a string datatype, it needs to get string size.
  • To get string size, it needs to get string value.
  • To get string value, it needs to iterate through extracted bytes.

And so the circle completes, once it gets from extracting bytes to calculating value, PyPlumIO realizes that there is no bytes set in object and crashes.

EM350 doesn't produce strings and test coverage doesn't include unpack for string data type yet, so this remained uncaught.
I'll try to refactor String datatype parser to get rid of this issue.

Done. Basically we now set whole bytesteam as object data, then find string in it and cut bytestream to it's size. I'll probably release a whole minor version just for this (and better exception handling😄) once ci finishes. No point in saving up version numbers in alpha releases anyway.

P. S. I also added test case, so that it'll never happen again :)

Now I got this while testing with python directly

Traceback (most recent call last):
File "C:\Users\Sven\AppData\Local\Programs\Python\Python310\lib\site-packages\pyplumio\connection.py", line 107, in _process
device.set_data(frame.data)
File "C:\Users\Sven\AppData\Local\Programs\Python\Python310\lib\site-packages\pyplumio\frame.py", line 99, in data
self.parse_message(self.message)
File "C:\Users\Sven\AppData\Local\Programs\Python\Python310\lib\site-packages\pyplumio\responses.py", line 218, in parse_message
param_type.unpack(message[offset:])
File "C:\Users\Sven\AppData\Local\Programs\Python\Python310\lib\site-packages\pyplumio\data_types.py", line 314, in unpack
super().unpack(self._data)
File "C:\Users\Sven\AppData\Local\Programs\Python\Python310\lib\site-packages\pyplumio\data_types.py", line 45, in unpack
self._data = data[0 : self.size]
File "C:\Users\Sven\AppData\Local\Programs\Python\Python310\lib\site-packages\pyplumio\data_types.py", line 330, in size
return len(self.value) + 1
File "C:\Users\Sven\AppData\Local\Programs\Python\Python310\lib\site-packages\pyplumio\data_types.py", line 322, in value
value += self._data[offset]
TypeError: can only concatenate str (not "int") to str

A little trace of what i see in modbus log of the self made device that relays the messages. "<<<" means its a packet sent from plumio to the device, other are responses after that in opposite direction. Maybe it helps:

"<<<" 682D04556305B01C0A8B70FFFFFF0000010000FFFFFF000001164000000A416
">>>" 684E010045FC3008626400010855F7B15420BE6101003D183136010064010040041C56599D0000000000FF0FFF0FFF0FFF0FFF0FFF0F9D04090FFF0FFF0F0000000000000000000000000000000000000000000000000000C07F0000C07F0000C07F0000C07F0000C07F0000C07F1894B241000000000000C07F0000C07F0000C07F0000C07F0000C07F0000C07F0000C07F0000C07F0000C07F0000C07F0000C07F0000C07F0000C07F0000C07F2D280000000029000000002828000000002828000000800000000000000000000000000000000000000000003FFF7F00000000200000000000404000403F124B01000000000000000000000002020100000000000000000000000000000000000000000000150009001A000D000C001D000000000000000000000000000000C0A80B70FFFFFF00000000000100000000FFFFFF00000000000101640000000516
">>>" 68A3000045FC30350755F7B15420BE56599D3601003802003901003D18310000000000FF03000009001894B24101FFFFFFFF02FFFFFFFF03FFFFFFFF04FFFFFFFF05FFFFFFFF060000000007FFFFFFFF08FFFFFFFF29002D800020000000000000000000000000000001120B3A4B01FFFFFFFF120A48FFFF05FFFFFFFF28000800FFFFFFFF28000800FFFFFFFF28000800FFFFFFFF28000800FFFFFFFF280008002116
">>>" 680A005045FC300AB1166824000050FC388954494D45090A2FE607041D050A4800000000000256599D3D18315316
">>>" 680A005945FC3040F216

Actually thi is better. Its the full log after I start python.

" <<<" 682D04556305B010000FFFFFF0000000000FFFFFF000001164000000B616
">>>" 684E010045FC3008626400010855F7B15420BE6101003D183136010064010040041C5698FA0000000000FF0FFF0FFF0FFF0FFF0FFF0F9F04080FFF0FFF0F0000000000000000000000000000000000000000000000000000C07F0000C07F0000C07F0000C07F0000C07F0000C07FD012B341000000000000C07F0000C07F0000C07F0000C07F0000C07F0000C07F0000C07F0000C07F0000C07F0000C07F0000C07F0000C07F0000C07F0000C07F2D280000000029000000002828000000002828000000800000000000000000000000000000000000000000003FFF7F00000000200000000000404000403F124B01000000000000000000000002020100000000000000000000000000000000000000000000150009001A000D000C001D00000000000000000000000000000000000000FFFFFF00000000000000000000FFFFFF0000000000010164000000FD16
">>>"
68A3000045FC30350755F7B15420BE5698FA3601003802003901003D18310000000000FF0300000900D012B34101FFFFFFFF02FFFFFFFF03FFFFFFFF04FFFFFFFF05FFFFFFFF060000000007FFFFFFFF08FFFFFFFF29002D800020000000000000000000000000000001120B3A4B01FFFFFFFF120A48FFFF05FFFFFFFF28000800FFFFFFFF28000800FFFFFFFF28000800FFFFFFFF28000800FFFFFFFF28000800C816
"<<<" 68A0056305555416
">>>" 680F030045FC30D501010400070A02060A00060A01060A02000A01000A03060A07060A05060A06060A08060A09060A0A060A03000A04060A0B060A0C060A0D060A0E060A0F060A10060A04000A11060A11060A12060A13060A14060A15060A16060A0500050800050B00050C00050D00050E00050A00050900050700050F00051200050600051000051100050600050600051400051500051600051700051800051900051A00070104070704070304071C00070604070204070004071B00070404070504070600070904070804070600070600071E00071F00070B04070A04072000072100072200040105040705040305042400040605040205040005042300040405040505040600040905040805040600040600042600042700040B05040A05042800042900042A00042C00042F00043000043100043200042E00042D00042B00043300043600040600043400043500040600040600043800043900043A00043B00043C00043D00043E000A40000A43000A44000A45000A46000A42000A41000A3F000A47000A53000A53000A48000A49000A4A000A53000A4C000A4D000A4E000A4F000A50000A51000A52000A53000A53000A53000A53000A53000A53000A53000A53000A55000A56000A57000A58000A59000A5A000A5B000A5C000A5D000A5E000A5F000A5F000A5F000A5F000A5F000A5F000461000462000400080A63000A64000401080A66000A67000A68000A69000A6A000A6B000A6C000A6D000A6E000A6F000A70000A71000A72000A72000A72000A72000473000474000402070A75000A76000A06070A77000A53000A78000A79000A53000A7A000A7B000A7C000A7D000A7E000A7F00048000048300048400049100049200049300049400049500049600049700049800049900049A000A9B000A9C000A9D000A9E000A9F0004A40004A50004A60004A00004A10007A20007A30004030704600007010707650004AA0004AD0005B00005B10005B20005B30005B50005B60007AB0005AC0006AE0006AF000FB7000FB8000FB90004BA000FBB000FBC000FBD0004BE0004BF0004C00004C10004C2000CC3001D16
"<<<" 68A0056305404116
">>>" 6829005600FC38C0FB3842981E104812010B41756720333120323032312031343A30363A3333507716FB3842981E104812010B41756720333120323032312031343A30363A333350383A32373A353745FB3842981E104812010B41756720333120323032312031343A30363A333350FFFF057A00000000000001000900566316
"<<<" 68A0056305393816
">>>" 682B000045FC30B90004000B000700140B313730315531040000000D65636F4D415839323050312D4B0F16
">>>" 680A005045FC300AB1166824000050FC388954494D45091D27E607041D050A480000000000025698FA3D1831EA16
"<<<" 68A045563053A7E16
">>>" 680F000045FC30BA04343939365216
">>>" 680A005945FC3040F216
">>>" 680A005145FC3040FA16

Thank you! This is exactly what I need to further investigate this issue.

I think I've now managed to completely fix string decoding and unrelated bug with EM920 regdata schema.
Could you please check again with latest commits.

If everything is working correctly I'll release pyplumio v0.1.10 and update hass integration.

On the unrelated note, from your data it seems that EM920 supports optical flame temperature sensor. I'll probably need to add this to hass integration or, even better, detect controller capabilities on setup.
2022-04-29 (1)
.

Yes - no more errors in python tests :) !
The device has flame sensor but it does not report flame temperature anywhere in the ui. But it has flame inensity sensor from 0-100 % which is an optcal sensor.

btw - I is it possible to add boiler temperature changing possibility ? Right now its only support turning boiler on and off. I suspect that you need frames from the ui module ? I was thinking to open the device fully and tap into ui module modbus...maybe I can get frames from there ?

btw - I is it possible to add boiler temperature changing possibility ? Right now its only support turning boiler on and off. I suspect that you need frames from the ui module ? I was thinking to open the device fully and tap into ui module modbus...maybe I can get frames from there ?

You already should be able to change boiler temperature using "[you device] Heating Temperature" slider.
If it doesn't work for you, than it's a bug and needs to be addressed.

Screenshot 2022-04-29 at 23-57-39 Overview – Home Assistant

Yes - no more errors in python tests :) ! The device has flame sensor but it does not report flame temperature anywhere in the ui. But it has flame inensity sensor from 0-100 % which is an optcal sensor.

Great! I'm releasing the update then.

I have in hass only a "Heating temperature" value that can take value of "0". Cant chage it to anyhing else. Also python report only parameter that I can chage is "boiler" from 0 to 1.

I have in hass only a "Heating temperature" value that can take vlaue of "0". Cant chage it to anyhing else.

OK. I'll look into this. It's possible that EM920 doesn't automatically issue Parameters response to PyPlumIO. Should be an easy fix. Although I probably do it after the weekend. In the meantime, I've released update to hass integration, so at least you getting correct numbers now and hopefully no errors :)

First of all. Thank you ! Hass logs are clear now. About the editable parameters I found that in my case the request to fetch editable parameters (type 0x31) was never sent to the device. Device itself didnt send responses either automatically (as you suggested). So I started looking at the code and actually didnt see any place where it was requested either (you probably assumed automatic frames). Next I made a quick fix in connection.py (_process function):

elif isinstance(frame, responses.UID):
device.uid = frame.data["UID"]
device.product = frame.data["reg_name"]
writer.queue(requests.Password(recipient=frame.sender))
added this -> writer.queue(requests.Parameters(recipient=frame.sender))

After that my python test became alive and I got wide range of editable parameters and was able to change them in the device as well (with python) :)
I dont know if this is the correct fix and in the right place.

You're welcome!

Yes, this is correct fix. As suspected, unlike EM350 and EM860, EM930 doesn't include Parameters request as part of versioning info in either RegData or CurrentData, so you've made a right call to request it manually.

Although I've done it a little bit differently, since I wanted to introduce rudimentary collision detection to the StreamWriter, to avoid queuing same request multiple times. This allows to queue all necessary requests when connecting to device and not depend so heavily on frame versions list, that seems to differ on different EM models. I look over it again in the evening and release one final update for EM930.

Thank you very much for your help with support for this device!

This is now solved as of version 1.1.11 by adding all required requests to the queue on connect/reconnect.