home-assistant / core

:house_with_garden: Open source home automation that puts local control and privacy first.
https://www.home-assistant.io
Apache License 2.0
73.81k stars 30.9k forks source link

elkm1 unable to serialize to JSON for climate entities #72981

Closed gormic75 closed 2 years ago

gormic75 commented 2 years ago

The problem

After the upgrade to 2022.6.0 by climate entities are no longer working:

The following error for both of my climate areas:

140477941995216] Unable to serialize to JSON. Bad data found at $.event.a.climate.lower_floor.a.mode=ThermostatMode.HEAT(<enum ‘ThermostatMode’>, $.event.a.climate.lower_floor.a.fan=ThermostatFan.AUTO(<enum ‘ThermostatFan’>, $.event.a.climate.upper_floor.a.mode=ThermostatMode.HEAT(<enum ‘ThermostatMode’>, $.event.a.climate.upper_floor.a.fan=ThermostatFan.AUTO(<enum ‘ThermostatFan’>

And warning in the log as follow:

'* State is not JSON serializable: <state climate.upper_floor=heat; hvac_modes=[<HVACMode.OFF: ‘off’>, <HVACMode.HEAT: ‘heat’>, <HVACMode.COOL: ‘cool’>, <HVACMode.HEAT_COOL: ‘heat_cool’>, <HVACMode.FAN_ONLY: ‘fan_only’>], min_temp=1, max_temp=99, target_temp_step=1, fan_modes=[‘auto’, ‘on’], current_temperature=74, target_temp_high=76, target_temp_low=70, current_humidity=0, fan_mode=on, aux_heat=off, name=Upper floor, mode=ThermostatMode.HEAT, hold=False, fan=ThermostatFan.ON, current_temp=74, heat_setpoint=70, cool_setpoint=76, humidity=0, index=2, friendly_name=Upper floor, supported_features=74 @ 2022-06-01T22:24:29.920081-07:00>: Object of type ThermostatMode is not JSON serializable

What version of Home Assistant Core has the issue?

2022.6.0

What was the last working version of Home Assistant Core?

2022.5.5

What type of installation are you running?

Home Assistant Core

Integration causing the issue

elkm1

Link to integration documentation on our website

https://www.home-assistant.io/integrations/elkm1

Diagnostics information

No response

Example YAML snippet

No response

Anything in the logs that might be useful for us?

No response

Additional information

No response

probot-home-assistant[bot] commented 2 years ago

elkm1 documentation elkm1 source (message by IssueLinks)

probot-home-assistant[bot] commented 2 years ago

Hey there @gwww, @bdraco, mind taking a look at this issue as it has been labeled with an integration (elkm1) you are listed as a code owner for? Thanks! (message by CodeOwnersMention)

gwww commented 2 years ago

Could you turn on debug and provide the log. I’ll take a look at the code in the meantime.

Logging instructions here: https://community.home-assistant.io/t/elk-m1-interface/4461/983

gwww commented 2 years ago

@bdraco I looked at the change and don't see anything obvious. This is related to the switch from .values to enums. Seems related to logbook somehow.

The debug log will certainly help.

FYI After this morning I'm away from my computer until Sunday evening to look further.

gormic75 commented 2 years ago

This is the debug log of the elkm1 entries:

elkm1_filtered.log

gormic75 commented 2 years ago

Less filtering to include the actual error.

elkm1_filtered.log

gwww commented 2 years ago

Thx for the logs. I don't see the problem in the code by just looking. I don't have time today to setup a dev environment to reproduce this. It will be Sunday/Monday before I can take a deeper look.

I'm not sure where the recorder is picking up ElkM1 library consts instead of what I believe should be HA consts.

bdraco commented 2 years ago

Thx for the logs. I don't see the problem in the code by just looking. I don't have time today to setup a dev environment to reproduce this. It will be Sunday/Monday before I can take a deeper look.

I'm not sure where the recorder is picking up ElkM1 library consts instead of what I believe should be HA consts.

The json serializer needs actual str objects as it doesn't know how to serialize enums. Converting them to strings in the asdict function will fix it

gwww commented 2 years ago

I’m not getting it. Where’s the asdict needed?

bdraco commented 2 years ago

I sent you a PM on discord

lmamakos commented 2 years ago

I've noticed this same issue; if additional logs are helpful, let me know. I also have a thermostat device connected to my Elk alarm system. I have a non-production Home Assistant instance running that I can experiment with if that'd be useful.

mstarks01 commented 2 years ago

Just a "me too." I reverted back to 5.6, but am happy to upgrade once again and help if I can.

bdraco commented 2 years ago

We need to stringify the enums here https://github.com/gwww/elkm1/blob/main/elkm1_lib/elements.py#L92 since the json serializer can't convert the Enum type to a string on its own

gwww commented 2 years ago

I’ll submit the fix tomorrow or Monday. Not at my computer right now.

lmamakos commented 2 years ago

FWIW. On my test Home Assistant instance, I just smashed this change in and the smoke-test passes and climate entity shows up. I don't know if converting both the key and value to strings are necessary..

bash-5.1# diff -u elements.py.orig elements.py
--- elements.py.orig
+++ elements.py
@@ -89,7 +89,7 @@
     def as_dict(self) -> dict[str, Any]:
         """Package up the public attributes as a dict."""
         attrs = vars(self)
-        return {key: attrs[key] for key in attrs if not key.startswith("_")}
+        return {str(key): str(attrs[key]) for key in attrs if not key.startswith("_")}

 T = TypeVar("T", bound=Element)

I've not tried any more extensive testing than restarting Home Assistant and seeing the entity appear and the previous two errors related not show in the log. @gwww, have a great weekend!

mstarks01 commented 2 years ago

Thank you @bdraco and @gwww. We appreciate you!