values mixup between hex and integer
Closed this issue · 9 comments
Since upgrading to 2023.5, I got various issues.
I could narrow it down to this change:
home-assistant/core@ab699d1
Somehow, it seems that data which is in EPC_CODE ends up being stored as hex string by pychonet:
https://github.com/scottyphillips/pychonet/blob/master/pychonet/EchonetInstance.py#L178
Should I fix it:
- on pychonet side by calling
int(xx.hex(), 16)orint.from_bytes()instead of just.hex()? (but maybe some stuff in EPC_CODE is not a proper integer? however I think those would have custom EPC_FUNCTIONS).
I tested this approach and it seems to work well. I am currently keeping this as a temporary workaround.
Note: I also had to removeint(, 16)inlight.brightness() - or should I change echonetlite extension to convert it (probably in sensor
native_value())? (doesn't seem easy as we might receive stuff that are not integers, such as "On".
The hex value is expected behavior for a echonet object that has not yet been implemented as a distinct device in pychonet, what type of echonet device are you trying to monitor? what is the EOJCI/EOJCC etc?
It's fine to keep it as hex (string) if intended.
But in that case, I believe we need to adjust a few other things so that this doesn't raise an exception:
https://github.com/home-assistant/core/blob/ab699d17a50acf14ca0183e7cf5a5ecbd4572a5f/homeassistant/components/sensor/__init__.py#L583
It seems that making sure device_class is NON_NUMERIC_DEVICE_CLASSES might be enough.
Can you please enable debug for the custom component and paste the log file here? what sort of device are you trying to monitor??
Let me give you more details.
What I am setting up is a Panasonic Light controller.
I simply want to write a value on 0xC0: Scene control setting
I added epc/definition in the plugin, then I was hoping to be able to rely on the generic pychonet.EchonetInstance => EchonetSensor code.
Pychonet changes
I added support in pychonet/lib/epc.py:
0xA3: { # Light Controller
0x80: "Operation status",
0xB0: "Illuminance level setting",
0xC0: "Scene control setting",
0xC1: "Number that can assign scene control setting",
},
echonetlite HA changes
On home assistant plugin side, I added:
0xA3: {
0xC0: { # Set scene
CONF_ICON: "mdi:palette",
CONF_TYPE: DEVICE_CLASS_ECHONETLITE_LIGHT_SCENE,
CONF_STATE_CLASS: SensorStateClass.MEASUREMENT,
CONF_SERVICE: [ SERVICE_SET_INT_1B ],
},
},
Runtime
Here's what happens at runtime since 2023.5:
I receive the following message
ECHONETLite Message Received from XX.XX.XX.XX - Raw data is b'\x10\x81\x00f\x02\xa3\x01\x05\xff\x01r\x03\x80\x010\xc0\x01\x11\xc1\x01T'
ECHONETLite Message Received from XX.XX.XX.XX - Processed data is {'EHD1': 16, 'EHD2': 129, 'TID': 102, 'SEOJGC': 2, 'SEOJCC': 163, 'SEOJCI': 1, 'DEOJGC': 5, 'DEOJCC': 255, 'DEOJCI': 1, 'ESV': 114, 'OPC': [{'EPC': 128, 'PDC': 1, 'EDT': b'0'}, {'EPC': 192, 'PDC': 1, 'EDT': b'\x11'}, {'EPC': 193, 'PDC': 1, 'EDT': b'T'}]}
ECHONETLite Message Received from XX.XX.XX.XX - tid_data is {}
ECHONETlite polling update data:
- Operation status (0x80): On
- Operation status (0x80): On
- Scene control setting (0xc0): 11
- Number that can assign scene control setting (0xc1): 54
This code will decode anything that doesn't have a EPC_FUNCTION as hex and store it as string in the sensor:
https://github.com/scottyphillips/pychonet/blob/d33129dafea63be6395ad23dc33a45a010494a39/pychonet/EchonetInstance.py#L178
Situation 1:
Scene control setting is 0x11 => value is stored as '11' (string) in the sensor and since 2023.5 HA interpret it as integer 11 (rather than 17). Before 2023.5 it was simply kept as '11' in the sensor.
Situation 2:
If I choose scene 13 (0x0D), string will be 0D and since 2023.5 sensor will throw an exception because it can't be converted and no custom type is defined. Before 2023.5 it was kept as '0D' in the sensor.
Note: this usually doesn't happen because values such as temperature are decoded using custom EPC_FUNCTIONS.
Also, for stuff such as brightness, it doesn't happen because it doesn't surface as a standalone HA sensor, only as an attribute on the light which is decoded manually in the light.py code using int(XX, 16) function.
To summarize:
- Prior 2023.5, (default) sensor values were kept as string, so it was working (but a bit tricky, i.e. if user try to compare as integer in script, it would be broken).
- Since 2023.5, the value is checked and converted to int, so situation 1 store invalid integer value and situation 2 simply crashes.
Code:
https://github.com/home-assistant/core/blob/ab699d17a50acf14ca0183e7cf5a5ecbd4572a5f/homeassistant/components/sensor/__init__.py#L583
So I was hoping to readd support for integer sensor so that they work out of the box (after adding definition).
So far I fixed it that way, and it works:
on pychonet side by calling int(xx.hex(), 16) or int.from_bytes() instead of just .hex()? (but maybe some stuff in EPC_CODE is not a proper integer? however I think those would have custom EPC_FUNCTIONS).
I tested this approach and it seems to work well. I am currently keeping this as a temporary workaround.
Note: I also had to remove int(, 16) in light.brightness()
If you agree with this approach, I can prepare PRs.
You are half-way there. But i dont want to change the generic class because i would have to fix too much stuff upstream. What is needed is a LightController.py class in 'pychonet' which will transform the value for you. Look at some of the other classes for a basic idea.
The generic Echonetlite object will always return a stringified hex value which is the raw value that is returned from the polling.
In __init__.py of pychonet it gives you the basic framework for creating child classes. You will add the import statement and update the Factory method. Its the Factory which is called in ENL_homeassistant, so by creating a seperate class you can override the default values.
Take a look at GasMeter below for an example. You define the class specific EPC functions and these get mapped to EPC_FUNCTIONS. Ideally, you refer to the ENL spec document which will explain how the EPC value is meant to be unpacked.
from pychonet.EchonetInstance import EchonetInstance
from pychonet.lib.epc_functions import _int, _signed_int
import struct
# 0xE2: "Cumulative amount of gas consumption measurement log"
def _0282E2(edt):
# return array x 48 unsigned long big-endian
return [x[0] for x in struct.iter_unpack('>L',edt)]
class GasMeter(EchonetInstance):
EPC_FUNCTIONS = {
0xE0: _int, # 0xE0: "Cumulative amount of gas consumption measurement value"
0xE2: _0282E2 #_0282E2
}
def __init__(self, host, api_connector=None, instance=0x1):
self._eojgc = 0x02
self._eojcc = 0x82
EchonetInstance.__init__(
self, host, self._eojgc, self._eojcc, instance, api_connector
)
The class is stitched back to _init__.py into the Factory function:
from .GasMeter import GasMeter
...
def Factory(host, server, eojgc, eojcc, eojci=0x01):
instance = None
if eojgc in EOJX_CLASS:
if eojcc in EOJX_CLASS[eojgc]:
instance = f"{eojgc}-{eojcc}"
"""Factory Method"""
instances = {
...
f"{0x02}-{0x82}": GasMeter,
...
None: None,
}
Looks like you could get away with just mapping them all to _int
So here is how LightController would look:
from pychonet.EchonetInstance import EchonetInstance
from pychonet.lib.epc_functions import _int, _signed_int
import struct
class LightController(EchonetInstance):
EPC_FUNCTIONS = {
0xB0: _int, # Illuminance level setting
0xC0: _int, # Scene control setting
0xC1: _int, # Number that can assign scene control setting
}
def __init__(self, host, api_connector=None, instance=0x1):
self._eojgc = 0x02
self._eojcc = 0xA3
EchonetInstance.__init__(
self, host, self._eojgc, self._eojcc, instance, api_connector
)
To be honest it may make more sense to convert the hex to an int because it is quite common, but ill have to figure that one out. For now a class is the way to go. I can probably have this fixed up tomorrow.
Understood!
I was initially hoping I could just use the generic passthrough code to avoid having to create such a class
But if you think it's too much work to rearrange the generic code to work well with integer by default, I don't mind creating the device. I did for SingleFunctionLighting (0x91), which I will PR soon already, so I can take care of it.
FYI, it worked with my workaround that only changed 2 lines in pychonet (add int(XX, 16)) and 1 line in light.py (to properly convert brightness which was assumed to be string).
It might have side effect on some device type I don't have, but I have AC, lighting, solar panel and battery already being tested with this setup.
You sure it's not worth pursuing for easier setup of new device type?
I think I went with hex values because they were easy to work with. I prefer to have each class mapped out for now. I think it might break too many other peoples setups if we are not careful. I think there is already a battery class? Dont think there is a solar panel class, be great if one was added?
😉
Closing in favor of scottyphillips/pychonet#60