power on from remote results in HA error

[Flopescu]

Home Assistant version

SmartIR version "domain": "smartir", "documentation": "https://github.com/litinoveweedle/SmartIR", "version": "1.17.14"

SmartIR configuration

  - platform: smartir
    name: LivingRoom AC
    unique_id: livingroom_ac
    device_code: 3120
    controller_data: remote.broadlink_rm4
    power_sensor: binary_sensor.livingroom_ac_state

Describe the bug

When the attached power sensor turns on (from off) the HA throws a temperature unknown error. This happens when I start the AC via the dumb remote control --> the binary sensor detects that the AC is now on.

Turning off the AC from the same dumb remote will correctly turn off the HA climate entity.

Debug log

Logger: homeassistant
Source: helpers/temperature.py:25
First occurred: 16:45:59 (12 occurrences)
Last logged: 17:03:30

Error doing job: Task exception was never retrieved
Traceback (most recent call last):
  File "/config/custom_components/smartir/climate.py", line 548, in _async_power_sensor_changed
    await self._async_update_hvac_action()
  File "/config/custom_components/smartir/climate.py", line 558, in _async_update_hvac_action
    self.async_write_ha_state()
  File "/usr/src/homeassistant/homeassistant/helpers/entity.py", line 1009, in async_write_ha_state
    self._async_write_ha_state()
  File "/usr/src/homeassistant/homeassistant/helpers/entity.py", line 1132, in _async_write_ha_state
    state, attr, capabilities, shadowed_attr = self.__async_calculate_state()
                                               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/src/homeassistant/homeassistant/helpers/entity.py", line 1069, in __async_calculate_state
    if state_attributes := self.state_attributes:
                           ^^^^^^^^^^^^^^^^^^^^^
  File "/usr/src/homeassistant/homeassistant/components/climate/__init__.py", line 324, in __getattribute__
    return super().__getattribute__(__name)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/src/homeassistant/homeassistant/components/climate/__init__.py", line 520, in state_attributes
    data[ATTR_TEMPERATURE] = show_temp(
                             ^^^^^^^^^^
  File "/usr/src/homeassistant/homeassistant/helpers/temperature.py", line 25, in display_temp
    raise TypeError(f"Temperature is not a number: {temperature}")
TypeError: Temperature is not a number: unknown

Additional context

I have no idea if this was working prior to this version as I just installed and configure it. Thank you for your work!

[Flopescu]

Additional info: These are the climate entity properties in HA:

hvac_modes: off, cool, heat, auto
min_temp: 17
max_temp: 30
target_temp_step: 1
fan_modes: low, med, high, auto
current_temperature: null
temperature: 23
fan_mode: auto
last_on_operation: auto
device_code: 3120
manufacturer: White-Westinghouse
supported_models: R51/BGE
supported_controller: Broadlink
commands_encoding: Base64
friendly_name: LivingRoom AC
supported_features: 393

[litinoveweedle]

Hello, thank you for logging, I can confirm this issue. I already created patch, but as I am currently not at home I am not able to test it. If I will prepare beta release are you able to test it?

[Flopescu]

sure, I can test it. let me know

[litinoveweedle]

ready to test as 1.17.15b1, thank you.

[Flopescu]

I installed today and did a few tests -> there are no errors or warnings in the log. But when the sensor detects the device is on the integration marks the state as "unknown". At OFF it switches it correctly switch it to off. Here's the extract from logbook:

LivingRoom AC  turned off
09:47:35 - 4 minutes ago

LivingRoom AC State  cleared (no power detected)
09:47:35 - 5 minutes ago

LivingRoom AC  became unknown
09:47:24 - 5 minutes ago

LivingRoom AC State  detected power
09:47:24 - 5 minutes ago

I understand that the hvac_mode in unknown (we just presses the remote), but I think it would be better to just set it as ON / AUTO with the last temp setting (maybe have a default set, probably auto with a mid value for degrees). This way we know it's on and we can adjust(sync) from HA if the default is not suitable for the day.

In the attributes list we have this details :

temperature: 23
fan_mode: auto
last_on_operation: auto

[litinoveweedle]

Thanks for testing.

I understand that the hvac_mode in unknown (we just presses the remote), but I think it would be better to just set it as ON / AUTO with the last temp setting (maybe have a default set, probably auto with a mid value for degrees). This way we know it's on and we can adjust(sync) from HA if the default is not suitable for the day.

This shall be very close to the functionality, which can be enabled by setting "power_sensor_restore_state" in the HA yaml configuration of the given entity to true:

climate:
  - platform: smartir
    name: Bedroom AC
    unique_id: bedroom_ac
    power_sensor_restore_state: true
....

in this case last state of the device is restored, although with set temperature and modes unknown.... This helps to identify that device is not set to the given temperature - and could/should be adjusted from within HA. Some climate devices use auto mode, so I don't like idea to use it for this purpose. Please test it and let me know if this suits your needs.

[Flopescu]

perfect! missed this setting in the documentation... changed it and everything is as it should! THANK YOU!

[litinoveweedle]

You are welcome. ;-) I need to do same power sensor handling correction for fan and media_player classes as well. I expect this to be finished today and then I release new version. Please try to test integration little bit more if possible and report any other potential bug. Thank you for help.

[Flopescu]

If you want a quick and easy option use a zigbee/RF door sensor (take the magnet out and stick it on the AC flap). ...

[litinoveweedle]

If you want a quick and easy option use a zigbee/RF door sensor (take the magnet out and stick it on the AC flap).

That is good suggestion I tend to add it into docs.

Just to check, (as I currently still can't by myself), if device is set to ON by remote, HA will display last temperature, correct? This is not yet intended target state. I would propose to show 'uknown' temperature, just to indicate it is not being managed by HA.

[Flopescu]

I did a few tests. There are no change in the attributes which means that the hvac_mode and temperature are preserved from OFf to ON. When OFf, the climate entity shows (and allows) to change the last known temperature. When switched ON (via remote) the only change I see is the state OFF->Auto, as Auto was the "last_on_operation"

[litinoveweedle]

There are no change in the attributes which means that the hvac_mode and temperature are preserved from OFf to ON.

This is intended functionality, to preserve latest temperature, mode, fan mode over ON/OFF cycle. (including one executed by IR remote)

When OFf, the climate entity shows (and allows) to change the last known temperature.

This is again indented, as even if you change temperature, or fan mode when device is OFF, when switched ON by setting any hvac mode in HA it will also set changed temperature and/or fan mode.

When switched ON (via remote) the only change I see is the state OFF->Auto, as Auto was the "last_on_operation"

This is actually not fully intended as I originally wanted to report/display both temperature, fan mode, etc. in HA as unknown when switched device is ON by IR remote - simply to indicate, that device is not being managed by HA. But I am open to discuss such behavior, as it is very close to the one with power_sensor_restore_state: false. What do you think?

[Flopescu]

I guess adding another attribute to the climate entity would be so much easier? OR force another state, other than Auto, Heat, Cold, Fan_Only etc like Remote_ON ? - this can be added to the json file..

But then again, it might not matter that much. One could set an automation to verify what turned on the AC (like sensor state changed first or if the HA actually sent any command)

In my case, I am more interested to know the state (on, off) [if someone turned the AC on by the ir remote OR to make sure that the AC received the HA command to turn on]

Once the state in known in HA, I can see it and take action if needed ( like when summer and someone started the AC on very cold, while the HA reports last state = heat ).... really not a problem.

Also, same as turning ON, it's important to have the state OFF set correctly when I turn it off from HA and from some reason the AC did not receive the command and remained ON.

[litinoveweedle]

I guess adding another attribute to the climate entity would be so much easier? OR force another state, other than Auto, Heat, Cold, Fan_Only etc like Remote_ON ? - this can be added to the json file..

Attribute can be added, but not a state. Climate entity states have to be subset of the hvac_mode list, which is hardcoded in HA. The question is what would do you want to be reported in this new attribute...

But then again, it might not matter that much. One could set an automation to verify what turned on the AC (like sensor state changed first or if the HA actually sent any command)

In my case, I am more interested to know the state (on, off) [if someone turned the AC on by the ir remote OR to make sure that the AC received the HA command to turn on]

This would be still achieved, as the state will reflect it by reporting UNKNOWN if AC was switched on by the IR remote control.

Once the state in known in HA, I can see it and take action if needed ( like when summer and someone started the AC on very cold, while the HA reports last state = heat ).... really not a problem.

Therefore I see as better solution to report state 'UNKNOWN' as HA knows, that someone started AC with IR remote and you can trigger automation based on that. (for example to force set AC temperature).

Also, same as turning ON, it's important to have the state OFF set correctly when I turn it off from HA and from some reason the AC did not receive the command and remained ON.

That will work always, disregarding which control (HA or IR remote) was used to switch AC off, as there is only one 'OFF' state, so no extra logic to distinguished modes is required - HA will always report state as OFF.

[Flopescu]

I personally would avoid having any UNKNOWN state - it can be easily mistaken with the entity not being available. HA will have the climate entity in this state if the integration is not working properly.

I guess adding another attribute to the climate entity would be so much easier? OR force another state, other than Auto, Heat, Cold, Fan_Only etc like Remote_ON ? - this can be added to the json file.. Attribute can be added, but not a state. Climate entity states have to be subset of the hvac_mode list, which is hardcoded in HA. The question is what would do you want to be reported in this new attribute...

^^ this was a suggestion on letting know the AC was turned on via the ir remote

[litinoveweedle]

I personally would avoid having any UNKNOWN state - it can be easily mistaken with the entity not being available. HA will have the climate entity in this state if the integration is not working properly.

yep, but the way SmartIR is implemented you will never get UNKNOWN state in any other case. You may get UNAVAILABLE when integration fail to load, but never UNKNOWN.

I guess adding another attribute to the climate entity would be so much easier? OR force another state, other than Auto, Heat, Cold, Fan_Only etc like Remote_ON ? - this can be added to the json file.. Attribute can be added, but not a state. Climate entity states have to be subset of the hvac_mode list, which is hardcoded in HA. The question is what would do you want to be reported in this new attribute...

^^ this was a suggestion on letting know the AC was turned on via the ir remote

Sorry, did not get this at first. There is already internal attribute on_by_remote implemented, but it was not published into HA. I will add it in next beta, thank for idea.

I have another topic. One of the users reported in the HA discussion issue, that in case if managed device fail to react to IR command from SmartIR the state is not corrected by the state of the power sensors i.e.:

1. Device is off
2. ON command is send from HA via SmartIR to device, but device fail to receive it.
3. Device is reported in the HA as state ON even if power sensors is showing OFF.

I can confirm, that this is an issue, as this use case was not implemented. I did add it just now, would you be please willing to test it? I will release new beta containing the fix soon. Thank you in any case for your help so far.

[litinoveweedle]

The issue is now fixed in the latest beta 1.17.15b2 which is ready for the testing. If no issues are discovered it will be released in the public release in few days. Thank you for help!

[gurglingtonic]

@litinoveweedle

I have another topic. One of the users reported in the HA discussion issue, that in case if managed device fail to react to IR command from SmartIR the state is not corrected by the state of the power sensors i.e.:

Device is off ON command is send from HA via SmartIR to device, but device fail to receive it. Device is reported in the HA as state ON even if power sensors is showing OFF. I can confirm, that this is an issue, as this use case was not implemented. I did add it just now, would you be please willing to test it? I will release new beta containing the fix soon. Thank you in any case for your help so far.

I tested the latest beta version. Here are my findings:-

Scenario A I turned on my AC through HA while my power sensor was off. A few seconds later, SmartIR reverted the AC to off. Nice job.

Scenario B I accidentally turned on my AC through HA, then quickly turned it off within a split second while my power sensor was still off. A few seconds later, SmartIR reverted the AC to on. Not so nice.

[litinoveweedle]

Hello, thank you very much for testing.

I tested the latest beta version. Here are my findings:-

Scenario A I turned on my AC through HA while my power sensor was off. A few seconds later, SmartIR reverted the AC to off. Nice job.

Great, this is supposed behavior. ;-)

Scenario B I accidentally turned on my AC through HA, then quickly turned it off within a split second while my power sensor was still off. A few seconds later, SmartIR reverted the AC to on. Not so nice.

That is strange, this should not happen. When first call for state change occurs (on/off) then call to revert to the previous state in the future (configurable time, default 10s) is scheduled. This future revert call is canceled by either:

• power sensors report change to correct target state
• another state change (on/off) is created, cancelling previous revert call and creating a new one.

so the described issue should not happen, as your scheduled revert to on should be replaced by the revert to off...

The only probable cause I can think of for now, is some race condition on the slower HA hardware. I will try to improve locking and the state machine implementation.

[gurglingtonic]

I ran your code in ChatGPT and it came back with this:-

It seems like you're trying to schedule a future revert call to revert to the previous state after a configurable time when a state change occurs (on/off), and then cancel that revert call under certain conditions. The code structure and logic seem generally correct, but I noticed a couple of issues and potential improvements:

1. Typo in Method Name: In the method _async_power_sensor_check_schedulle, "schedulle" is misspelled. It should be "schedule".

2. Cancellation of Previous Power Sensor Check: The cancellation of the previous power sensor check seems properly implemented.

3. Power Sensor Check Execution: The method _async_power_sensor_check is supposed to execute when the power sensor check timer elapses. However, it seems like there's a missing line to execute the check, i.e., calling _async_power_sensor_check function inside the async_call_later callback.

4. Power Sensor Check Expectation: The line self._power_sensor_check_expect = state sets the expectation for the power sensor check, but it doesn't seem to consider the case where the current state already matches the expected state. It might be beneficial to check if the current state matches the expected state and avoid scheduling a redundant check in such cases.

So, the _async_power_sensor_check_schedulle method should look like this after the modification:

@callback
def _async_power_sensor_check_schedulle(self, state):
    if self._power_sensor_check_cancel:
        self._power_sensor_check_cancel()
        self._power_sensor_check_cancel = None

    async def _async_power_sensor_check(*_):
        self._power_sensor_check_cancel = None
        current_state = self.hass.states.get(self._power_sensor)
        if current_state and current_state.state == self._power_sensor_check_expect:
            return
        if self._power_sensor_check_expect == STATE_OFF:
            self._state = STATE_ON
        else:
            self._state = STATE_OFF
        self._power_sensor_check_expect = None
        self.async_write_ha_state()

    self._power_sensor_check_expect = state
    self._power_sensor_check_cancel = async_call_later(
        self.hass, self._power_sensor_delay, _async_power_sensor_check
    )

I tried the solution provided and it solved Scenario B.

[litinoveweedle]

I hate being beaten by AI. :-)

You are actually correct, I did found the same issue meanwhile, together with one more edge case. The problem was, that power sensor run check cancellation was not triggered if the power sensors did not changed the state, which was not happening because of the quick there and back change. Actually with the current state of the sensor being directly polled during power sensors check, it is IMHO not need to cancel it when power sensor change state. It simplifies the logic (less place for potential race condition) and seems to be more robust, as the power sensors are sketchy at best, having delays etc. So it is better to always check them after some time to see the converged state.

The other issue was small race condition where two coroutines could run in independent threads causing potential race conditions.

Thanks for help, I will release beta 3 today, your testing will be as always very welcomed. ;-)

[litinoveweedle]

[beta 1.17.15b3] (https://github.com/litinoveweedle/SmartIR/releases/tag/1.17.15b3) is released.

[gurglingtonic]

@litinoveweedle

After updating to beta 1.17.15b3, Scenario A no longer working :(

Am I missing some settings?

[litinoveweedle]

Scenario A I turned on my AC through HA while my power sensor was off. A few seconds later, SmartIR reverted the AC to off. Nice job.

Scenario A I turned on my AC through HA while my power sensor was off. A few seconds later, SmartIR reverted the AC to off.

Just to check, what is the state of your power sensor? I am afraid it is UKNOWN or UNAVAILABLE... The functionality now works only if the power sensors reports either on or off.

[gurglingtonic]

I'm using aqara door sensor on my AC. State is either on or off.

[litinoveweedle]

could you please set on debug logging for climate? There are few new debug log messages placed (as you very well know now ;-) in the given functionality, so we shall see whats going on.

This should do it:

logger:
  default: warning
  logs:
     custom_components.smartir.climate: debug

[litinoveweedle]

ok, I did that and I see the problem:

2024-06-10 17:30:04.665 DEBUG (MainThread) [custom_components.smartir.climate] Executing power sensor check for expected state 'off', current state '<state binary_sensor.ac_ison=unknown; friendly_name=ac_ison @ 2024-06-10T17:29:56.979268+02:00>'

[litinoveweedle]

So the AI was wrong at the end ;-)

current_state = self.hass.states.get(self._power_sensor)

vs

current_state = self.hass.states.get(self._power_sensor).state

if you need to compare states and not state objects. I will fix it ASAP.

[gurglingtonic]

So the AI was wrong at the end ;-)

I sense your contentment in outwitting AI. Hahaha. Adding ".state" solved. Kudos!

[litinoveweedle]

Yep, you have to beat it before it will replace us. 🤣 Anyway fixed in that latest beta. Thank you for testing!

[gurglingtonic]

Tested the latest beta for climate and fan. All good on my side.

[litinoveweedle]

Thank you for testing! I am still getting some feedback for the media_player class. Once this is done (hopefully today/tomorrow) I think we can release it.

[litinoveweedle]

Hello, thanks to all for help and support, this issue is now fixed in the release 1.17.15. I am closing this issue now.