HomeKit Controller Climate Integration Cannot Set Temp

[stshontikidis]

The problem

Added Honeywell T6 Pro to HA with HomeKit Controller integration, states coming through correctly but service call climate.set_temp appears to not work. I am able to succesfully climate.turn_on so I am not fully locked out. Seems to be an issue with the temperature value being sent, possibly too many digits. No official errors are raised, just fails silently, but can see a status code in response from unit in debug logs.

Environment

• Home Assistant Core release with the issue: 0.111.4
• Last working Home Assistant Core release (if known): N/A
• Operating environment (OS/Container/Supervised/Core): OS
• Integration causing this issue: HomeKit Controller or base Climate
• Link to integration documentation on our website: https://www.home-assistant.io/integrations/homekit_controller/

Problem-relevant configuration.yaml

Traceback/Error logs

2020-06-24 21:59:18 DEBUG (MainThread) [aiohomekit.controller.ip.connection] REDACTED: raw request: b'PUT /characteristics HTTP/1.1\r\nHost: REDACTED\r\nContent-Type: application/hap+json\r\nContent-Length: 66\r\n\r\n{"characteristics":[{"aid":1,"iid":13,"value":22.22222222222222}]}'
2020-06-24 21:59:18 DEBUG (MainThread) [aiohomekit.controller.ip.connection] REDACTED: raw response: bytearray(b'{"characteristics": [ {"aid":1,"iid":13,"status":-70410}]}')

Additional information

I believe the issue is around the precision of the value. I googled the status code returned and found this https://github.com/apple/HomeKitADK/blob/master/HAP/HAPIPAccessoryServer.c#L59 which pointed me in that direction of something in the payload being off. I picked a Fahrenheit value that converts to Celcius, 77->25.0, and that worked so we probably just need to round a bit of the value. My problem is I don't know if this is something that should be rounded across the board, would love to help but have not worked in this code base before so would need a bit of help.

2020-06-24 21:59:47 DEBUG (MainThread) [aiohomekit.controller.ip.connection] REDACTED: raw request: b'PUT /characteristics HTTP/1.1\r\nHost: REDACTED\r\nContent-Type: application/hap+json\r\nContent-Length: 53\r\n\r\n{"characteristics":[{"aid":1,"iid":13,"value":25.0}]}'
2020-06-24 21:59:47 DEBUG (MainThread) [aiohomekit.controller.ip.connection] REDACTED: raw response: bytearray(b'')

Note that an empty response seems to be good, got empty response when turning on/off when that worked. Also wrote in the Lyric thread https://community.home-assistant.io/t/honeywell-lyric-thermostat/3520/857?u=stshontikidis

[stshontikidis]

@Jc2k tagging since you are the code owner in the manifest.

[probot-home-assistant[bot]]

homekit_controller documentation homekit_controller source _{^{(message by IssueLinks)}}

[Jc2k]

Brilliant. Thanks for the debug logs. We've had a spate of these lately. One of the thermostat brands can't handle spaces between values in JSON ({"foo":"bar"} vs {"foo": "bar"}) and another will take 1, 0, false but not true.

You are correct, empty is a valid response. I think further under the hood there is actually a HTTP Accepted response with no body. The debug logs currently print the body because of a fight I was loosing with one device that was returning HTML instead of a HAP error code :-(.

Are you comfortable trying a fix for me before i tag a new release of aiohomekit?

You are looking for the check_convert_value function in aiohomekit/controller/tools.py and there around like 59 or so you should see:

if target_type == CharacteristicFormats.float:

We are going to add a single line to round() the float values:

    if target_type == CharacteristicFormats.float:
        try:
            val = float(val)
        except ValueError:
            raise FormatError('"{v}" is no valid "{t}"!'.format(v=val, t=target_type))

        # Honeywell T6 Pro cannot handle arbritary precision, so round the value.
        # See https://github.com/home-assistant/core/issues/37083
        val = round(val, 2)

If you prefer, here is the same change as a diff:

diff --git a/aiohomekit/controller/tools.py b/aiohomekit/controller/tools.py
index e944aa3..0eea340 100644
--- a/aiohomekit/controller/tools.py
+++ b/aiohomekit/controller/tools.py
@@ -62,6 +62,10 @@ def check_convert_value(val: str, target_type: str) -> int:
         except ValueError:
             raise FormatError('"{v}" is no valid "{t}"!'.format(v=val, t=target_type))

+        # Honeywell T6 Pro cannot handle arbritary precision, so round the value.
+        # See https://github.com/home-assistant/core/issues/37083
+        val = round(val, 2)
+
     if target_type == CharacteristicFormats.data:
         try:
             base64.decodebytes(val.encode())

Or if you prefer, there is a branch here as well.

If possible can you try different values for round() and report which ones do and don't work? 2 was a guess, but maybe it has to be 1 or maybe it could be 3 or 4.

[stshontikidis]

So it looks like that library was not included when I did the environment setup scripts/setup I can pull down that package and make the change. My issue is how to test against my thermostat that is paired with my prod HA? Am I going to have to reset the HomeKit on the thermostat and add it to my dev box? Or is there away to be risky and try things "in production"?

Edit: just saw it is part of the requirements_all.txt is it recommended to pip that req file or pulling things in as you need?

[Jc2k]

So for a change that small i'd be tempted to try it in prod, but I don't know your setup enough to know how best to proceed. I use the docker container and i sometimes edit the file in place inside the container as i know redploying the container will revert the change. Or sometimes i use the bind mount feature to replace some components with their development counterpart. This can be somewhat tricky with Home Assistant OS, though some people I've helped have been able to.

If proceeding with a dev environment, you shouldn't need to install requirements_all.txt in a dev env. Indeed I don't for my dev environment (it pulls a lot of stuff, and i think some of it doesnt install cleanly on macOS). Instead I start up HA and it will detect HomeKit devices on the network and pull the aiohomkeit dependency automatically. Alternatively, just pip install aiohomekit manually first.

There isn't a way to have to 2 different client pairings at once when using HA. iOS devices do this by having the "main" client add public keys on behalf of other iOS clients. We know the API, but this is quite a niche thing to do for HA environments so I haven't looked into providing a UI around it.

I don't think it's generally a good idea for all HA integrations, but for homekit_controller it is safe to copy the config entry between environments. The file in question is .storage/core.config_entries. You want to copy that file to your dev env and delete all but the thermostat. Both your prod and dev homekit_controller will work at the same time. I haven't noticed any problems doing this with my devices, but your thermostat will see connections from multiple IP's using the same client id - that's sort of undocumented behaviour so in theory it could do something weird if you go down that route.

[stshontikidis]

Got it, still new to HA all around and if I were to go back I maybe would have not gone the Full OS route but I am not looking to start over. haha. I will see if I can get a dev box spun up with my forked code base after work and see if I can test this out. All I forked was core and followed the developers documentation, I do see a container running but I think that is from the VS Code plugin, and in my venv I can run hass commands but I am not sure I have it fully working, not able to navigate to localhost:8123 any usual gotchas getting a dev instance off the ground? Sorry for all the back and forth, still trying to wrap my head around the dev environment. I am more used to pure web backends.

[Jc2k]

Sure thing, no problem. I felt the same when I started poking at home assistant. And my first PR was... Oh boy. It needed a few follow up PR's :-)

I haven't actually used the current developer documentation, as I got into it before the VS Code stuff was a thing. It looks pretty impressive, but I haven't had the time to try it yet. On a fresh MacOS it was enough for me to do something like:

git clone git://github.com/home-assistant/core home-assistant
cd home-assistant
python3 -m venv venv
source venv/bin/activate
pip install -r requirements_test.txt -e .
mkdir config
hass -c config

Eventually HA will come up on 127.0.0.1:8123 - it might take a while.

This is a really quiet logging configuration by default so it didn't tell me it had started the webserver.

I don't know if there is something there that will give you a clue about your own environment.

[stshontikidis]

long story short we are kind of F'd, it looks like it only takes whole numbers tried 22.2 22.22 22.222 22.2222 and all failed, while 22.0 did work. I was able to set my thermostat to a range of temperatures where this was ok but there are some that do not work as expected, no error status code but does not actually set correct 74F being an example. Is it possible we can set the units and send F instead of C? Is there anything I can send to the device to get any endpoint info or info on accepted values for iid: 13 is?

val = round(val, 0) and val = round(val) work see logs below for PUT and response followed by usual poll of GET characteristics for current state. Which also note is always a whole number from what I have been seeing

2020-06-25 17:47:59 DEBUG (MainThread) [aiohomekit.controller.ip.connection] 192.168.20.39: raw request: b'PUT /characteristics HTTP/1.1\r\nHost: 192.168.20.39\r\nContent-Type: application/hap+json\r\nContent-Length: 53\r\n\r\n{"characteristics":[{"aid":1,"iid":13,"value":22.0}]}'
2020-06-25 17:47:59 DEBUG (MainThread) [aiohomekit.controller.ip.connection] 192.168.20.39: raw response: bytearray(b'')
2020-06-25 17:49:00 DEBUG (MainThread) [aiohomekit.controller.ip.connection] 192.168.20.39: raw request: b'GET /characteristics?id=1.10,1.11,1.12,1.13 HTTP/1.1\r\nHost: 192.168.20.39\r\n\r\n'
2020-06-25 17:49:00 DEBUG (MainThread) [aiohomekit.controller.ip.connection] 192.168.20.39: raw response: bytearray(b'{"characteristics": [ {"aid":1,"iid":10,"value":2},{"aid":1,"iid":11,"value":2},{"aid":1,"iid":12,"value":22},{"aid":1,"iid":13,"value":22}]}')

2020-06-25 17:57:22 DEBUG (MainThread) [aiohomekit.controller.ip.connection] 192.168.20.39: raw request: b'PUT /characteristics HTTP/1.1\r\nHost: 192.168.20.39\r\nContent-Type: application/hap+json\r\nContent-Length: 51\r\n\r\n{"characteristics":[{"aid":1,"iid":13,"value":23}]}'
2020-06-25 17:57:22 DEBUG (MainThread) [aiohomekit.controller.ip.connection] 192.168.20.39: raw response: bytearray(b'')
2020-06-25 17:58:10 DEBUG (MainThread) [aiohomekit.controller.ip.connection] 192.168.20.39: raw request: b'GET /characteristics?id=1.10,1.11,1.12,1.13 HTTP/1.1\r\nHost: 192.168.20.39

val = round(val, 1) on logs below I try and set back to 73F which is why we see 22.8 instead of 22.2 which is 72, the value I previously sent as the temp via a service all

2020-06-25 17:51:43 DEBUG (MainThread) [aiohomekit.controller.ip.connection] 192.168.20.39: raw request: b'PUT /characteristics HTTP/1.1\r\nHost: 192.168.20.39\r\nContent-Type: application/hap+json\r\nContent-Length: 53\r\n\r\n{"characteristics":[{"aid":1,"iid":13,"value":22.8}]}'
2020-06-25 17:51:43 DEBUG (MainThread) [aiohomekit.controller.ip.connection] 192.168.20.39: raw response: bytearray(b'{"characteristics": [ {"aid":1,"iid":13,"status":-70410}]}')

val = round(val, 2)

2020-06-25 17:54:36 DEBUG (MainThread) [aiohomekit.controller.ip.connection] 192.168.20.39: raw request: b'PUT /characteristics HTTP/1.1\r\nHost: 192.168.20.39\r\nContent-Type: application/hap+json\r\nContent-Length: 54\r\n\r\n{"characteristics":[{"aid":1,"iid":13,"value":22.78}]}'
2020-06-25 17:54:37 DEBUG (MainThread) [aiohomekit.controller.ip.connection] 192.168.20.39: raw response: bytearray(b'{"characteristics": [ {"aid":1,"iid":13,"status":-70410}]}')

[Jc2k]

From the HomeKit spec:

This characteristic describes the target temperature in Celsius that the accessory is actively attempting to reach. For example, a thermostat cooling a room to 75 degrees Fahrenheit would set the target temperature value to 23.9 degrees Celsius.

The only C vs F thing in the spec is the display units, which is for toggling what to display on the thermostat UI not for changing the format the API expects.

I remembered that the API does expose a minStep and thats actually exactly what this is for. I'm guessing if you look in .storage/homekit_controller-entity-map you'll see a UUID of 00000035-0000-1000-8000-0026BB765291 (possibly its short form of 35) and it will have a minStep property of 1 presumably. It would be good if you could send your entity map file both to confirm it and also so i can include it in my regression tests.

(Make sure you are happy it doesn't contain any personal information first if posting here, otherwise send me a private message on the Home Assistant discord - and I can clean it for you).

[stshontikidis]

Hey was just typing this up, yea I found it in there.... are you on discord or anything? this could be quick but it is takes a value in 0.5 steps.

{
                                        "format": "float",
                                        "iid": 12,
                                        "maxValue": 100,
                                        "minStep": 0.5,
                                        "minValue": 0,
                                        "perms": [
                                            "pr",
                                            "ev"
                                        ],
                                        "type": "00000011-0000-1000-8000-0026BB765291",
                                        "unit": "celsius",
                                        "value": 23
                                    },
                                    {
                                        "format": "float",
                                        "iid": 13,
                                        "maxValue": 32,
                                        "minStep": 0.5,
                                        "minValue": 10,
                                        "perms": [
                                            "pr",
                                            "pw",
                                            "ev"
                                        ],
                                        "type": "00000035-0000-1000-8000-0026BB765291",
                                        "unit": "celsius",
                                        "value": 23
                                    },

b'PUT /characteristics HTTP/1.1\r\nHost: 192.168.20.39\r\nContent-Type: application/hap+json\r\nContent-Length: 53\r\n\r\n{"characteristics":[{"aid":1,"iid":13,"value":23.5}]}'
2020-06-25 18:30:19 DEBUG (MainThread) [aiohomekit.controller.ip.connection] 192.168.20.39: raw response: bytearray(b'')

[stshontikidis]

would rather ping you the full thing over discord, I think I found you on there... I am honestly not sure the sensitivity of the GUIDs and stuff.

[Jc2k]

As discussed on discord, have implemented clamping to minStep in the branch now. This reverts and replaces the previous approach. See here.

This was a little more dicey because a minStep of 0.1 (which ecobee uses) really triggers some messy floating point approximations. So had to use Decimal. And even converting the float 0.1 to Decimal was annoying, so had to lower the precision. Hopefully there aren't any devices out there that set the temperature in 0.000001 increments...

Want to add some tests on my side for other minStep sizes, but this is ready for testing again on your side when you are ready.

[stshontikidis]

Looks like a solid solution and should cover most sane device styles. Everything on my end testing against the Lyric T6 Pro was good, went through a ton of F values from and each one took. Output logs

[Jc2k]

Great! Thanks for testing.

I've added a bunch more testing on my side and, sigh, I found some issues. First of all integers have a minStep property too. It's usually 1 but applying the code we have so far turns the integers into floats. Given the problems we've seen with the manufacturers supporting JSON i'm pretty sure there will be a device on the market that can't handle receiving a float value for an int field, so I had to fix that. Whilst writing tests for it I learned something new about python3 rounding. It changed from how it worked in py2! The default is now bankers rounding. This meant some values that i was expecting to round up rounded down. So i've had to stop using round() and instead lean even more into the decimal module. The tests I have still pass, as do the new tests, but it would be great if you could test again. The changes are quite a bit larger than before as i've refactored the code so check_convert_value has access to all the information on a characteristic. The changes are back on the main master branch now. If you want to see the diff its here.

[stshontikidis]

Been unable to get around to testing just yet, hope to get time tonight or tomorrow night at the latest (-5 GMT).

[stshontikidis]

@Jc2k Looks good on my end against the Lyric did a bunch of setting of temps, on/off, heat/cool/target.
lyric_output.txt

2020-06-29 18:27:06 DEBUG (MainThread) [aiohomekit.controller.ip.connection] 192.168.20.39: raw request: b'PUT /characteristics HTTP/1.1\r\nHost: 192.168.20.39\r\nContent-Type: application/hap+json\r\nContent-Length: 50\r\n\r\n{"characteristics":[{"aid":1,"iid":11,"value":0}]}'
2020-06-29 18:27:06 DEBUG (MainThread) [aiohomekit.controller.ip.connection] 192.168.20.39: raw response: bytearray(b'')
2020-06-29 18:27:08 DEBUG (MainThread) [aiohomekit.controller.ip.connection] 192.168.20.39: raw request: b'PUT /characteristics HTTP/1.1\r\nHost: 192.168.20.39\r\nContent-Type: application/hap+json\r\nContent-Length: 50\r\n\r\n{"characteristics":[{"aid":1,"iid":11,"value":2}]}'
2020-06-29 18:27:08 DEBUG (MainThread) [aiohomekit.controller.ip.connection] 192.168.20.39: raw response: bytearray(b'')
2020-06-29 18:27:11 DEBUG (MainThread) [aiohomekit.controller.ip.connection] 192.168.20.39: raw request: b'PUT /characteristics HTTP/1.1\r\nHost: 192.168.20.39\r\nContent-Type: application/hap+json\r\nContent-Length: 50\r\n\r\n{"characteristics":[{"aid":1,"iid":11,"value":3}]}'
2020-06-29 18:27:11 DEBUG (MainThread) [aiohomekit.controller.ip.connection] 192.168.20.39: raw response: bytearray(b'')
2020-06-29 18:27:17 DEBUG (MainThread) [aiohomekit.controller.ip.connection] 192.168.20.39: raw request: b'PUT /characteristics HTTP/1.1\r\nHost: 192.168.20.39\r\nContent-Type: application/hap+json\r\nContent-Length: 50\r\n\r\n{"characteristics":[{"aid":1,"iid":11,"value":2}]}'
2020-06-29 18:27:17 DEBUG (MainThread) [aiohomekit.controller.ip.connection] 192.168.20.39: raw response: bytearray(b'')
2020-06-29 18:27:26 DEBUG (MainThread) [aiohomekit.controller.ip.connection] 192.168.20.39: raw request: b'PUT /characteristics HTTP/1.1\r\nHost: 192.168.20.39\r\nContent-Type: application/hap+json\r\nContent-Length: 50\r\n\r\n{"characteristics":[{"aid":1,"iid":11,"value":3}]}'
2020-06-29 18:27:26 DEBUG (MainThread) [aiohomekit.controller.ip.connection] 192.168.20.39: raw response: bytearray(b'')

just wanted to highlight some of the integer minStep stuff which is the off/cool/heat/heat_cool states

[Jc2k]

Brilliant! Thanks for your help testing this! It’s the end of the day here but I’ll get a new release of aiohomekit made tomorrow, and hopefully put in a PR to HA as well. Off the top of my head I think we might miss the time window for the next HA release but we’ll see...

[stshontikidis]

cool, not like the release cycle is that slow so not too worried. Though we had a flurry but it calmed down, are they gearing up for the move to 112? I guess I should have noted my testing environment was based on 112. Thanks again for the responsiveness, glad I don't have to use Honeywell dev API and can keep it local.

[stshontikidis]

@Jc2k Were you able to tag a new release of the aiohomekit library or did you run into any more oddities? I had my dev environment going for a few days and it seemed good but that was obviously only against 1 device.

[Jc2k]

Thanks for the reminder! It's been super busy here and i've not yet had chance to do it. Was just lamenting to my partner last night that we had done the hard bit and I didn't have time to do the easy bit! Super frustrating!

[Jc2k]

There we go - PR raised here.

[stshontikidis]

Awesome, thanks again! Also I hit you up on discord about looking into lack of fan control through HomeKit Controller but after some debugging and speaking with users that have it integrated through the native HomeKit on iOS, no control there either, it looks like that is not an issue and just something not exposed.

[mr-stay-puft]

Been following this issue myself. Thank you both for your efforts!

[Jc2k]

This is now on dev. @stshontikidis if you can test the dev branch in your dev environment that would be great. Otherwise it should make it into 0.113.0.

And I can't thank you enough for your help testing @stshontikidis. It was great working with you on it!

[stshontikidis]

Man you got that reviewed fast! I have been sitting around on another PR for like 5 days waiting for a review. Was told there was no real process except to wait and someone will tackle it when they have time.

I am currently working on a new integration but when I get to a good point I will pull in dev.

[Jc2k]

@stshontikidis there is some method to the madness. homekit_controller is an existing integration, the PR was small, and it was by the listed codowner. Those sorts of PR's tend to get picked up first to keep things moving and because they are somewhat easy. A version bump PR is the easiest.

But even though i've got over a hundred PR's under my belt, a new integration PR of mine still took over a month to be picked up. They will get to you eventually, don't lose hope. Like it was over a month and in danger of autoclosing for one of mine. But there are just 223 PR's in flight right now and that number never seems to go down for long.