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

WIP: Add Intergas InComfort Lan2RF gateway #23591

Closed
wants to merge 27 commits into from

Conversation

@zxdavb
Copy link
Contributor

commented May 1, 2019

Description:

Adds the Intergas InTouch Lan2RF gateway.

Pull request in home-assistant.io with documentation (if applicable): home-assistant/home-assistant.io#<home-assistant.io PR number goes here>

Example entry for configuration.yaml (if applicable):

incomfort:
  host: 192.168.0.15

Checklist:

  • The code change is tested and works locally.
  • Local tests pass with tox. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.

If user exposed functionality or configuration variables are added/changed:

If the code communicates with devices, web services, or third-party tools:

  • The manifest file has all fields filled out correctly (example).
  • New dependencies have been added to requirements in the manifest (example).
  • New or updated dependencies have been added to requirements_all.txt by running script/gen_requirements_all.py.
  • New files were added to .coveragerc.

If the code does not interact with devices: N/A

zxdavb added 11 commits Apr 29, 2019
@royduin

This comment has been minimized.

Copy link
Contributor

commented May 1, 2019

Thanks for your work! InTouch is the name of the new thermostat? InComfort is the old one? Or was the Android/iOS app called InTouch? Both are using the same gateway? Which name should we use? Pretty confusing 🤯

I've created and I'm maintaining a custom component for InComfort which is currently used by most of the Home Assistant and InComfort users: https://github.com/royduin/home-assistant-incomfort. How does this differ from your implementation? Besides the separation of InComfort specific code to a Python library?

@zxdavb

This comment has been minimized.

Copy link
Contributor Author

commented May 1, 2019

Hi @royduin, thanks for your good work with your custom component.

InTouch is the name of the new thermostat? InComfort is the old one?

This integration is, strictly-speaking, for the Lan2RF gateway and not just it's (first) room thermostat.

Each heater connected to the gateway (the code assumes only 1 heater) may have 0, 1 or 2 room thermostats connected to it (I have none, but I imagine most people will have 1).

Thus, this integration has a water_heater, several binary_sensors, some sensors, and 0-2 climate entities.

Note: I do not expect that HA will accept all four entity types in this initial PR (as is their usual fashion), but all will be added over time.

I thought long and hard whether to call the integration lan2rf, intouch or incomfort (or even intergas), and intouch won. The thermostats are simply called 'Room 1', and 'Room 2'. You can rename the entity to 'InComfort', or anything you like, using customize.yaml or such.

I've created and I'm maintaining a custom component for InComfort which... How does this differ from your implementation?

I've seen your custom component, and had a good look at it. There are some big differences between the two, including:

  • this integration was written to be accepted into HA & should satisfy their (understandably strict) requirements, alongside the benefits that will bring
  • the underlying client API uses asyncio (aiohttp), yours uses syncio (requests)
  • it exposes all the heater data in a HA-way, for example: a binary_sensor for the failed state, with device state attributes for the fault_code (e.g. a sensor fault)
  • it is implementation-neutral: system v combi boiler, 0 v 1 room thermostat, etc.
  • it addresses some minor bugs, and is more robust in other ways

I could have taken your code and sent PRs to you to achieve the above but, having tried to do such before, it is just easier if I start from scratch (this integration took just a couple of evening's work - all the heavy lifting is in the client).

I also accept that internationalization would be useful, but that will have to wait until after the first PRs are accepted.

Would you be able to test it at all? You'll appreciate how useful feedback is to catch/squash bugs.

zxdavb added 6 commits May 1, 2019
@royduin

This comment has been minimized.

Copy link
Contributor

commented May 1, 2019

Thanks for the information! I think incomfort is a better name, when I login to the webinterface I don't see intouch anywhere:

gateway

What is the easiest way to test your code from a Docker container?

@zxdavb

This comment has been minimized.

Copy link
Contributor Author

commented May 1, 2019

What is the easiest way to test your code from a Docker container?

I dunno! I use VMware ESX VMs (and occasionally LXC containers): follow these instructions, and use my git instead of HAs: https://github.com/zxdavb/home-assistant

... and git checkout add_intergas_intouch before you start hass.

Thanks for the information! I think incomfort is a better name, when I login to the webinterface I don't see intouch anywhere:

Hmmm... Mine is 'InTouch':

Untitled

I had a good look around, and didn't see any consensus. I've even seen it called Comfort Touch! What is the name of the app are you using?

@royduin

This comment has been minimized.

Copy link
Contributor

commented May 1, 2019

I had a good look around, and didn't see any consensus. I've even seen it called Comfort Touch! What is the name of the app are you using?

I don't use the app anymore since my Home Assistant integration 🤪but the app is called "Intergas" on my Android with "incomfort" in the logo. But it's named "incomfort" in the Play store: https://play.google.com/store/apps/details?id=com.elninoict.IntergasAndroid, no results for "intouch": https://play.google.com/store/search?q=intouch&c=apps but there is another "Intergas Comfort Touch" app: https://play.google.com/store/apps/details?id=com.intergas.comforttouch which is a new product? It looks like a thermostat with the gateway build in.

@zxdavb

This comment has been minimized.

Copy link
Contributor Author

commented May 1, 2019

I don't use the app anymore since my Home Assistant integration 🤪but the app is called "Intergas" on my Android with "incomfort" in the logo.

My play store app is called InTouch: https://play.google.com/store/apps/details?id=com.elninoict.IntouchAndroid

However, I am inclined to rename to InComfort, as I am guessing it is InTouch in UK, and InComfort in EU, and there will be many more EU users,

My gateway looks exactly like this one, but I am not using the Honeywell thermostat.

@royduin

This comment has been minimized.

Copy link
Contributor

commented May 1, 2019

I didn't know it was sold and used outside the Netherlands. I though it was a Dutch product. Maybe we've to ask Intergas for information about which name is used more or where the most of the gateways are sold. Mine looks exactly the same.

Edit: I've emailed Intergas. To be continued.

@NdS-Research-Facilities

This comment has been minimized.

Copy link

commented May 5, 2019

OK, assuming that your configuration file is correctly formatted, then we can go no further without me making some changes to the code.

This would be a small change but, I am doing all this from my smartphone, won't be home until about 18 hours from now.

I will make changes to the component 2 significantly increase the login and we can go from there if you like.

We're on the verge of greatness!

I think you are, if you need any test system, I'm here, take care

@zxdavb

This comment has been minimized.

Copy link
Contributor Author

commented May 6, 2019

@NdS-Research-Facilities I have identified the issue.

Could you tell me which cURL command returns your heater list?

Mine is:

curl -X GET http://${HOSTNAME}/heaterlist.json

But yours may use a different URL, or require authentication (or both, or neither):

curl --user ${USER}:${PASS} -X GET http://${HOSTNAME}/protect/heaterlist.json
@zxdavb

This comment has been minimized.

Copy link
Contributor Author

commented May 6, 2019

... and this in your configuration.yaml may be a workaround:

  hostname: lan2rf.desmidt.org/protect
@royduin

This comment has been minimized.

Copy link
Contributor

commented May 6, 2019

The new gateway with authentication uses indeed a different url, for reference see how I've implemented support for the new gateway in my custom component: royduin/home-assistant-incomfort@12e1369

@zxdavb

This comment has been minimized.

Copy link
Contributor Author

commented May 6, 2019

Roy, I don't think your code GETs heaterlist.json?

@royduin

This comment has been minimized.

Copy link
Contributor

commented May 6, 2019

Roy, I don't think your code GETs heaterlist.json?

That's true because probably 99% only got one heater so the ID is 0. Haven't heard anyone who has multiple.

@zxdavb

This comment has been minimized.

Copy link
Contributor Author

commented May 6, 2019

Probably closer 99.7%, but the client library was written for that, and also for any of 0, 1 or 2 room thermostats.

@NdS-Research-Facilities

This comment has been minimized.

Copy link

commented May 6, 2019

@NdS-Research-Facilities I have identified the issue.

Could you tell me which cURL command returns your heater list?

Mine is:

curl -X GET http://${HOSTNAME}/heaterlist.json

But yours may use a different URL, or require authentication (or both, or neither):

curl --user ${USER}:${PASS} -X GET http://${HOSTNAME}/protect/heaterlist.json
newyork-180:~ via2326$ curl --user admin:ho9619pi -X GET http://lan2rf.desmidt.org/protect//heaterlist.json
404: File not found<br>Use <a href="/mpfsupload">MPFS Upload</a> to program web pages
newyork-180:~ via2326$ curl -X GET http://lan2rf.desmidt.org/protect//heaterlist.json
401 Unauthorized: Password required
newyork-180:~ via2326$ 
@NdS-Research-Facilities

This comment has been minimized.

Copy link

commented May 6, 2019

lan2rf.desmidt.org/protect

seems to work

image

@zxdavb

This comment has been minimized.

Copy link
Contributor Author

commented May 6, 2019

@NdS-Research-Facilities

The cURL fail is because the double slash should be a single slash? protect//heaterlist.json

@zxdavb

This comment has been minimized.

Copy link
Contributor Author

commented May 6, 2019

@NdS-Research-Facilities

The bug is in the client api (and exception reporting should be improved) so will push these out soon enough. When that happens, you'll need to change it back to hostname: lan2rf.desmidt.org (without the /protect).

Meantime, you will have full functionality - there should be a water_heater (which you have shown me), some sensors, binary_sensors and a climate device (if you have one).

@NdS-Research-Facilities

This comment has been minimized.

Copy link

commented May 6, 2019

@NdS-Research-Facilities

The bug is in the client (and exception reporting should be improved) so will push these out soon enough.

Meantime, you will have full functionality - there should be a water_heater (which you have shown me), some sensors, binary_sensors and a climate device (if you have one).

Looking good

image

@NdS-Research-Facilities

This comment has been minimized.

Copy link

commented May 6, 2019

@NdS-Research-Facilities

The cURL fail is because the double slash should be a single slash? protect//heaterlist.json

This better

newyork-180:~ via2326$ curl --user admin:greatsecretpassword -X GET http://lan2rf.desmidt.org/protect/heaterlist.json
{"heaterlist":["1802f02483",null,null,null,null,null,null,null]}

1802f02483 = the heater serial#

@zxdavb

This comment has been minimized.

Copy link
Contributor Author

commented May 6, 2019

@NdS-Research-Facilities

Looking good

How many room thermostats do you have? Currently, the code will show two thermostats no matter what, but will remove inactive thermostats (temp = 327.7) in future.

@zxdavb

This comment has been minimized.

Copy link
Contributor Author

commented May 6, 2019

This is the states UI (not lovelace)

Untitled

@zxdavb

This comment has been minimized.

Copy link
Contributor Author

commented May 6, 2019

Anyway, have a play with it & let me know if you find any more bugs.

@NdS-Research-Facilities

This comment has been minimized.

Copy link

commented May 6, 2019

@NdS-Research-Facilities

Looking good

How many room thermostats do you have? Currently, the code will show two thermostats no matter what, but will remove inactive thermostats (temp = 327.7) in future.

Currently I have a manual (old style) wired thermostat, but would like to control heating through HA.
I will receive several temp sensors shortly so that would be my room input.
I have Danfoss BLE radiator valves which also measure room temperature.
I'm hoping to put all those together and control heating (burning) in a more efficient fashion

zxdavb added 3 commits May 6, 2019

@zxdavb zxdavb changed the title Add Intergas InComfort Lan2RF gateway WIP: Add Intergas InComfort Lan2RF gateway May 7, 2019

@zxdavb zxdavb referenced this pull request May 7, 2019
8 of 8 tasks complete
@zxdavb

This comment has been minimized.

Copy link
Contributor Author

commented May 26, 2019

This PR has been replaced by PRs #23736, #23830, and (WIP) #23812.

@zxdavb zxdavb closed this May 26, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.