Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Invalid Lennox E30 pairing flow via HomeKit controller #20885

Closed
jeromelaban opened this issue Feb 9, 2019 · 123 comments
Closed

Invalid Lennox E30 pairing flow via HomeKit controller #20885

jeromelaban opened this issue Feb 9, 2019 · 123 comments

Comments

@jeromelaban
Copy link

jeromelaban commented Feb 9, 2019

Home Assistant release with the issue:

0.87.0

Last working Home Assistant release (if known):
None

Operating environment (Hass.io/Docker/Windows/etc.):
Docker

Component/platform:

Homekit controller

Description of problem:
Here's the "normal" flow for pairing a Lennox E30 to home kit:

  • Enter the WAC mode on the Lennox E30
  • Associate it with an iOS device to a known wifi network. No pairing code appears on the Lennox E30 screen
  • Open the Home app on iOS, select add an accessory
  • Select "I don't have a code"
  • Once the input UI appears on iOS, the pairing code appears on the Lennox E30 device
  • Type in the code, then the device is paired

Pairing a Lennox E30 with home-assistant using the HomeKit Controler does not work properly, for two reasons:

  • The homekit_python HA component does not handle properly the fact the the c# attribute is exposed as C# by the Lennox E30, making home-assistant fail for a case sensitive lookup here:
    https://github.com/home-assistant/home-assistant/blob/d16d14b648b00ffe97c10180472fd004a131c99c/homeassistant/components/homekit_controller/__init__.py#L353
    Changing this line with an upper case C# makes the homekit_controller discover the Lennox E30 properly, and show the pairing dialog in the HA UI. This is obviously not a proper fix, but maybe homekit_python should sanitize this attribute?
  • Once the HA pairing dialog appears, asking for the pairing code, there's no code available on the Lennox E30 screen. The pairing code only appears briefly (<250ms) on the Lennox E30 screen when entering a dummy pairing code in HA. The briefly shown pairing code changes every time a dummy code is entered in HA (to make things harder 😊).

Tracking down the issue in homekit_python, I could create a manual pairing by adding:

    pin = input("PIN: ")

at this line:
https://github.com/jlusiardi/homekit_python/blob/9604d76131a9955c8b4dbd9f10dc92394cc61ae3/homekit/protocol/__init__.py#L102

and run it as CLI:

python3 pair.py -d XX:XX:XX:XX:XX:XX -f pairing.json -a "LennoxE30" -p 123456

Blocking the pairing procedure to input the pin at the precise moment (after step 3, the verify request) during the negotiation makes the pairing code appear and stay visible long enough on the Lennox E30 screen to be able to input it at the CLI.

I've yet to be able to reuse the pairing file in home-assistant, and I'm not sure either how to create such an asynchronous flow using the HA UI.

Update 2019-02-09: I was able to reuse in HA the pairing I made using my modified CLI of homekit_python. The name of the pairing has to be the device ID XX:XX:XX:XX:XX:XX (AccessoryPairingID value in the pairing file). I'm only able to see the temperature, modes are not available and setting the temperature does not seem to have any actual effect.

@jeromelaban jeromelaban changed the title Lennox E30 Support via HomeKit controller Invalid Lennox E30 pairing flow via HomeKit controller Feb 9, 2019
@Jc2k
Copy link
Member

Jc2k commented Feb 11, 2019

Hi - thanks for raising this. Good work :-) You'll be pleased to know that I think most of this is known and being worked on but there isn't an ETA yet.

The pairing bug is already tracked here. There is a branch with a very similar hack here and the upstream bug for async pairing is here.

There is already a bug for C# (#17853). I haven't commented yet but my local branch should no longer choke on that.

The main work right now is to port homekit_controller from Configurator to ConfigEntries. It's quite a large change so it's probably going to have to be split up and sent through the HA review process in stages.

Are there any errors when setting the temperature etc?

If you can send me the output of:

python3 get_accessories.py -f pairing.json -a "LennoxE30" -o json

That would be super helpful.

@jeromelaban
Copy link
Author

My pleasure! Good work on this component!

When setting the temperature from HA, there are no errors in the log, the thermostat value does not change, and the value in HA is reset to the actual thermostat value after a while.

Which logger should I enable to see the errors, if any ?

Here's the accessories result:

[
    {
        "aid": 1,
        "services": [
            {
                "characteristics": [
                    {
                        "format": "bool",
                        "iid": 2,
                        "perms": [
                            "pw"
                        ],
                        "type": "14"
                    },
                    {
                        "format": "string",
                        "iid": 3,
                        "perms": [
                            "pr"
                        ],
                        "type": "20",
                        "value": "Lennox"
                    },
                    {
                        "format": "string",
                        "iid": 4,
                        "perms": [
                            "pr"
                        ],
                        "type": "21",
                        "value": "E30 2B"
                    },
                    {
                        "format": "string",
                        "iid": 5,
                        "perms": [
                            "pr"
                        ],
                        "type": "23",
                        "value": "Lennox"
                    },
                    {
                        "format": "string",
                        "iid": 6,
                        "perms": [
                            "pr"
                        ],
                        "type": "30",
                        "value": "XXXXXXXX"
                    },
                    {
                        "format": "string",
                        "iid": 7,
                        "perms": [
                            "pr"
                        ],
                        "type": "52",
                        "value": "3.40.XX"
                    },
                    {
                        "format": "string",
                        "iid": 8,
                        "perms": [
                            "pr"
                        ],
                        "type": "53",
                        "value": "3.0.XX"
                    }
                ],
                "iid": 1,
                "type": "3E"
            },
            {
                "characteristics": [
                    {
                        "format": "uint8",
                        "iid": 101,
                        "maxValue": 2,
                        "minStep": 1,
                        "minValue": 0,
                        "perms": [
                            "pr",
                            "ev"
                        ],
                        "type": "F",
                        "value": 1
                    },
                    {
                        "format": "uint8",
                        "iid": 102,
                        "maxValue": 3,
                        "minStep": 1,
                        "minValue": 0,
                        "perms": [
                            "pr",
                            "pw",
                            "ev"
                        ],
                        "type": "33",
                        "value": 3
                    },
                    {
                        "format": "float",
                        "iid": 103,
                        "maxValue": 100,
                        "minStep": 0.1,
                        "minValue": 0,
                        "perms": [
                            "pr",
                            "ev"
                        ],
                        "type": "11",
                        "unit": "celsius",
                        "value": 20.5
                    },
                    {
                        "format": "float",
                        "iid": 104,
                        "maxValue": 32,
                        "minStep": 0.5,
                        "minValue": 4.5,
                        "perms": [
                            "pr",
                            "pw",
                            "ev"
                        ],
                        "type": "35",
                        "unit": "celsius",
                        "value": 21
                    },
                    {
                        "format": "uint8",
                        "iid": 105,
                        "maxValue": 1,
                        "minStep": 1,
                        "minValue": 0,
                        "perms": [
                            "pr",
                            "pw",
                            "ev"
                        ],
                        "type": "36",
                        "value": 0
                    },
                    {
                        "format": "float",
                        "iid": 106,
                        "maxValue": 37,
                        "minStep": 0.5,
                        "minValue": 16,
                        "perms": [
                            "pr",
                            "pw",
                            "ev"
                        ],
                        "type": "D",
                        "unit": "celsius",
                        "value": 29.5
                    },
                    {
                        "format": "float",
                        "iid": 107,
                        "maxValue": 100,
                        "minStep": 1,
                        "minValue": 0,
                        "perms": [
                            "pr",
                            "ev"
                        ],
                        "type": "10",
                        "unit": "percentage",
                        "value": 34
                    },
                    {
                        "format": "float",
                        "iid": 108,
                        "maxValue": 32,
                        "minStep": 0.5,
                        "minValue": 4.5,
                        "perms": [
                            "pr",
                            "pw",
                            "ev"
                        ],
                        "type": "12",
                        "unit": "celsius",
                        "value": 21
                    }
                ],
                "iid": 100,
                "primary": true,
                "type": "4A"
            }
        ]
    }
]

@Jc2k
Copy link
Member

Jc2k commented Feb 12, 2019

My gut feeling is that if you don't see any tracebacks in the logs with the default config then its probably the accessory rejecting the change, and we aren't logging it. If you are comfortable editing the code then track down this line of code in your environment and print the output:

https://github.com/home-assistant/home-assistant/blob/dev/homeassistant/components/homekit_controller/__init__.py#L315

@jeromelaban
Copy link
Author

I added this:

        res = self._accessory.pairing.put_characteristics(chars)
        _LOGGER.info("put_characteristics  %s", res)

which said this:

2019-02-14 01:30:30 INFO (SyncWorker_19) [homeassistant.components.homekit_controller] put_characteristics  {}

Which could mean this, if I'm not mistaken: https://github.com/jlusiardi/homekit_python/blob/88b40a9a64a1a897f3751c07c48e117472b92d3a/homekit/controller.py#L458

I'll put some logs deeper to see what happens.

@jeromelaban
Copy link
Author

Also interesting, if I do this:

python3 homekit/put_characteristic.py -f pairing.json -a "XX:XX:XX:XX:XX:X" -c 1.108 22

the temperature changes properly on the thermostat.

@Jc2k
Copy link
Member

Jc2k commented Feb 14, 2019

Ok, gut feeling one disproved. Dang. You are correct, it looks like there are no errors. I guess we should also log the value of chars right before we call put_characteristics too and make sure its trying to write to the correct characteristic.

@GaryOkie
Copy link
Contributor

GaryOkie commented Apr 1, 2019

Have an Lennox iComfort S30 which currently lacks any HA integration, and am just commenting here to track progress with Homekit integration. Thanks very much for all the work done on this so far!

@garyak
Copy link

garyak commented Apr 1, 2019

I have two S30s, one controls four zones, the other just one. Wanted interested parties to know I'd be happy to test any Home Assistant / Homekit integration work involving these thermostats.

@niemyjski
Copy link

I'm getting a similar error when I try to pair my Hunter Douglas PowerView Hub via HomeKit controller in 0.91.

Thu Apr 04 2019 09:34:01 GMT-0500 (CDT)

step 3
Traceback (most recent call last):
  File "/usr/local/lib/python3.7/site-packages/homeassistant/components/websocket_api/commands.py", line 122, in handle_call_service
    connection.context(msg))
  File "/usr/local/lib/python3.7/site-packages/homeassistant/core.py", line 1138, in async_call
    self._execute_service(handler, service_call))
  File "/usr/local/lib/python3.7/site-packages/homeassistant/core.py", line 1160, in _execute_service
    await handler.func(service_call)
  File "/usr/local/lib/python3.7/site-packages/homeassistant/components/configurator/__init__.py", line 221, in async_handle_service_call
    call.data.get(ATTR_FIELDS, {}))
  File "/usr/local/lib/python3.7/concurrent/futures/thread.py", line 57, in run
    result = self.fn(*self.args, **self.kwargs)
  File "/usr/local/lib/python3.7/site-packages/homeassistant/components/homekit_controller/connection.py", line 127, in device_config_callback
    self.controller.perform_pairing(self.hkid, self.hkid, code)
  File "/usr/local/lib/python3.7/site-packages/homekit/controller/controller.py", line 337, in perform_pairing
    pairing = perform_pair_setup(pin, str(uuid.uuid4()), write_fun)
  File "/usr/local/lib/python3.7/site-packages/homekit/protocol/__init__.py", line 132, in perform_pair_setup
    error_handler(response_tlv[1][1], 'step 3')
  File "/usr/local/lib/python3.7/site-packages/homekit/protocol/__init__.py", line 53, in error_handler
    raise BusyError(stage)
homekit.exceptions.BusyError: step 3

@Jc2k
Copy link
Member

Jc2k commented Apr 4, 2019

@niemyjski - a BusyError means an "existing pairing is ongoing" and is on its own unrelated to the issues in this ticket. Is the PowerView Hub a device with a screen and a pin code that changes like the Lennox E30?

@niemyjski
Copy link

niemyjski commented Apr 4, 2019 via email

@Jc2k
Copy link
Member

Jc2k commented Apr 4, 2019

I meant the device thinks you were already trying to pair it, then started to pair it a second time. Which is subtly different to it already being paired.

@Jc2k
Copy link
Member

Jc2k commented May 15, 2019

For those of you who have paired manually (like @jeromelaban), 0.93 should fix a bunch more issues that were mentioned on this ticket. This includes correctly populating operation_list, support for thermostats that have humidity and target humidity features, support for auto and more.

(@jeromelaban i would appreciate an update on how this looks on your end).

The main config flow work is now on dev so should be in 0.94. If you have paired manually the pairing will be migrated over to a config entry and your device will start showing up on the Integrations screen. This doesn't fix the underlying pairing issue but was the big change blocking me from working on pairing.

There are some other changes pending:

  • There is a related branch here that adds device registry support. This allows HA to know the relationship between entities and physical devices attached to HomeKit hubs.

  • Now that config entries is done I've also finished polishing the pairing patch which is really the one most people tracking this ticket are interested in. It's here. With this we should be able to support any HomeKit with a display (i.e. randomised pairing codes) - save for device specific quirks.

@GaryOkie
Copy link
Contributor

@Jc2k,

I really appreciate you updating us on the progress! I've been following the PR, but have held off attempting a manual sync. Sure hope the Lennox S30 isn't one to have "device specific quirks" and will pair successfully with the .94 update.

@Jc2k
Copy link
Member

Jc2k commented May 30, 2019

The first 0.94 beta has just been tagged while i was asleep (about 8 hours ago). This includes:

  • The pairing fix for devices with screens and random pairing codes
  • The new config entry support (so your devices appear under 'Integrations' like Hue etc)
  • Device info support (groups entities by phsyical device and lets you put them in areas)
  • Migrates to the new zeroconf module for discovery

This is still beta so there is still time to fix any surprise gotchas. This is obviously quite a big set of changes so if anyone can run the beta and let me know how they get on please do.

@Jc2k
Copy link
Member

Jc2k commented Jun 7, 2019

0.94 is out now - so would be great if you guys could let me know if you could now pair your Lennox E30's.

@GaryOkie
Copy link
Contributor

GaryOkie commented Jun 7, 2019

I certainly plan to! My E-30 is installed at my lake house, so I hope to get up there soon. Area flooding/road access is a bit of mess right now.

EDIT: I meant my S30. Same thing really from the interface perspective.

@garyak
Copy link

garyak commented Jun 7, 2019

Are S30's included? If so, I can test when I return home.

@Jc2k
Copy link
Member

Jc2k commented Jun 7, 2019

@garyak It depends! I don't have a ticket about S30 in particular, so I don't know why its not already working. If its just that the S30 has random pairing codes which it shows on a display, then you should test it with 0.94 as the issues around that should be fixed now.

@GaryOkie
Copy link
Contributor

GaryOkie commented Jun 7, 2019

Yep - the Lennox iComfort S30/E30/M30 all have displays that upon pairing, display a random code.

There is another separate iComfort Wifi model with a different API that has its own component and doesn't require Homekit.

@garyak
Copy link

garyak commented Jun 7, 2019

OK. I'll test the S30's and report the results.

@jeromelaban
Copy link
Author

@Jc2k thanks for the update!

I tried the pairing (reset the E30 completely). it fails and this is what I'm getting:

,2019-06-10 20:35:33 ERROR (MainThread) [homeassistant.components.homekit_controller.config_flow] Pairing attempt failed with an unhandled exception
,Traceback (most recent call last):
,  File "/usr/src/app/homeassistant/components/homekit_controller/config_flow.py", line 259, in async_step_pair
,    pairing)
,  File "/usr/src/app/homeassistant/components/homekit_controller/config_flow.py", line 336, in _entry_from_accessory
,    pairing.list_accessories_and_characteristics
,  File "/usr/local/lib/python3.7/concurrent/futures/thread.py", line 57, in run
,    result = self.fn(*self.args, **self.kwargs)
,  File "/usr/local/lib/python3.7/site-packages/homekit/controller/ip_implementation.py", line 81, in list_accessories_and_characteristics
,    accessories = json.loads(tmp)['accessories']
,  File "/usr/local/lib/python3.7/json/__init__.py", line 348, in loads
,    return _default_decoder.decode(s)
,  File "/usr/local/lib/python3.7/json/decoder.py", line 337, in decode
,    obj, end = self.raw_decode(s, idx=_w(s, 0).end())
,  File "/usr/local/lib/python3.7/json/decoder.py", line 355, in raw_decode
,    raise JSONDecodeError("Expecting value", s, err.value) from None
,json.decoder.JSONDecodeError: Expecting value: line 1 column 1 (char 0)
,2019-06-10 20:35:33 ERROR (MainThread) [homeassistant.components.homekit_controller.config_flow] Pairing attempt failed with an unhandled exception
,Traceback (most recent call last):
,  File "/usr/src/app/homeassistant/components/homekit_controller/config_flow.py", line 288, in async_step_pair
,    start_pairing, self.hkid, self.hkid
,  File "/usr/local/lib/python3.7/concurrent/futures/thread.py", line 57, in run
,    result = self.fn(*self.args, **self.kwargs)
,  File "/usr/local/lib/python3.7/site-packages/homekit/controller/controller.py", line 370, in start_pairing
,    raise AlreadyPairedError('Alias "{a}" is already paired.'.format(a=alias))
,homekit.exceptions.AlreadyPairedError: Alias "XX:XX:XX:XX:XX:XX" is already paired.

Each new pairing attempt shows the homekit.exceptions.AlreadyPairedError with a new alias.

The E30 itself thinks it's paired, though, so the flow works properly from the devices standpoint.

Keep up the good work!

@Jc2k
Copy link
Member

Jc2k commented Jun 11, 2019

@jeromelaban can you take a copy of your .homekit directory, then delete it (or just move it to the side? Then reset the E30 and restart HA. What happens now? I suspect the old pairing.json is involved somehow.

@jeromelaban
Copy link
Author

@Jc2k that's the first thing I tried, but it did not help, .homekit folder is not re-created either, which is also curious.

@Jc2k
Copy link
Member

Jc2k commented Jun 11, 2019

Yeah with config entries that folder is no more, woo.

So I’m guessing you attempted to pair twice (or at least submitted twice) and that’s where the AlreadyPairedError comes from. But why the first call to /accessories is failing is a bit of a mystery right now.

@jeromelaban
Copy link
Author

Excellent news, I'll poke somewhere else then. The error message happened on the first try, and the ID changed every time. Where can I find those new entries ?

Also, I'll try to find out about that invalid document, will let you know.

@GaryOkie
Copy link
Contributor

I finally made it up the the lakehouse and am now trying to pair the Lennox S30. I must be missing something since I never get a pairing code prompt in HA.

All I've added to the configuration is zeroconf: (I had discovery/enable/homekit initially per the docs, but the log said it wasn't needed). I enter WAC mode on the S30 and then at some point I should expect to see a prompt for code in the HA notifications?

I did try to manually add the Homekit Accessory Integration via the UI, but it said "aborted, no unpaired devices found" - and this was while the S30 was in WAC mode.

The HASSIO/Raspi is Ethernet connected, and on the same subnet as the S30, not on wifi, but there shouldn't be any network isolation between the two. Does this WAC mode pairing require Hassio be on wifi too?

@Jc2k
Copy link
Member

Jc2k commented Jun 12, 2019

@GaryOkie I can’t give specific instructions because every device is different - here I’m struggling a bit because I’ve never even heard of WAC mode before.

Home Assistant is looking for an mdns record of type _hap._tcp.local. If it can’t see that it won’t work. My HA is a VM with the host connected by Ethernet, and it can see WiFi homekit devices just fine. The only gotcha is if you have seperate subnets then mdns records are not visible across subnets without a reflector. But as you say this shouldn’t be a problem for you.

Can you run python3 -m homekit.discover on the machine or container where hass is?

Do you have a iPhone and can that see your device?

Is it definitely pingable from the hass machine?

@GaryOkie
Copy link
Contributor

Ok @Jc2k - will do. As I had mentioned, the only code changes other than the logging was I kept the import time; time.sleep(10) delay in ip_implementation. I had already removed all the logging but I will remove the sleep as well and reset/repeat the pairing.

back soon with an update...

@GaryOkie
Copy link
Contributor

Good news @Jc2k - using production code, I was able to repeat the pairing process 3 times with no problems at all!

@Jc2k
Copy link
Member

Jc2k commented Oct 15, 2019

That is wonderful news indeed :-)

@jeromelaban
Copy link
Author

It is working for me as well, paired on the first try! Thank you very much :)

@GaryOkie
Copy link
Contributor

Would someone with an iComfort S30/M30/E30 please temporarily enable the "wider temperature setpoints" (under settings/heat&cool)? This will change the allowed range from 60-90 to 40-99 (F).

What is being reported to HA via the Homekit integration is an incorrect min_temp=60, instead of 40. The max_temp is correct at 99. I'm unable to set the temp lower than 60 via HA, but it works fine via the thermostat touchpad or iComfort app.

Can this min_temp reporting problem be duplicated by anyone?

Thanks!

@garyak
Copy link

garyak commented Oct 16, 2019

With wider set points set on the S30, HA reports min 40, max 90. Unfortunately these settings, when made from HA, don't reflect back to the S30.

The lovelace card returns to 60 degrees, or 40 depending on your setting at the S30.

@GaryOkie
Copy link
Contributor

Thanks @garyak for testing. Your reported wide min/max setpoint range is different I am seeing and I think that may be because my Tstat was in cool mode and yours was in Heat mode. The default range is 60-90 and the optional wider range is 40-99. I don't think it is correct to have either a 40-90 or 60-99 range reported, regardless of which mode the Tstat is in.

@Jc2k - I have the original debug GET_Accessories output from when the S30 first paired and have compared it to the homekit_controller-entity-map pairing file, and both agree.

There appear to be two relevant Homekit IID's pertaining to temperature range...
IID: 106 (COOL Mode?) shows a range of 15.5-37 C, which is 60-99F, the exact values I'm seeing.
IID: 108 (HEAT Mode?) shows a range of 4.5-32 C which is 40-90F, the exact values @garyak gets.

I have since switched from Cool to Heat mode since the pairing was done, but the range has not changed for me - it is still 60-99. Problem is, I need to remotely lower the temp below 60 for an empty vacation cabin that is winterized and can't do it via HA/Homekit, only via the iComfort app which properly honors the wider setpoint range of 40-99.

Formatted debug output follows:

accessories:[ |  
aid:1,"services":[ |  
characteristics:[ |  
format:"bool","iid":2,"perms":["pw"],"type":"14",
format:"string","iid":3,"perms":["pr"],"type":"20","value":"Lennox",
format:"string","iid":4,"perms":["pr"],"type":"21","value":"S30   2B",
format:"string","iid":5,"perms":["pr"],"type":"23","value":"iComfort   S30 xxxxx",
format:"string","iid":6,"perms":["pr"],"type":"30","value":"WL18Exxxxx",
format:"string","iid":7,"perms":["pr"],"type":"52","value":"3.55.626",
format:"string","iid":8,"perms":["pr"],"type":"53","value":"3.0.002"],"iid":1,"type":"3E",

characteristics:[ |  
format:"uint8","iid":101,"maxValue":2,"minStep":1,"minValue":0,"perms":["pr","ev"],"type":"F","value":0,
format:"uint8","iid":102,"maxValue":3,"minStep":1,"minValue":0,"perms":["pr","pw","ev"],"type":"33","value":2,
format:"float","iid":103,"maxValue":100,"minStep":0.1,"minValue":0,"perms":["pr","ev"],"type":"11","unit":"celsius","value":21.5,
format:"float","iid":104,"maxValue":37,"minStep":0.5,"minValue":15.5,"perms":["pr","pw","ev"],"type":"35","unit":"celsius","value":26.5,
format:"uint8","iid":105,"maxValue":1,"minStep":1,"minValue":0,"perms":["pr","pw","ev"],"type":"36","value":1,
format:"float","iid":106,"maxValue":37,"minStep":0.5,"minValue":15.5,"perms":["pr","pw","ev"],"type":"D","unit":"celsius","value":26.5,
format:"float","iid":107,"maxValue":100,"minStep":1,"minValue":0,"perms":["pr","ev"],"type":"10","unit":"percentage","value":60,
format:"float","iid":108,"maxValue":32,"minStep":0.5,"minValue":4.5,"perms":["pr","pw","ev"],"type":"12","unit":"celsius","value":19.5,
format:"string","iid":109,"perms":["pr"],"type":"23","value":"Thermostat"],"iid":100,"primary":true,"type":"4A",

characteristics:[ |  
format:"string","iid":301,"perms":["pr"],"type":"37","value":"1.1.0"],"iid":300,"type":"A2"]]

And a snip from the pairing file...

"iid":   106,
--
"maxValue": 37,
"minStep": 0.5,
"minValue": 15.5,
"type":   "0000000D-0000-1000-8000-0026BB765291",
"unit": "celsius",
"value": 25
  |  
"iid": 108,
"maxValue": 32,
"minStep": 0.5,
"minValue": 4.5,
"type":   "00000012-0000-1000-8000-0026BB765291",
"unit": "celsius",
"value": 19.5

And finally, HA climate.icomfort_s30 entity state:

State: HEAT
Attributes:
hvac_modes: off,heat,cool,heat_cool
current_temperature: 65
min_temp: 60
max_temp: 99
temperature: 60
current_humidity: 55
hvac_action: on
friendly_name: Downstairs iComfort S30
supported_features: 1

@GaryOkie
Copy link
Contributor

After restarting HA while the S30 is in Heat mode, I am now getting a min/max range of 40-90, the same as @garyak. I'll just have to remember to restart HA any time the mode is changed to get the new range. Not sure why, but I can live with that.

Evidently Lennox hardcoded their Homekit interface to use the 40-90F min/max range for Heat, and 60-99F for Cool. The option to enable a wider setpoint range regardless of mode is not honored by the Homekit interface and seems to be only relevant for the Tstat and iComfort app the best I can tell.

LOL - I switched back to Cool mode, restarted, and the range is still 40-90. Oh well, that range is fine, even though it beats me how/when this Homekit component refreshes attributes.

@Jc2k
Copy link
Member

Jc2k commented Oct 17, 2019

So what is supposed to happen is the thermostat bumps the c# attribute whenever the attribute database changes. This is broadcast by bonjour and HA should see it and pull the latest "entity map". Could be the comfort is always updating the attributes but not always updating c# in a timely fashion or we aren't noticing. You could use the CLI to see if c# is changing. If it's not, theres not much i can do their implementation is buggy. If it is, its my problem to sort out (:sob:).

RE: The 2 UUID's you posted - they are the cooling and heating thresholds and are apparently only applicable to Auto mode. We don't support them yet, mostly because i don't have a way to test what happens.

@GaryOkie
Copy link
Contributor

GaryOkie commented Oct 17, 2019

Thanks for the info! I have checked the timestamp of .storage/homekit_controller-entity-map and noticed that it doesn't change when I change heat/cool modes or temp values from either the HA UI or the iComfort app. It also doesn't change after an HA restart.

So what is this CLI you mention and how do I use it to see if c# is changing? I know of C#, the programming language, but not the "c#" you are referring to.

Do you have a documentation link that describes the UUID's? I've tried, but not found one yet. Still don't understand where HA is getting the temperature ranges from if 106/108 only apply to Auto mode that isn't supported by the component. And since it's not supported, why is "Heat_Cool" being reported as an available mode and show up as a button on the climate card? Isn't that Auto mode?

EDIT: On second thought, by "CLI", you mean the SSH/Bash terminal into HA?

@Jc2k
Copy link
Member

Jc2k commented Oct 17, 2019

The cli ships with the library we use to make the HA integration:

https://github.com/jlusiardi/homekit_python/

It should be already installed in your HA install so it should be enough to do something like:

python3 -m homekit.discover

And after about 30s you should see output like this:

Name: smarthomebridge3._hap._tcp.local.
Url: http://192.168.178.21:51827
Configuration number (c#): 2
Feature Flags (ff): Paired (Flag: 0)
Device ID (id): 12:34:56:78:90:05
Model Name (md): Bridge
Protocol Version (pv): 1.0
State Number (s#): 1
Status Flags (sf): 0
Category Identifier (ci): Other (Id: 1)

(This is the data that we use to detect the devices available to pair)

@GaryOkie
Copy link
Contributor

GaryOkie commented Oct 17, 2019

bash-5.0# python3 -m homekit.discover

Name: iComfort S30 5185f0._hap._tcp.local.
Url: http_impl://192.168.0.53:7373
Configuration number (c#): 2
Feature Flags (ff): Supports HAP Pairing (Flag: 1)
Device ID (id): 4A:D4:7A:95:0A:42
Model Name (md): S30 2B
Protocol Version (pv): 1.1
State Number (s#): 1
Status Flags (sf): Accessory has been paired. (Flag: 0)
Category Identifier (ci): Thermostat (Id: 9)

EDIT: Ah, just now see the c# number!

@Jc2k
Copy link
Member

Jc2k commented Oct 17, 2019

:-) As it is currently 2 it seems it does not change when you change between heat and cool. But you should be able to change settings on the thermostat and verify.

As for the UUID documentation, you can download the official spec here:

https://developer.apple.com/homekit/specification/

You'll need an Apple ID but i don't think you need to be in the paid for developer program or anything like that.

For the 2 you called out it says:

This characteristic describes the cooling threshold in Celsius for devices that support simultaneous heating and cooling. The value of this characteristic represents the 'maximum temperature' that must be reached before cooling is turned on.

For example, if the Target Heating Cooling State (page 161) is set to "Auto" and the current temperature goes above the 'maximum temperature', then the cooling mechanism should turn on to decrease the current temperature until the 'minimum temperature' is reached.

and

This characteristic describes the heating threshold in Celsius for devices that support simultaneous heating and cooling. The value of this characteristic represents the 'minimum temperature' that must be reached before heating is turned on.

For example, if the Target Heating Cooling State (page 161) is set to "Auto" and the current temperature goes below the 'minimum temperature', then the heating mechanism should turn on to increase the current temperature until the 'minimum temperature' is reached.

There are 2 key things in these that have made me reluctant to press ahead with making arbritary changes:

  • "This characteristic describes the X threshold in celsius for devices that support simultaneous heating and cooling"
  • "For example, if the target heating cooling state is set to "auto" and..."

Clearly in some cases we need to respect these characteristics and in others we need to respect the target temperature characteristic (like we do now). But IMO it is vague and it doesn't say it is OK to refer to these threshold characteristics when not using the "auto" ("heat_cool") mode. I would say it implied that you shouldn't But it doesn't explicitly ban that either. My hope is that if the characteristic is present we should just use it for all relevant modes over the "target temperature" one.

Ultimately i'm going to have to build a mock thermostat accessory and verify which characteristics are surfaced in the iPhone UI in which circumstances - but thats a lot more work than i have time to do right now.

@garyak
Copy link

garyak commented Oct 17, 2019

@GaryOkie are you able to modify temperature settings from HA? All I can do is On/Off and change modes. HA Current temp and Humidity values accurately reflect thermostat readings.

@GaryOkie
Copy link
Contributor

Hi @garyak - Yes, I'm able to modify temps from HA - at least according to the iComfort app which updates within several seconds of any HA change. I'm not at the remote S30 site to see it first hand, but the iComfort app must be showing the actual status.

@GaryOkie
Copy link
Contributor

GaryOkie commented Oct 18, 2019

@Jc2k - thanks for providing the link to the Apple Homekit docs.

I see in the official doc, that Current Temperature (IID 103) has an Apple-defined min/max range of 0-100 C, and this is exactly the characteristic that is being reported by the S30 and recorded in the pairing file.

format:"float","iid":103,"maxValue":100,"minStep":0.1,"minValue":0,"perms":["pr","ev"],"type":"11","unit":"celsius","value":21.5

Clearly Lennox hardcoded their min/max temp range to match the spec rather than pass the normal min/max ranges set on the thermostat. But why is the Homekit controller component ignoring the current temp range 0-100C and picking up the hardcoded trigger point ranges for the Auto Heat or sometimes Auto Cool instead?

This wouldn't be an issue for me except when the component sometimes decides to set the minimum heat to 60F.

EDIT - my apologies... I should have looked at TARGET Temperature (IID 104), not current temp. I see now the Apple-defined min/max range is 10-38C. The S30 originally passed a range of 15.5-37C (60-99F). However, I see that the S30 Heat state remains at min_temp: 40F and max_temp: 90F. (But that may be because I have changed from cool to heat mode).

format:"float","iid":104,"maxValue":37,"minStep":0.5,"minValue":15.5,"perms":["pr","pw","ev"],"type":"35","unit":"celsius","value":26.5,

@Jc2k
Copy link
Member

Jc2k commented Oct 18, 2019

Your edit is correct, we are currently using the target temperature min and max values.

My next step is to make a mock thermostat with homekit_python which has the target temperature, heating threshold and cooling threshold characteristics. I will give each of the 3 different min and max values. Will then pair with an iOS device and see which of the 3 min/max ranges it uses in each of the 3 operating modes. Hopefully this will show that iOS prefers the heating and cooling thresholds if they are present, regardless of operating mode. This will mean that we have a way forward.

If it shows my narrow interpration of the spec holds, and that the heating and cooling thresholds are only for heat_cool mode (i.e. auto mode) then we are somewhat stuck. I could potentially add a "force reload" service for the characteristics database. But I don't want to dwell on this until we have been able to verify the "nice fix" is a no go.

In the mean time if you delete your homekit_controller-entity-map and restart HA it should fetch the latest one. You could verify this does cause the target temperature to update (without a change to c#). Worth taking a copy just in case it doesn't, i guess.

@garyak
Copy link

garyak commented Oct 18, 2019

OK @GaryOkie . For Heat and Cool modes I can adjust temperature set points. For Heat/Cool, I cannot. I believe I understand now what you and @Jc2k are discussing.

@GaryOkie
Copy link
Contributor

GaryOkie commented Oct 18, 2019

That sounds great @Jc2k as a potential way forward! I am quite happy with the functionality I now have and greatly appreciate all the attention you have been giving to improve this component.

@garyak - I don't use the Auto (Heat/Cool) mode, but I'll do some testing with it.

EDIT: I see what you mean now - there isn't any way I've found to set the separate heat setpoint and the cool setpoint for heat/cool mode. I currently use the simple-thermostat lovelace card, but it has the same limitation as the official thermostat card. Even though both provide a heat/cool button, there is only a single setpoint.

Now, there is a dual-thermostat card that has a nice dual setpoint capability, and it works just like the iComfort app round slider. I tried using it, but it seems to require two separate entities for heat and cool. Giving it the same S30 entity for both heat/cool didn't work. This has to be a common issue, but I haven't found a solution yet.

@ccormier
Copy link

I can also confirm successful pairings and retrieval of the accessory list after updating the homekit firmware on the S30 on HA release code.

@GaryOkie
Copy link
Contributor

GaryOkie commented Nov 7, 2019

Shouldn't it be possible to set the hvac_action attribute as IDLE, instead of OFF, when the HVAC unit is not actually turned off?

@garyak
Copy link

garyak commented Nov 7, 2019

Another question. Are the Fan settings exposed by the API?

@GaryOkie
Copy link
Contributor

GaryOkie commented Nov 7, 2019

Nope, looking through the Apple Homekit HAP R2 non-commercial spec, there are no references to Fan settings or status for the Thermostat accessory.

@garyak
Copy link

garyak commented Nov 7, 2019

Do without I guess. Thanks for checking.

@GaryOkie
Copy link
Contributor

GaryOkie commented Nov 7, 2019

@Jc2k -
The Homekit "Current Heating/Cooling State" shows possible values of  0=Off; 1=Heat is on; 2=Cooling is on. I think that in this context, "Off" really means the hvac_action should be "Idle".

The "Target Heating/Cooling State" of 0=OFF is what is used to turn off the unit (hvac_mode). I don't see any other setting that can indicate if the unit is actually off, but it would probably be best to set hvac_action to off based on this target 0 value.

@Jc2k
Copy link
Member

Jc2k commented Nov 7, 2019

We should probably take any new issues to new tickets from now on guys, the pairing issue is resolved by the firmware (no one has said otherwise yet) and i'm going to lose track of what else is and isn't resolved. (Feel free to @ me directly for homekit stuff).

I think you are probably right about this @GaryOkie. I don't have any spare cycles right now but if you were to submit a PR to update the mapping here i would be able to review it.

I'm fine with simply replacing CURRENT_HVAC_OFF with CURRENT_HVAC_IDLE, i'm less certain about inferring the hvac_action is CURRENT_HVAC_OFF when the hvac_mode is HVAC_MODE_OFF. I think i'd prefer to not do that now, unless it is causing a reduction in functionality in the Home Assistant UI or someone demonstrates that's what the official HomeKit app is doing for its UI.

@GaryOkie
Copy link
Contributor

GaryOkie commented Nov 7, 2019

Thanks - I've only done a PR on documentation before, not yet for code, but will give it a shot if it's just a simple matter of remapping.

The most important thing is to define hvac_action as idle (or heating, or cooling) when the unit is on, and off otherwise. The thermostat operation state is working properly and shows off, when the unit is off, and heat/cool/heat_cool when on.

EDIT: Another thermostat component I tried returned Hvac_action to Idle (not off) when the system was turned off. So yeah, this simple remapping of 0=CURRENT_HVAC_IDLE should suffice.

And I agree - this original E30 pairing issue should be closed, with the advice that the latest firmware needs to be installed.

@Jc2k
Copy link
Member

Jc2k commented Jan 2, 2020

I'm going to close this because pairing now seems to be working.

If you are waiting for support the temperature ranges in heat/cool mode, you can track this ticket.

Cheers

@Jc2k Jc2k closed this as completed Jan 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

9 participants