Skip to content

Fix Plugwise climate issues#31209

Merged
pvizeli merged 2 commits intohome-assistant:devfrom
bouwew:plugwise-update-2
Jan 29, 2020
Merged

Fix Plugwise climate issues#31209
pvizeli merged 2 commits intohome-assistant:devfrom
bouwew:plugwise-update-2

Conversation

@bouwew
Copy link
Copy Markdown
Contributor

@bouwew bouwew commented Jan 27, 2020

Breaking change

Proposed change

Bump Haanna to 0.14.1, fixing the reported Home Assistant issues:
fixes #28815
fixes #28947
fixes #30714
and the error reported in haanna issue fixes #14.
Also, the proposed fix for the issue reported by fwestenberg, related to 2 Annas in one HA system, is included.

Update to climate.py, fixes #28947: add detection of the heating_state for on/off-type heaters via the boiler_state parameter.

Type of change

  • Dependency upgrade
  • Bugfix (non-breaking change which fixes an issue)
  • New integration (thank you!)
  • New feature (which adds functionality to an existing integration)
  • Breaking change (fix/feature causing existing functionality to break)
  • Code quality improvements to existing code or addition of tests

Example entry for configuration.yaml:

# Example configuration.yaml

Additional information

Checklist

  • The code change is tested and works locally.
  • Local tests pass. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.
  • I have followed the development checklist
  • The code has been formatted using Black (black --fast homeassistant tests)
  • Tests have been added to verify that the new code works.

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.
    Updated and included derived files by running: python3 -m script.hassfest.
  • New or updated dependencies have been added to requirements_all.txt.
    Updated by running python3 -m script.gen_requirements_all.
  • Untested files have been added to .coveragerc.

The integration reached or maintains the following Integration Quality Scale:

  • No score or internal
  • 🥈 Silver
  • 🥇 Gold
  • 🏆 Platinum

@probot-home-assistant
Copy link
Copy Markdown

Hey there @laetificat, @CoMPaTech, mind taking a look at this pull request as its been labeled with a integration (plugwise) you are listed as a codeowner for? Thanks!

Comment thread homeassistant/components/plugwise/climate.py Outdated
Copy link
Copy Markdown
Member

@MartinHjelmare MartinHjelmare left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

Copy link
Copy Markdown
Member

@pvizeli pvizeli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe you should offload boiler to the right component

@pvizeli pvizeli merged commit 61a8618 into home-assistant:dev Jan 29, 2020
@bouwew
Copy link
Copy Markdown
Contributor Author

bouwew commented Jan 29, 2020

@pvizeli
I added the boiler_status parameter because it represents the heating-device-state when there is an on/off-connection between the Plugwise gateway and the heating-device.
When there is a heating-device connected via an OpenTherm-connection, the heating-device-state is represented by the heating_status parameter.

I think you are referring to boiler = hot water heater. For this Plugwise uses the term domestic_hot_water_state, I had this in my climate.py-code before but was asked to remove it :)
Now I have put it back, but only to make sure that the hvac_action in not showing idle when hot water is being tapped. Several users reported this as a bug (hvac_action = idle when tapping hot water, instead of showing heating).

@Mariusthvdb
Copy link
Copy Markdown
Contributor

Just as a +1 for the addition of boiler status. Please add to the Plugwise indeed.

@bouwew
Copy link
Copy Markdown
Contributor Author

bouwew commented Jan 29, 2020

@Mariusthvdb
Don't worry, it's there like in the Dev-code.

@Mariusthvdb
Copy link
Copy Markdown
Contributor

Mariusthvdb commented Jan 29, 2020

Great, thanks! Do we get the tap icon too now? The interface would benefit from that to indicate the heater is heating for hot water... just like the original Plugwise interface

67FBDDB5-DA3D-41DD-82EB-D273741EA340

@bouwew
Copy link
Copy Markdown
Contributor Author

bouwew commented Jan 29, 2020 via email

@Mariusthvdb
Copy link
Copy Markdown
Contributor

Mariusthvdb commented Jan 29, 2020

thanks,

it might be confusing though we when see the heating icon (which is like the icon on the Plugwise Anna) while missing the tap icon. One might think the heater is heating.....while in fact heating for hot water

so, who to talk to about the missing tap-icon on the thermostat card? Could we ping you @pvizeli for that? or @MartinHjelmare ?

@bouwew
Copy link
Copy Markdown
Contributor Author

bouwew commented Jan 29, 2020

@Mariusthvdb
The climate-card will never have a tapping icon, see here: https://developers.home-assistant.io/docs/en/entity_climate.html#hvac-modes
Also, please note, on the climate-card the schedule-icon relates to HVAC_MODE_AUTO and the flame-icon relates to the state HVAC_MODE_HEAT. The icons are related to the active MODE, and NOT to the active HVAC_ACTION, which shows the heater-device-state you are interested in: idle, heating.

As I've understood it, to show the tapping of hot water, we need to create an additional device, a water_heater: https://developers.home-assistant.io/docs/en/entity_water_heater.html. But this one is much less "developed" compared to the Climate device. And there is no card supporting it (someone please correct me if I'm wrong).

Also, to get a tap-icon, better to get this into the Simple Thermostat card, this one uses the HVAC_ACTION to show what is going on. But then the Simple Thermostat must also support the water_heater device. Maybe when I've added support for the Plugwise Adam, then we can get some help from an Adam-owner to realise the card that we all want...

@lock lock Bot locked and limited conversation to collaborators Jan 30, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Plugwise plugin fails with XML parse error Plugwise Anna not reporting heating Plugwise Anna integration update error

5 participants