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
71.03k stars 29.68k forks source link

Homekit Device Integration issues with MEL-AC-Homekit (mongoose-os) #100183

Closed sindudas closed 11 months ago

sindudas commented 11 months ago

The problem

I have 4 Mitsubishi air conditioners, and I have installed 4 esp32 flashed with mongoose-os app Mel-AC-HomeKit. They work directly with HomeKit, they can be controlled from there. But when I connect directly to HomeAssistant I can't control it. Only can put On "fan-mode" or "dry-mode". Can see the HVAC state, but can't change it because always returns that error log:

set_hvac_mode.[<class 'decimal.DivisionByZero'>]

Aditionally, when AC is in "cooling" mode, I can change Target Temperature only.

What version of Home Assistant Core has the issue?

core-2023.9.1

What was the last working version of Home Assistant Core?

No response

What type of installation are you running?

Home Assistant Supervised

Integration causing the issue

HomeKit Device

Link to integration documentation on our website

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

Diagnostics information

homekit_controller-92bd86201cce421db82fb372b04fb857-MEL-FEB0-b6b994b665d3ba4255d52602bec339d8.json.txt

Example YAML snippet

No response

Anything in the logs that might be useful for us?

Sep 12 12:03:22 apu2 homeassistant[505]: #033[31m2023-09-12 12:03:22.861 ERROR (MainThread) [homeassistant.components.websocket_api.http.connection] [140480592828992] [<class 'decimal.DivisionUndefined'>]
Sep 12 12:03:22 apu2 homeassistant[505]: Traceback (most recent call last):
Sep 12 12:03:22 apu2 homeassistant[505]:   File "/usr/src/homeassistant/homeassistant/components/websocket_api/commands.py", line 227, in handle_call_service
Sep 12 12:03:22 apu2 homeassistant[505]:     await hass.services.async_call(
Sep 12 12:03:22 apu2 homeassistant[505]:   File "/usr/src/homeassistant/homeassistant/core.py", line 1969, in async_call
Sep 12 12:03:22 apu2 homeassistant[505]:     response_data = await coro
Sep 12 12:03:22 apu2 homeassistant[505]:                     ^^^^^^^^^^
Sep 12 12:03:22 apu2 homeassistant[505]:   File "/usr/src/homeassistant/homeassistant/core.py", line 2006, in _execute_service
Sep 12 12:03:22 apu2 homeassistant[505]:     return await target(service_call)
Sep 12 12:03:22 apu2 homeassistant[505]:            ^^^^^^^^^^^^^^^^^^^^^^^^^^
Sep 12 12:03:22 apu2 homeassistant[505]:   File "/usr/src/homeassistant/homeassistant/helpers/entity_component.py", line 235, in handle_service
Sep 12 12:03:22 apu2 homeassistant[505]:     return await service.entity_service_call(
Sep 12 12:03:22 apu2 homeassistant[505]:            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Sep 12 12:03:22 apu2 homeassistant[505]:   File "/usr/src/homeassistant/homeassistant/helpers/service.py", line 870, in entity_service_call
Sep 12 12:03:22 apu2 homeassistant[505]:     response_data = await _handle_entity_call(
Sep 12 12:03:22 apu2 homeassistant[505]:                     ^^^^^^^^^^^^^^^^^^^^^^^^^^
Sep 12 12:03:22 apu2 homeassistant[505]:   File "/usr/src/homeassistant/homeassistant/helpers/service.py", line 942, in _handle_entity_call
Sep 12 12:03:22 apu2 homeassistant[505]:     result = await task
Sep 12 12:03:22 apu2 homeassistant[505]:              ^^^^^^^^^^
Sep 12 12:03:22 apu2 homeassistant[505]:   File "/usr/src/homeassistant/homeassistant/components/homekit_controller/climate.py", line 513, in async_set_hvac_mode
Sep 12 12:03:22 apu2 homeassistant[505]:     await self.async_put_characteristics(
Sep 12 12:03:22 apu2 homeassistant[505]:   File "/usr/src/homeassistant/homeassistant/components/homekit_controller/entity.py", line 90, in async_put_characteristics
Sep 12 12:03:22 apu2 homeassistant[505]:     payload = self.service.build_update(characteristics)
Sep 12 12:03:22 apu2 homeassistant[505]:               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Sep 12 12:03:22 apu2 homeassistant[505]:   File "/usr/local/lib/python3.11/site-packages/aiohomekit/model/services/service.py", line 133, in build_update
Sep 12 12:03:22 apu2 homeassistant[505]:     value = check_convert_value(value, char)
Sep 12 12:03:22 apu2 homeassistant[505]:             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Sep 12 12:03:22 apu2 homeassistant[505]:   File "/usr/local/lib/python3.11/site-packages/aiohomekit/model/characteristics/characteristic.py", line 378, in check_convert_value
Sep 12 12:03:22 apu2 homeassistant[505]:     ((val - offset) / min_step).to_integral_value() * min_step
Sep 12 12:03:22 apu2 homeassistant[505]:      ~~~~~~~~~~~~~^~~~~~~~~~~~
Sep 12 12:03:22 apu2 homeassistant[505]: decimal.InvalidOperation: [<class 'decimal.DivisionUndefined'>]#033[0m

Additional information

No response

home-assistant[bot] commented 11 months ago

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

Code owner commands Code owners of `homekit_controller` can trigger bot actions by commenting: - `@home-assistant close` Closes the issue. - `@home-assistant rename Awesome new title` Renames the issue. - `@home-assistant reopen` Reopen the issue. - `@home-assistant unassign homekit_controller` Removes the current integration label and assignees on the issue, add the integration domain after the command.

(message by CodeOwnersMention)


homekit_controller documentation homekit_controller source (message by IssueLinks)

Jc2k commented 11 months ago

Unfortunately lots of devices don't follow the spec, and the official client is very tolerant of bad data. In the past we've had devices with JSON parsers that can't handle true and JSON serializers that output too many commas!

Unfortunately the code you are hitting is a workaround for a Honeywell climate device specific behaviour. This device insists that any number you send respects the minStep field. For example, if minStep is 0.5 and you send a temperature of 24.3, it won't round it for you. Instead, that is invalid for a honeywell device. We must round to 24.5 or 24.0 for it.

This clamping applies to any number field that has a minStep. For your device, the metadata is:

              {
                "type": "00000033-0000-1000-8000-0026BB765291",
                "iid": 311,
                "perms": [
                  "pr",
                  "pw",
                  "ev"
                ],
                "format": "uint8",
                "value": 0,
                "description": "Target Heating Cooling State",
                "minValue": 0,
                "maxValue": 3,
                "minStep": 0
              },

The minStep for your device here is 0. This is kind of an invalid value the mongoose firmware is giving us, our code is saying that the gap between 0 and 1 must be 0, and so that causes a division error in the code that "clamps" the precision.

I think the work around for this device specific behaviour (bug?) is that we should just ignore a minStep of 0 as an invalid value and not do the clamping.

sindudas commented 11 months ago

@Jc2k thanks for letting me see among all this debug the important information about the problem. Now I have managed to correct that problem at the source mel-ac-homekit and it now works for me (now will try to make a PR to submit on that project).

But I see the modification in HA as logical and necessary to avoid this type of errors when the devices do not report all the necessary information, a step of 1 is better than a division by zero

Thanks

Jc2k commented 11 months ago

Indeed. While we generally are less concerned about uncertified devices with home brew firmware, when a fix is easy and iOS can handle the device we do try to match what iOS does (where we can figure it out).

In this case we can be pretty sure iOS is just ignoring nonsensical minStep values. So we will too. As you can see in the links above your last comment the problem is fixed in aiohomekit. The bump in ha to use it isn't far behind.