Add binary sensor for Vallox post heater

[viiru-]

When and whether the post heater activates is an important piece of
information when attempting to configure an optimal heating strategy.

Leave room for adding other binary sensors without needing to write more
code, there are a lot of things the Vallox devices report as binary
values.

Breaking change

Proposed change

Add binary sensor for Vallox post heater

When and whether the post heater activates is an important piece of information when attempting to configure an optimal heating strategy.

Leave room for adding other binary sensors without needing to write more code, there are a lot of things the Vallox devices report as binary values.

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

Additional information

• This PR fixes or closes issue: fixes #
• This PR is related to issue:
• Link to documentation pull request:

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:

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

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.
• For the updated dependencies - a link to the changelog, or at minimum a diff between library versions is added to the PR description.
• Untested files have been added to .coveragerc.

The integration reached or maintains the following Integration Quality Scale:

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

To help with the load of incoming pull requests:

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

[probot-home-assistant]

Hey there @andre-richter, mind taking a look at this pull request as it has been labeled with an integration (vallox) you are listed as a code owner for? Thanks!
_{^{(message by CodeOwnersMention)}}

[andre-richter]

In sensor.py there is the case of the profile sensor which doesn’t need a metric key, because we have to query using get_profile().

As long as we don’t have a similar case for binary sensors, we could remove the None here, which simplifies the is_on() a bit.

[viiru-]

Mypy doesn't accept that: error: Attributes without a default cannot follow attributes with one (this comes from the attributes defined in the superclass EntityDescription)

[andre-richter]

I think this should do the trick:

core/homeassistant/components/zwave_js/binary_sensor.py

Lines 61 to 72 in 1bd1a12

	@dataclass
	class PropertyZWaveJSMixin:
	"""Represent the mixin for property sensor descriptions."""
	on_states: tuple[str, ...]
	@dataclass
	class PropertyZWaveJSEntityDescription(
	BinarySensorEntityDescription, PropertyZWaveJSMixin
	):
	"""Represent the entity description for property name sensors."""

[viiru-]

I'm not sure what you mean. The implementation in zwave_js seems quite a bit more complex as it allows multiple different values that mean "on" depending on sensor. As far as I can tell the Vallox API always uses 0 for off and 1 for on, so we shouldn't need that kind of flexibility here. Other than that the is_on method implementation there is exactly the same as the one we have here.

[andre-richter]

I was referring to the double inheritance using the “Mixin”, which should allow you to get rid of the default None as originally mentioned.

[github-actions]

There hasn't been any activity on this pull request recently. This pull request has been automatically marked as stale because of that and will be closed if no further activity occurs within 7 days.
Thank you for your contributions.

[viiru-]

As far as I'm aware this MR isn't waiting on any activity on my part. There was a suggestion for a different implementation, but as far as I can tell that implementation isn't viable.

[andre-richter]

Now that tests have been enabled, I think we need full coverage? @bdraco?

CC @slovdahl

[bdraco]

Uncovered files need coverage or to be added to .coveragerc. This is a simple platform though so we really should get tests for it.

[viiru-]

Any pointers to an integration that has uses the DataUpdateCoordinator pattern and has tests for individual sensors? I admit I have no idea where to start.

[bdraco]

tplink does (they are sensor though not binary_sensor)

[viiru-]

I took a look at the tplink case but that appears to be different enough from this one (discovery instead of config) that I don't know how to get the parts I need from there. In general the initial introduction of tests for an integration appears to need a lot of deep knowledge which I don't have.

Would it be unacceptable to add this to .coveragerc for now? Adding a test for this sensor is trivial once the integration has one test (enough scaffolding to initialize), I can definitely add it if someone adds that.

[bdraco]

Thank you for your contribution thus far! 🎖 Since this is a significant contribution, we would appreciate you'd added yourself to the list of code owners for this integration. ❤️

Please, add your GitHub username to the manifest.json of this integration.

For more information about "code owners", see: Architecture Decision Record 0008: Code owners.

[frenck]

Thanks, @viiru- 👍