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

Add Intergas InComfort Lan2RF gateway #23736

Merged
merged 40 commits into from May 7, 2019
Merged
Changes from 1 commit
Commits
Show all changes
40 commits
Select commit Hold shift + click to select a range
d98ef86
fixed __init__.py
zxdavb Apr 29, 2019
21bacd9
add sensors
zxdavb Apr 30, 2019
efb697f
switch to parent-child architecture
zxdavb Apr 30, 2019
1f5ceec
make binary_sensor the parent
zxdavb May 1, 2019
acf823a
revert parent - to water_heater
zxdavb May 1, 2019
9ee6a78
first working version
zxdavb May 1, 2019
8f33f91
working, delinted (except TODO)
zxdavb May 1, 2019
045cd30
update to latest client library
zxdavb May 1, 2019
590c53f
remove debug code
zxdavb May 1, 2019
5f68d26
delint
zxdavb May 1, 2019
b22a8a3
Merge remote-tracking branch 'upstream/dev' into add_intergas_intouch
zxdavb May 1, 2019
521f0f2
tweak device_state_attributes
zxdavb May 1, 2019
c0adc37
tweak device_state_attrbutes
zxdavb May 1, 2019
97fc989
minor tweaks
zxdavb May 1, 2019
40a1cef
bump client library for bugfix
zxdavb May 1, 2019
dc6de80
improve state attributes for pumping
zxdavb May 1, 2019
6304809
update .coverage
zxdavb May 1, 2019
d1845a1
bugfix - binary sensors not updating
zxdavb May 1, 2019
cfc2827
rename to incomfort from intouch
zxdavb May 2, 2019
691a4f5
fix coveragerc regression
zxdavb May 6, 2019
deaf2b0
Merge remote-tracking branch 'upstream/dev' into add_intergas_intouch
zxdavb May 6, 2019
c21f080
Merge remote-tracking branch 'upstream/dev' into add_intergas_intouch
zxdavb May 6, 2019
72c0532
bump client (bugfix for /protected URL)
zxdavb May 6, 2019
228dc67
bump client (bugfix for /protected URL) 2
zxdavb May 7, 2019
ac1e428
bump client
zxdavb May 7, 2019
2fac73e
remove debug code
zxdavb May 7, 2019
5a380a2
ready for PR
zxdavb May 7, 2019
afcd0de
fix regression
zxdavb May 7, 2019
3dd5391
use xx._boiler instead of xx._objref
zxdavb May 7, 2019
e9eefc2
improve current_temperature and delint
zxdavb May 7, 2019
5978615
use current_operation instead of state
zxdavb May 7, 2019
f7ab847
refactor vol.Schema
zxdavb May 7, 2019
59ef9f4
remove unneeded instance attribute
zxdavb May 7, 2019
ec43141
remove unneeded instance attribute 2
zxdavb May 7, 2019
3e4d6ab
refactor device_state_attributes
zxdavb May 7, 2019
97786c3
change 'boiler' to 'heater'
zxdavb May 7, 2019
79212f5
change 'boiler' to 'heater' 2
zxdavb May 7, 2019
7607be9
correct a typo
zxdavb May 7, 2019
05388b7
bugfix: add missing comma
zxdavb May 7, 2019
895ddd3
small tidy-up
zxdavb May 7, 2019
File filter...
Filter file types
Jump to…
Jump to file or symbol
Failed to load files and symbols.

Always

Just for now

use xx._boiler instead of xx._objref

  • Loading branch information...
zxdavb committed May 7, 2019
commit 3dd53913abe1c2e253ea9ecc9dddde532b625554
@@ -31,7 +31,7 @@ class IncomfortWaterHeater(WaterHeaterDevice):
def __init__(self, client, boiler):
"""Initialize the water_heater device."""
self._client = client
self._objref = boiler
self._boiler = boiler
self._name = 'Boiler'
This conversation was marked as resolved by zxdavb

This comment has been minimized.

Copy link
@MartinHjelmare

MartinHjelmare May 7, 2019

Member

Boiler can be extracted to a module level constant.

This comment has been minimized.

Copy link
@zxdavb

zxdavb May 7, 2019

Author Contributor
self._name = BOILER_NAME

This comment has been minimized.

Copy link
@MartinHjelmare

MartinHjelmare May 7, 2019

Member

Nah, just return BOILER_NAME directly in the name property.

This comment has been minimized.

Copy link
@zxdavb

zxdavb May 7, 2019

Author Contributor

Sorry, I can see now how it's stupid idea - I think this is one of those things that will make more sense when the other components/platforms are added to the integration.

Anyway, will do as you suggest.


@property
@@ -44,14 +44,14 @@ def device_state_attributes(self):
"""Return the device state attributes."""
keys = ['nodenr', 'rf_message_rssi', 'rfstatus_cntr', 'room_1',
'room_2']
state = {k: self._objref.status[k]
for k in self._objref.status if k not in keys}
state = {k: self._boiler.status[k]
for k in self._boiler.status if k not in keys}
This conversation was marked as resolved by zxdavb

This comment has been minimized.

Copy link
@MartinHjelmare

MartinHjelmare May 7, 2019

Member

What items will be included?

This comment has been minimized.

Copy link
@zxdavb

zxdavb May 7, 2019

Author Contributor

Here's an example:

{'display_code': 126, 'display_text': 'standby', 'fault_code': 0, 'is_burning': False, 'is_failed': False, 'is_pumping': False, 'is_tapping': False, 'heater_temp': 57.4, 'tap_temp': 50.61, 'pressure': 1.29, 'serial_no': '179t23082'}

I used not in because it was shorter. Can change to in if you require.

This comment has been minimized.

Copy link
@MartinHjelmare

MartinHjelmare May 7, 2019

Member

It's good to be able to see what attributes we report as state so I would recommend that.

Serial no is not allowed in state attributes. We only want dynamic info that is related to the state of the device. Serial no can go into device info when there is a config entry for the integration (not at this point).

This comment has been minimized.

Copy link
@zxdavb

zxdavb May 7, 2019

Author Contributor

changed to in HEATER_ATTRS, after removing serial number.

return {'status': state}
This conversation was marked as resolved by zxdavb

This comment has been minimized.

Copy link
@MartinHjelmare

MartinHjelmare May 7, 2019

Member

Why should we nest the items under the 'status' key?

This comment has been minimized.

Copy link
@zxdavb

zxdavb May 7, 2019

Author Contributor

I can't think of an example with this specific component, but in other cases (e.g. evohome), I have had distinct pieces of data, but with the same key, e.g.:

{
  "status_v1": {"temperature": 25.3},
  "status_v2": {"temperature": 32.33}
}

Doing it this way let's me (us) extend the device state attributes schema in future without have to rename keys, e.g.:

{
  "temperature": 25.3
}

... becomes:

{
  "temperature": 25.3,
  "temperature_v2": 32.33
}

This comment has been minimized.

Copy link
@MartinHjelmare

MartinHjelmare May 7, 2019

Member

Do you think that is probable? We should prefer flat over nested if there's no specific need for the nesting.

This comment has been minimized.

Copy link
@zxdavb

zxdavb May 7, 2019

Author Contributor

I have written 3 integrations: evohome, geniushub and incomfort. The first two are both hierarchical, and are both likely to be extended in the future (I have plans for this).

However, this one is flat, and I doubt it will be extended in the future, so will remove the nesting.


@property
def current_temperature(self):
"""Return the current temperature."""
return self._objref.heater_temp # or: self._objref.tap_temp?
return self._boiler.heater_temp # or: self._boiler.tap_temp?
This conversation was marked as resolved by zxdavb

This comment has been minimized.

Copy link
@MartinHjelmare

MartinHjelmare May 7, 2019

Member

What does the comment ask?

This comment has been minimized.

Copy link
@zxdavb

zxdavb May 7, 2019

Author Contributor

This is better:

        if self._boiler.is_tapping:
            return self._boiler.tap_temp
        return self._boiler.heater_temp

@property
def min_temp(self):
@@ -76,10 +76,10 @@ def supported_features(self):
@property
def state(self):
This conversation was marked as resolved by zxdavb

This comment has been minimized.

Copy link
@MartinHjelmare

MartinHjelmare May 7, 2019

Member

Don't overwrite state property. That's the responsibility of the base water_heater entity.

This comment has been minimized.

Copy link
@zxdavb

zxdavb May 7, 2019

Author Contributor

Oops, changed to:

    def current_operation(self):
"""Return the current operation mode."""
if self._objref.is_failed:
return "Failed ({})".format(self._objref.fault_code)
if self._boiler.is_failed:
return "Failed ({})".format(self._boiler.fault_code)

return self._objref.display_text
return self._boiler.display_text

@property
def should_poll(self) -> bool:
@@ -89,7 +89,7 @@ def should_poll(self) -> bool:
async def async_update(self):
"""Get the latest state data from the gateway."""
try:
await self._objref.update()
await self._boiler.update()

except (AssertionError, asyncio.TimeoutError) as err:
_LOGGER.warning("Update for %s failed, message: %s",
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.