Add proper support for zwave_js Indicator CC

[raman325]

Breaking change

On/Off properties for the Indicator CC are no longer discovered as entities. They were previously discovered as sensor entities which were not useful in cases where you wanted to change these values, and you would need to change the value for all three On/Off properties simultaneously in order for them to work which wasn't supported with our existing discovery model.

Proposed change

Resolves #90049

Note that I couldn't find real test data for all of the cases so I created a fake device to test them. We can replace these with real devices if/when we get proper fixtures for them.

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)
• 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

• 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
• I have followed the perfect PR recommendations
• 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.

To help with the load of incoming pull requests:

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

[home-assistant]

Hey there @home-assistant/z-wave, mind taking a look at this pull request as it has been labeled with an integration (zwave_js) you are listed as a code owner for? Thanks!

Code owner commands

Code owners of zwave_js 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 zwave_js Removes the current integration label and assignees on the pull request, add the integration domain after the command.

[kpine]

For a Zooz ZEN72 device, this generates a few new number entities (previously they were disabled sensors):

Unfortunately, these entities are not usable. According to the Z-Wave specification, these particular indicator properties must be set at the same time (in the same command). With a compliant device, these commands will either fail or be ignored. The only way to set them currently is to use the CC API Indicator.set command passing in multiple identifier properties (which is how the Identify button works). I would suggest either not discovering these specific properties, or disabling them by default, as they are currently non-functional. Or Z-Wave JS needs to be updated to set all of these correctly when a single value is changed.

[kpine]

Also, maybe this is a frontend issue, but I can't set the number entity value to 0. I don't see the setting ever reaching HA.

[kpine]

You can also see that properties 3 and 5 units are 100 ms. If you examine the rest of the properties, you'll see there are other time units and not all ranges of 0-255 are allowed.

Z-Wave JS does not consistently provide max-min values for these properties, nor does it provide any units.

[MartinHjelmare]

Requesting changes to reflect the PR status. I only had a question and a side note though.

[MartinHjelmare]

Side note: I think we should refactor the discovery and design it like we do in the matter integration. There we attach an Entity class and an EntityDescription to each schema instance and define the schema instances in each platform.

entity_category and entity_registry_enabled_default are entity details and belong in the entity description not in the discovery info.

It's a bigger refactor so something for a separate PR of course.

[home-assistant]

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

Learn more about our pull request process.

[raman325]

You can also see that properties 3 and 5 units are 100 ms. If you examine the rest of the properties, you'll see there are other time units and not all ranges of 0-255 are allowed.

Z-Wave JS does not consistently provide max-min values for these properties, nor does it provide any units.

I'm not sure how to address this. To me this seems like a driver issue as we rely on the driver to tell us these things. The 0 and 255 came from this comment: #46956 (comment)

Open to changing the defaults but the better option would be to get an accurate min and max

[kpine]

I'm not sure how to address this. To me this seems like a driver issue as we rely on the driver to tell us these things. The 0 and 255 came from this comment: #46956 (comment)

Open to changing the defaults but the better option would be to get an accurate min and max

There's nothing you can really do without help from the driver (unless you want to manually add everything yourself based on the property IDs), this was just an observation. The entities as they currently are will allow the setting of invalid values.

The bigger problem is that these v2 property entities just won't work, and for them to work properly also requires help from the driver. I would suggest omitting these until a solution is reached. Is adding these new non-functional entities better than doing nothing? I also don't think the user model works. These values need to be set at the same time, and separate entities don't allow that.

v1 indicator value and identity button are a good start. The latter is the only one required for certification.

[MartinHjelmare]

Besides the merge conflict, is this PR waiting for anything?

[raman325]

Besides the merge conflict, is this PR waiting for anything?

no, I will mark this as ready for review as soon as I fix the merge conflict