Add Water Heater platform to MQTT integration

[hookedonunix]

Proposed change

An addition to the existing MQTT component that allows support for the water heater entity. The code lends heavily from the MQTT climate integration, as the functionality is mostly a simplified version of it.

I'm building this to communicate with a desktop vaporizer I own, which I've been able to test it successfully with. It's built to the spec of the base water heater component but I don't own an actual smart water heater to test it with, so some functionality I've just tested with mocks. This PR is paired with this one, which will allow for the temperature of water heater entities to be controllable through the google assistant component.

Type of change

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

Additional information

• Link to documentation pull request: https://github.com/home-assistant/home-assistant.io/pull/27549

I've never contributed to the climate entity (or hass) before, so all my knowledge is assumptions based on the code. With that in mind, I have a couple questions that have come up as I've been working on it:

• The ATTR_TEMP_HIGH and ATTR_TEMP_LOW attributes are in both the climate and water heater entities, which seem to control the target temperature range, if applicable. They seem to be implemented in the climate entity, but I can't see any way of setting them in the water heater entity, are these actually used or were they never implemented for the water heater? A test I saw for the water heater only accepted a single temperature instead of a range.
• The power and away topics and commands have been deprecated from the mqtt climate integration. I've not included power, but did include the away topic, as the base water heater entity has them and from what I could find the reason for the removal wasn't relevant for the water heater. If someone who knows more could advise I would appreciate it though!

Checklist

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

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

• [x] Documentation added/updated for www.home-assistant.io

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

• [x] The manifest file has all fields filled out correctly.
  Updated and included derived files by running: python3 -m script.hassfest.
• [x] New or updated dependencies have been added to requirements_all.txt.
  Updated by running python3 -m script.gen_requirements_all.
• [x] For the updated dependencies - a link to the changelog, or at minimum a diff between library versions is added to the PR description.
• [x] Untested files have been added to .coveragerc.

To help with the load of incoming pull requests:

• [ ] I have reviewed two other open pull requests in this repository.

[home-assistant[bot]]

Hi @hookedonunix

It seems you haven't yet signed a CLA. Please do so here.

Once you do that we will be able to review and accept this pull request.

Thanks!

[home-assistant[bot]]

Hey there @emontnemery, @jbouwh, mind taking a look at this pull request as it has been labeled with an integration (mqtt) you are listed as a code owner for? Thanks!

Code owner commands

Code owners of mqtt can trigger bot actions by commenting:

• @home-assistant close Closes the pull request.
• @home-assistant rename Awesome new title Renames the pull request.
• @home-assistant reopen Reopen the pull request.
• @home-assistant unassign mqtt Removes the current integration label and assignees on the pull request, add the integration domain after the command.

[home-assistant[bot]]

Please take a look at the requested changes, and use the Ready for review button when you are done, thanks :+1:

Learn more about our pull request process.

[hookedonunix]

@jbouwh I'm using this to communicate with the Volcano Hybrid which is a desktop vaporizer that exposes bluetooth. I've adapted a Bluetooth to MQTT package to keep most of the logic out of Home Assistant, but have been trying to find the best way to integrate it. I started with MQTT Climate but was having trouble getting Google Assistant working as they place a limit on the temperature setting (the device operates from 40C to 230C), so the water heater entity seemed like the best fit. I really only need current temp (topic) and target temp (command and topic) as I'm controlling the heat and fan with separate switches, but I imagine mode might be useful for other people using the water heater component.

I wrote it with "batteries included" but agree it's definitely over-complicated, I think the minimum you could get it down to would be something like the TemperatureControl trait, which exposes:

• Min temp
• Max temp
• Current temp
• Target temp
• Temperature unit
• Precision (maybe?)

To remove some of the clutter I'll take out the temp high/low and away mode, as I don't think either are needed. Let me know if you have any other recommendations, thank you!

To answer your last question I can see a few people expressing interest a couple years ago, as well as a Tasmota OpenTherm module. I'll have a look through that now.

[jbouwh]

Thanx that helps.

[jbouwh]

May be we can somehow integrate the two classes in one. If we keep the the platform setup we could easily implement an common class that implements the shared methods. We should only take out the not shared elements.

[hookedonunix]

I like the sound of that, like class MqttTemperatureEntity(MqttEntity): for the parent and class MqttClimateEntity(MqttTemperatureEntity, ClimateEntity): for the child as an example?

[jbouwh]

Yeah,

I like the sound of that, like class MqttTemperatureEntity(MqttEntity): for the parent and class MqttClimateEntity(MqttTemperatureEntity, ClimateEntity): for the child as an example?

That is the idea. So we can combine all common functionality into MqttTemperatureEntity

[jbouwh]

We probably could do this better in a follow up PR I guess

[jbouwh]

@hookedonunix I have opened https://github.com/home-assistant/core/pull/93751 to do some rework on the climate entity class. I asked @emontnemery to have a look at it, The idea is to let the shared MqttTemperatureControlEntity class control temperature, handle all subscriptions (for both platforms) and handle incoming messages. All other specific stuff stays in MqttClimate or MqtWaterHeater. Please feel free to leave a review.

[jbouwh]

@hookedonunix Can you rebase and use the new code from climate.py as #93751 is merged now?

[hookedonunix]

Apologies for the delay, I've made the changes requested above and refactored the MqttWaterHeater class to use the MqttTemperatureControlEntity base. I had to add a key check for self._topic in add_subscription to stop the high and low target temps from throwing an error.

Additionally as HVAC mode is typed to climate, I wasn't sure how best to allow the operation_mode for water heater to be set in the async_set_temperature function. Currently I have both setters as abstract on the MqttTemperatureControlEntity class, but it might be better to allow both climate and water heater to implement the async_set_temperature separately so they can set their respective operation mode then call a parent function to handle the self._set_climate_attribute and async_write_ha_state.

I'm going to take another look everything to see if anything needs cleaning up, or if anymore tests can be made common. Coverage looks okay now.

[hookedonunix]

Before merging I'd like to do some testing with it first.

Sounds great, I'm going to build an image and run it on my home configuration to give it a try.

After that I could also help finish the google assistant PR you made.

That would be fantastic, it should hopefully be a lot more straightforward.

[jbouwh]

Please have a look at https://github.com/home-assistant/core/pull/93965 as well. Since the initial values are in degrees F this gives issues for users that use degrees C as temperature unit. This seems to be an issue for MQTT climate as well. But there the default unit is degrees Fahrenheit. Not sure why this has been implemented differently BTW.

[jbouwh]

@jbouwh Does the same apply for temp initial as with temp min/max?

I believe so

[jbouwh]

Nice. So the initial and min temp is 110 °F, max TEMP is 140 °F. When the unit is Celsius this is 43.3 °C and 60 °C max. which is fine I guess. Let us just document this correctly.

I'd like to have the fix for Climate to be merged first as this also influences this PR.

BTW When you rebase with dev please do a Squash push. Or if you use the command line:

git fetch upstream
git rebase upstream/dev
git push --force

[hookedonunix]

I'd like to have the fix for Climate to be merged first as this also influences this PR.

I think solving all the climate changes beforehand is a good idea.

When you rebase with dev please do a Squash push

Sure thing!

[jbouwh]

@hookedonunix can you do another rebase with dev, as #93965 is merged now.

[hookedonunix]

@jbouwh It's been good fun, thank you so much for all your help! The build has been running well for the past couple days, I'll keep an eye on it though

[jbouwh]

@jbouwh It's been good fun, thank you so much for all your help! The build has been running well for the past couple days, I'll keep an eye on it though

Thank you too! after this has been merged we will pick up the google assistant PR.

[hookedonunix]

@jbouwh Thanks for fixing those conflicts, LGTM

[hookedonunix]

Cheers! 🎉