Add support for the new ComfoConnect Pro#74
Conversation
|
Related to this discussion: #55 |
|
Thanks! I think this looks fine, odd that there is such a small difference between the two devices. Can you take a look at the lint issue? |
|
I agree it's odd that there is this difference between the two devices, but I suspect that they may have rewritten some of the networking inside the device, but were able to salvage much of the software of the old device? Or they just completely rebuild it using the existing spec, who knows. To be honest, I haven't tested the complete integration yet so I'm not sure if all the addresses are the same - but there's no reason for me to suspect they've changed. Linting issue has been fixed. |
|
Thanks! I do wonder if maybe the flow should be changed, because currently, we are waiting for a timeout to know that we should register. This can perhaps explain the timeout you are also seeing in Home Assistant. If you don't have a better idea, I'm okay with merging this. |
|
CI still fails. Can you also run black to format the code? poetry run black --check aiocomfoconnect/*.py |
|
The current flow for this fix is far from elegant, I agree. Obviously we could add a flag to the CLI interface but I don't like that and it would require the Home Assistant integration to add another option to the setup wizard. There is an option to do a version call before logging in. I've added the command to my branch, but I don't have a LAN C so I am unable to see the difference, could you run this? poetry run python -m aiocomfoconnect version --host <your host>On the Pro, this is the output: I've asked my digital junior developer for other options, this is the response:
However, I'm not very hopeful we can differentiate with only 2 devices. One other option is to call the HTTP endpoint that is exposed on port 80 on the device, and search the returned HTML for the text I should have fixed the linting and formatting issues btw. |
|
I might try to get a pcap from the Zehnder application to see how it is handling it later this week, but we could also accept this fix for now and perhaps later (well.... you know ;) ) find a better way. |
|
Using your version call times out on my Comfoconnect LAN C It's because you are using the same source UUID and destination UUID. Oddly enough, this then works fine on the ComfoConnect PRO? When I change your version code to be like this: async def run_version(host: str):
"""Request version information from the bridge (no registration required)."""
bridges = await discover_bridges(host)
if not bridges:
raise BridgeNotFoundException("No bridge found")
bridge = bridges[0]
await bridge.connect(DEFAULT_UUID)
try:
msg = await bridge.cmd_version_request()
print(f"gatewayVersion : {msg.gatewayVersion}")
print(f"serialNumber : {msg.serialNumber}")
print(f"comfoNetVersion : {msg.comfoNetVersion}")
finally:
await bridge.disconnect()It works, because the destination UUID is correct then. |
|
Also, calling the HTTP endpoint won't work, because the Comfoconect LAN C doesn't expose a HTTP server, and I don't like to try and wait for a timeout just to know what system you have. But maybe the Comfoconnect PRO sends a specifc UUID (because in your code, it seemed to accept the default uuid)? What do you see when you run |
|
If I run that command: |
|
I think this might be the key to recognize the new version. It seems like it is identifying itself with a new GatewayType in the SearchGatewayResponse. I think I'm going to rewrite this PR so we handle the registration flow slightly different for either device. |
|
Nice. What is the version of that response? On my device, it's 1. I think I don't have the gatewaytype, but I should check this. |
|
The protob file I use specifies this Unsure what season is, but is the comfoconnect pro maybe 2? It might make sense to extract a new protob file from the android apk. That's where I got the current version. Your raw string seems to end with 2. |
|
Yes, it's 2 indeed. But I think, looking at the current codebase, that this needs to be handled in main.py. The downside of that is that this also needs to be changed in the config flow of the homeassistant repository (https://github.com/michaelarnauts/home-assistant-comfoconnect/blob/master/custom_components/comfoconnect/config_flow.py). The downside is that this logic now resides in both repositories. I think it's up to you if you're ok with that, or if you want to somehow refactor this so the logic is inside the ComfoConnect class. If we fix it in main.py, I'd probably add a "is_pro" branch after the connect call (https://github.com/michaelarnauts/aiocomfoconnect/blob/master/aiocomfoconnect/__main__.py#L92). Edit: GatewayType would probably look like this: |
80c36d2 to
b64d45a
Compare
jvanhees
left a comment
There was a problem hiding this comment.
I've written a refactor that changes some of the function and introduces a new register method on the bridge. Could you test if this also works on the LAN C?
| if self.bridge_type != GATEWAY_TYPE_PRO: | ||
| # LAN C: check whether we are already registered by starting a session. | ||
| try: | ||
| await self.cmd_start_session(True) | ||
| return False # Already registered; session is now active. | ||
| except ComfoConnectNotAllowed: | ||
| pass # Not registered yet; fall through to register below. |
There was a problem hiding this comment.
If we are ok with changing the actual behaviour, we could get rid of this part. I'm not really sure how that would work on the LAN C tho.
| _LOGGER = logging.getLogger(__name__) | ||
|
|
||
| TIMEOUT = 5 | ||
| GATEWAY_TYPE_PRO = 2 |
There was a problem hiding this comment.
Might want to get this from the protobuf file.
I installed a new ComfoConnect Pro a few weeks ago, which seems to be replacing the previous version, and adds Wifi and other functionality that was in another connect box previously. The library didn't seem to properly connect to this new ComfoConnect Pro, even though it does support the same protocol. After some retrying, the library is able to connect, but since it initially fails the HACS integration doesn't really play nice. I used Claude to change the connection logic to also support this device, apparently it keeps the TCP connection open instead of disconnecting after a failed authentication, and just waits for a new registration.
I asked Claude to also add some test for this functionality, all other tests are still working. I do note that whenever I run the discover functionality the first time, it tends to not find the bridge. After running it a second time, it does run. Might need some more investigation but I suspect this won't be an issue with the Home Assistant integration.
Please let me know if you need anything changed or something else. Hopefully we can merge this, update the HACS integration as well with the new version and get the ComfoConnect Pro working in Home Assistant :) .