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

Z-Wave JS: Certification requirements for Sound Switch CC not fulfilled #16

Closed
marcelveldt opened this issue Jul 13, 2023 · 10 comments
Closed
Assignees
Labels
frontend This bug or request (primary) involves the Home Assistant frontend new feature New feature or request zwave-certification Required for Z-Wave certification

Comments

@marcelveldt
Copy link
Collaborator

marcelveldt commented Jul 13, 2023

The end user MUST be able to play the default tone, play a selected tone or stop playing a tone.

The UI for siren control doesn't seem to allow selecting a specific tone, but it shows a weird dropdown with some attributes:
grafik

Diagnostics information

zwave_js-f0926e113c178046d0d249fcdc42964a-Node 65-69bfeb31c9b5eca03be46337ae416f51.json.txt

@marcelveldt
Copy link
Collaborator Author

I believe I once wrote an architecture proposal for the siren platform, based on this Soundswitch CC and only the on/off feature has been implemented so far. Imo we should extend the siren platform as these feature are not unique to Z-Wave but also exist for zigbee/wifi sirens/chimes.

@AlCalzone
Copy link
Member

To work around this in the short term, adding a simple dropdown to play a given tone like for the default tone should be enough to fulfill the certification requirements:

Please play tone 'Tone003'!

Image


Oddly enough, stopping the tone playing changes the controls to a toggle - seems unintended?

Image

@marcelveldt
Copy link
Collaborator Author

Fix the "Play Tone" so its always stateless (shows both start and stop)
Add select entity to play a specific tone --> on select it will simply play the tone and revert the state of the select entity (or the state is simply always unknown)

@MartinHjelmare
Copy link
Collaborator

Would it be enough to change the default tone and turn on the siren to fullfil the "play a selected tone"?

@AlCalzone
Copy link
Member

No, the specific play command is expected. Also it could be unexpected if playing a different tone once changes the default tone.

@MartinHjelmare
Copy link
Collaborator

MartinHjelmare commented Sep 2, 2024

I think this needs a selector from available tones, in the siren card in the frontend to enable passing the selected tone in the service call to turn_on. We shouldn't add more select entities for this. That would just be confusing since we have select entitites already for the default tones, and it would also not be in line with how a select entity should be used.

The available tones are part of the state attributes, so all data should exist already for the frontend to build this.

https://developers.home-assistant.io/docs/core/entity/siren#properties

@MartinHjelmare MartinHjelmare added the frontend This bug or request (primary) involves the Home Assistant frontend label Sep 2, 2024
@AlCalzone AlCalzone added the zwave-certification Required for Z-Wave certification label Sep 5, 2024
@marcelveldt
Copy link
Collaborator Author

marcelveldt commented Sep 5, 2024

@MartinHjelmare so you are basically proposing that we can allow a "tone" argument to the "turn_on" action for a siren then ? Which basically means "turn on using sound X" ?

OK, me being blind; I see we already have that but just the frontend is missing the selector.

@MartinHjelmare
Copy link
Collaborator

MartinHjelmare commented Sep 5, 2024

Yes, exactly. It should just be a frontend improvement of the turn on feature for all sirens that support tones.

@marcelveldt
Copy link
Collaborator Author

siren.dump.zip

attached dump file to use in the mock server

@AlCalzone
Copy link
Member

Test passed 🥳

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
frontend This bug or request (primary) involves the Home Assistant frontend new feature New feature or request zwave-certification Required for Z-Wave certification
Projects
Development

No branches or pull requests

4 participants