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

New platform for siren/chime devices #375

Closed
marcelveldt opened this issue Apr 27, 2020 · 42 comments
Closed

New platform for siren/chime devices #375

marcelveldt opened this issue Apr 27, 2020 · 42 comments

Comments

@marcelveldt
Copy link
Member

marcelveldt commented Apr 27, 2020

Context

Currently there is no native support for "siren devices". Best described as a small noise producing box, commonly used to be triggered by an alarm but it's also pretty common that they're used as chime for a doorbell. A great example for such device is the Aeotec Siren 6 (Z-Wave) which can be used to make a loud noise when triggered by burglar alarm but can also be paired with a button to use as doorbell. While the Aeotec siren is a pretty advanced one, allowing you to set separate volume and tones for all kind of actions, there are also some more basic examples like the Neo Coolcam Siren.

These siren devices do not only exist in the Z-wave world. The Neo Coolcam for example is also available as WiFi version (Tuya) and a quick Google search learned that there are several Zigbee and WiFi based sirens: Nedis, Nexa, Netatmo, D-Link, BrilliantSmart, Tuya, Devolco (Zigbee), Wattle, Gigaset.

I own the Neo Coolcam and Aeotec devices myself and while working on the zwave_mqtt integration I tried getting these device integrated in a userfriendly but feature complete manner. The only thing I could think of was abusing the media_player platform but that just did not feel right. The siren device is much more primitive and may or may not support selecting a (predefined) Tone, not a source or media_url, nor will it support media actions.

Proposal

Add a new siren integration, so we have a good fit for these devices and do not have to abuse other platforms.

From what I've explored most sirens seem to use a state (is it making noise or not), an ability to set the volume level and some will even have the feature to set what tone to play.

In case of the Aeotec Siren/Doorbell it has multiple instances. You can specify a sound/volume for different actions/triggers (and the state is also separated). For example different sound and volume for the doorbell than for the intrusion alert. This will be represented as multiple siren entities. Trying to grab that all in one entity will make it horrible to maintain and control imo,

Properties:

  • state (on/off)
  • available tones (list of available tones) [optional]
  • active tone (which of the available tones is active for this siren (instance) [optional]
  • volume_level [optional]

Functions:

  • set_tone (select a new active tone) [optional]
  • set_volume [optional]
  • turn_on/turn_off (some of the devices allow you to manually trigger it)

Consequences

People have to use all kind of workarounds atm to make their siren device functional with Home Assistant, all which comes with a negative impact on user experience, especially for the non technical people. For example, my wife likes to lower the volume of our (Aeotec) doorbell when the kids are asleep but she does not have an easy control to do that. In fact, most siren devices are not even recognised in Home Assistant at all, simply because there is no box to fit them in.

While adding this additional platform for siren devices makes life easier for those with a connected doorbell/siren but it will also mean another component is needed in the frontend to represent this siren device, although maybe the media_player component can be extended/simplified for this.

@balloob
Copy link
Member

balloob commented Apr 30, 2020

Seems reasonable .

@blakadder

This comment has been minimized.

@ranrinc

This comment has been minimized.

@MartinHjelmare
Copy link
Member

This architecture issue has been accepted. It's just waiting for implementation. I've planned to do this when I get some time if it's still not done then.

Please don't comment with +1.

@lacojim
Copy link

lacojim commented Oct 7, 2020

@MartinHjelmare Let me know when you have time to start on this project and I will order one and help out best as I can. I am pretty new to HASS and am currently running it in tandem with my Mozilla IoT Pi, which also has no support for sounding devices.

@wileyrya
Copy link

wileyrya commented Oct 7, 2020

I have an Aeotec Siren and would love to help out with coding and/or testing.

@MartinHjelmare
Copy link
Member

If you want to build the siren entity integration, take a look at the PR for the humidifier integration that was relatively recently added:
home-assistant/core#28693

That PR shows what's required to add a new base entity integration.

@chrismdann

This comment has been minimized.

@MartinHjelmare
Copy link
Member

There's no need to ask if there's been any movement. Please don't spam this issue.

@peterjobrien
Copy link

I had some time so I put together a draft of the siren entity, mostly aped from the light entity codebase. It's certainly not ready for a pull request so I have it as a gist at https://gist.github.com/peterjobrien/a39a258c60ecbe382e30496c52f49ec3

I've never developed anything like this before. It's incomplete for sure missing tests, demo, etc but I hope it's helpful anyway.

Please feel free to take over, copy, use, or not use this work. If I find more time I'll revisit this but can't commit to a timeline.

There are two open questions I think regarding how much structure to put into the core entity - first, custom/multiple endpoints per device (alarm, doorbell/chime, tamper, etc), and second, what attributes to expose.

I avoided any dedicated alarm vs tamper vs doorbell/chime endpoint functionality. The aeotec gen6 exposes multiple endpoints which get picked up as separate siren entities. The user can determine or assign which entity is for alarm, doorbell, etc. so no dedication in the core entity is needed. However, there may be other devices (likely non-zwave) where this approach doesn't work as well.

I went pretty wide on the attributes, referencing the aeotec gen6 siren functionality. These are still pretty generic but I later realized many will require device-specific code somewhere downstream, even for Zwave devices. It seems that Zwave soundswitch CC only exposes Tone_Count, Tones, Volume, Default_Tone, the rest are custom configurations from the device. Given that, I'm not sure if the breadth of attributes I have here is appropriate or should be scaled back.

To test this with a device it also really needs the ozw component created.
It looks like at minimum that entails updating ozw/discovery.py and creating a new ozw class for siren. However, I think this may need upstream help in openzwavemqtt as I don't see any valueIDs defined there relating to sirens.

@MartinHjelmare
Copy link
Member

MartinHjelmare commented Dec 4, 2020

We should start small with only the features in the proposal above. And device actions, state reproduce etc should not be part of the first PR.

@raman325
Copy link
Contributor

raman325 commented Mar 25, 2021

Hi all, I put up a PR for this proposal based on @marcelveldt's suggestions. However, thinking through this some more, I had some thoughts/questions (this was all in my PR but @frenck rightly pointed out it belongs here instead of the PR):

  • on and off states to me represent whether the device is enabled or disabled. Like media players that have an idle and playing state, I think we should have an active and idle state for sirens. That way, on can just represent that the siren is able to accept commands (if idle/active states can't be detected), and off means that it won't play a tone. EDIT: Martin made a good counter point below that invalidates this
  • Based on above, I think turn_on and turn_off should be optional services that can enable or disable the siren but don't necessarily trigger a sound to be emitted. Should we have and include SUPPORT_TURN_ON and SUPPORT_TURN_OFF flags like we do for media players. I don't know that all sirens would support this capability (think of an ESP connected analog doorbell as an example) but some more sophisticated sirens might. EDIT: Based on the invalidation of the above point, this has been updated
  • Again following the logic from above, perhaps we should have an activate and deactivate service that actually triggers the audible noise to turn on or turn off. Furthermore, it's not likely to me that all sirens would have the ability to turn off the noise (again thinking of the ESP connected analog doorbell), so this would have to be feature gated by another support flag IMO. @frenck also pointed out that some sirens may not be able to be manually activated, so perhaps these should be supported features gated as well?
  • The siren I have (an Aeotec Siren) will let you specify the tone when you activate it I think. This suggests to me that the activate service should have an optional tone parameter that lets you specify a tone. EDIT: Now I am thinking we have a separate play_tone service for cases where you can specify a specific tone to play (this is different from setting an active tone)

@frenck
Copy link
Member

frenck commented Mar 25, 2021

Would mute be clearer?

One is able to turn on or off the sound of the siren using the turn_on/turn_off, which can maybe have the tone, duration and other stuff.

While muting (just like media players) allows to deafen it?

I think on/off/active/deactivate are terms too close to be clearly differentiable

@raman325
Copy link
Contributor

how about silence instead of mute? Your point about activate and deactivate is fair, I was thinking that we couldn't add parameters to turn_on and turn_off services but I think it just means we can't make this a ToggleEntity

@lcotten
Copy link

lcotten commented Mar 25, 2021

Shouldn't HA follow the convention that other home automation software uses today? Anyone know what Homeseer's home automation software uses?

@frenck
Copy link
Member

frenck commented Mar 25, 2021

If anything, we should look at z-wave, Zigbee, Google Assistant Smart Home traits, HomeKit or Alexa.
As those are the most important integrators for Home Assistant.

Raman325, in this case, uses Z-Wave as a lead. Looking at Google, HomeKit & Alexa, it seems like they are generally not aware of Sirens specifically; instead, they focus on the alarm system. Which makes sense I guess.

@Kane610
Copy link
Member

Kane610 commented Mar 25, 2021

In deconz case, I only know of one type of siren, "warning device" with possibility to report state and turn on/off.

Apparently there is a new Tuya siren which provides more mechanisms, that just got supported. volume and siren warning time are at least configurable

@blakadder
Copy link

Tuya does treat sirens as siren devices (regardless whether its wifi or zigbee) where you can turn it on/off, set volume and chime sound.

Here's a list of capabilities of a wifi siren (with a integrated temperature and humidity sensor:

  • dpId 102 - alarm type chime sound (0-17)
  • dpId 103 - alarm duration time (in seconds)
  • dpId 104 - alarm control on/off
  • dpId 105 - temp in C
  • dpId 106 - humidity
  • dpId 107 - minimum temperature to trigger alarm
  • dpId 108 - maximum temperature to trigger alarm
  • dpId 109 - minimum humidity to trigger alarm
  • dpId 110 - maximum humidity to trigger alarm
  • dpId 112 - unit change (on = C, off = F) - does not affect dpid 105 values
  • dpId 113 - temperature alarm control (0 = off, 1 = on)
  • dpId 114 - humidity alarm control (0 = off, 1 = on)
  • dpId 116 - alarm volume (0 = high, 1 = medium, 2 = low)

See https://templates.blakadder.com/neo_coolcam_NAS-AB02W.html for more info

@MartinHjelmare
Copy link
Member

I don't think we need to take the power on/off state into consideration for the siren integration. That can be a separate switch entity for the devices that support that. The siren integration should just be about controlling the sound of the siren, nothing more.

@raman325
Copy link
Contributor

raman325 commented Mar 26, 2021

Fair point Martin, so then how about this:

  • Add a SUPPORT_TURN_OFF supported feature to gate turn_off since that may not be possible
  • Add a SUPPORT_TURN_ON supported feature to gate turn_on since that may not be possible to manually triggere
  • Change type for tones to int | str. Do we want to have SUPPORT_TONE_RANGE and SUPPORT_TONE_LIST to distinguish between something that has a list of tones and one that has a range of values for tones (so a min/max vs a list of numbers)? Or just keep it simple and have a list of available tones regardless of whether the tones are strings or ints?
  • Add an alarm duration property along with a set_duration service gated by a SUPPORT_DURATION feature
  • Add a play_tone service to handle the case where you can specify the tone to use when activating the siren.

I am basing this off of thee link that @blakadder shared as well as past comments

@MartinHjelmare
Copy link
Member

MartinHjelmare commented Mar 26, 2021

Add a play_tone service to handle the case where you can specify the tone to use when activating the siren.

Why can't we just have a set_tone service to change the tone of the siren that is heard when the siren is active?

@raman325
Copy link
Contributor

the siren I have allows you to set a default tone but to also specify a tone to play, so the two use cases are different

@MartinHjelmare
Copy link
Member

Do we need to support both? The user will be able to automate to change tone before activating the siren.

@Crinisen
Copy link

Do we need to support both? The user will be able to automate to change tone before activating the siren.

I can obviously adjust my workflow but I use the "default" and group memberships to have my doorbell on the far side of the house near the living trigger my other Aeotec Siren 6 (Z-Wave) by the bedrooms. The doorbell button does not have that great of range and I prefer to let both the doorbell and some security devices trigger the default alarms even when the controller is offline. The Siren/Doorbell supports several different endpoints for different purposes and I am thinking of using them for door chimes with different alerts. I would prefer to be able to use both default for device-to-device and specific triggered via HA. Not a huge deal depending on the coding / maintenance costs involved in having support for both.

@raman325
Copy link
Contributor

Do we need to support both? The user will be able to automate to change tone before activating the siren.

What's the concern with supporting both? There's a valid use case for having a single call that can both activate the siren and set the tone for that particular action - my siren would do that as a single call.

@MartinHjelmare
Copy link
Member

When we add features and options we increase complexity. We should always take this into consideration.

Will the play_tone service also modify state if the device doesn't feedback a state report? Ie both turn on/off and play_tone will modify the same state.

Will play_tone also modify the active tone state? What happens after the tone has stopped playing? Should the default tone be set back as active tone?

@raman325
Copy link
Contributor

raman325 commented Mar 28, 2021

Understood, I certainly don't want to add features for the sake of it.

I think those questions would be up to the implementer, but if we wanted to look at the Aeotec Siren as an example, play_tone would not modify the active tone, which can be set independently of playing a different tone. As for the state of the siren, if we can detect the state then play_tone and turn_on should result in the same on state for the duration of the noise being emitted.

@peterjobrien
Copy link

Below is a table of elementary siren functions and how they are implemented (or not) in Zwave , Zigbee, Tuya Wifi, and Aeotec's extra functions outside the zwave Sound_Switch spec.
Given this base platform will later be implemented for each of these protocols, I though it helpful for context.

@raman325 @MartinHjelmare - One observation for both handling the 'default tone' and also separation of configuration vs. on/off functionality. Neither Zwave nor Zigbee have a simple 'turn on/turn off' parameter-free function. In Zwave, you can leave the tone and volume parameters blank/zero to use the default values, but in Zigbee you have to specify the warning mode (I believe these are different tones), volume, etc for every call to turn the siren on/off. The Tuya Wifi implementation does separate a simple on/off from the config setup.

Because at least Zigbee needs the siren configuration context to properly call 'turn_on', it seems like that function should be within this platform and not exposed as a separate binary switch entity.

Also, some diverging concepts across these protocols is - Light (strobe) effect is unique to Zigbee but used by Aeotec through custom configurations. The concept of a tone list is unique to Zwave.
I'm not sure how is best to handle these differences within this base platform.

I'm referencing
ZCL: https://zigbeealliance.org/wp-content/uploads/2019/12/07-5123-06-zigbee-cluster-library-specification.pdf
Zwave: https://www.silabs.com/documents/login/miscellaneous/SDS13781-Z-Wave-Application-Command-Class-Specification.pdf

The double-dashed entries are the parameters of the function above:

Function Zwave Sound Switch Zigbee IAS WD Wifi (Tuya)
Turn On TONE_PLAY_SET
--ToneID
--Volume
START WARNING
--Warning Mode
--Strobe Enable
--Siren volume
--Duration
--Strobe duty cycle
--Strobe intensity

SQUAWK
--Squawk Mode
--Strobe enable
--Squawk volume
dpId 104 - alarm control
Turn Off TONE_PLAY_SET START WARNING dpid 104 - alarm control
AdjustVolume TONE_PLAY_SET START WARNING dpid 116 - alarm volume
Get Tone Info TONES_NUMBER_REPORT
+
TONE_INFO_REPORT
--Tone Identifier (Integer)
--Tone Name
--Tone Duration
Not Implemented Not Implemented
Select Tone TONE_PLAY_SET START WARNING dpid 102 - set chime sound
Configure Tone CONFIGURATION_SET
--default tone
--default volume

Device Custom configs
 (eg, Aeotec Siren 6):
--Tone Repeat Number
--Tone Repeat Delay
--Tone Duration (clipping)
MaxDuration Attribute dpid 103 - alarm duration
Get Light Effect Info Device Custom config
(eg, Aeotec Siren 6):
--Read parameters listed below
Not Implemented Not Implemented
Select Light Effect Device Custom config
(eg, Aeotec Siren 6):
--light effect index
START WARNING Not Implemented
Configure Light Effect Device Custom config
(eg, Aeotec Siren 6):
--Gradually bright duration
--Gradually extinguished duration 
--Keep bright duration
--Keep extinguished duration
START WARNING Not Implemented

@MartinHjelmare
Copy link
Member

All of the features we implement in the base integration will have a meaning and should behave the same for all implementing platforms. So we have to decide this and not let it be up to each platform.

Eg the active tone attribute: Should it mean the tone of the siren when it is playing or just the default tone and another tone could be active if called some other way than the default activation?

@raman325
Copy link
Contributor

raman325 commented Mar 30, 2021

All of the features we implement in the base integration will have a meaning and should behave the same for all implementing platforms. So we have to decide this and not let it be up to each platform.

Eg the active tone attribute: Should it mean the tone of the siren when it is playing or just the default tone and another tone could be active if called some other way than the default activation?

I think it should be the latter. Perhaps it really should be called default_tone. For the zigbee example where you always have to include a tone in the payload, the implementation can use the default_tone in the turn_on implementation.

thanks @peterjobrien for sharing that table, it is extremely helpful!

@Crinisen
Copy link

Crinisen commented Apr 1, 2021

Will play_tone also modify the active tone state? What happens after the tone has stopped playing? Should the default tone be set back as active tone?

At least with my experimentation using MQTT + OpenZWave Beta and Zwavejs2Mqtt, the Aeotec Siren 6 (Z-Wave) will keep playing a tone forever if not told to play tone 0 to stop. If you use the play default, it only plays once.

@raman325
Copy link
Contributor

raman325 commented Apr 1, 2021

Will play_tone also modify the active tone state? What happens after the tone has stopped playing? Should the default tone be set back as active tone?

At least with my experimentation using MQTT + OpenZWave Beta and Zwavejs2Mqtt, the Aeotec Siren 6 (Z-Wave) will keep playing a tone forever if not told to play tone 0 to stop. If you use the play default, it only plays once.

I tried it using ZWaveJS by setting a value on the ToneID value and it played once then stopped

@Crinisen
Copy link

Crinisen commented Apr 2, 2021

I tried it using ZWaveJS by setting a value on the ToneID value and it played once then stopped

Hmm, Firmware variation? I just tested via the ZWaveJS2MQTT GUI setting [13-121-8-toneId] Play Tone to 01 Ding Dong (5 sec) and it kept ringing for 30 Seconds before I set it back to off.

Either way I will be willing to donate one of these to whomever decides to work the implementation if needed.

@raman325
Copy link
Contributor

raman325 commented Apr 2, 2021

as of now I am working on the implementation, and I have one to play with, but appreciate the offer 🙂

@raman325
Copy link
Contributor

raman325 commented Apr 6, 2021

Alright so here's where I landed so far:

  1. There are feature flags for SUPPORT_TURN_ON, SUPPORT_TURN_OFF, SUPPORT_TONES, SUPPORT_DURATION, SUPPORT_VOLUME_SET.
  2. turn_on can accept up to three parameters optionally: tone, duration, and volume level. The actual attributes that are supported are device dependent.
  3. default_tone is the tone that plays when no tone is provided in the turn_on service call. This can either be something that is set on the device or something that is managed within the integration.
  4. duration can either be set using the set_duration service if its supported, otherwise it can be a read only property that's dependent on the default tone.

How does that sound? Anything I am missing here? Note that I did not handle the strobe portion, I'm not sure if it belongs in this platform or not

@MartinHjelmare
Copy link
Member

Should we call it SUPPORT_TONES or SUPPORT_TONE?

Should we name the duration attribute default_duration same as for default_tone?

We should decide the validation schemas for the service attributes.

Suggestion:
tone: cv.string (any string)
duration: float (seconds)
volume_level: cv.small_float (0 - 1)

@raman325
Copy link
Contributor

raman325 commented Apr 6, 2021

Good suggestions. The thought behind SUPPORT_TONES was that some devices may only support one tone, in which case there is no value in showing what that tone is or available tones. Every siren that emits a noise should support at least a single tone.

Tone may not always have a string identifier so I think it needs to be a union of int | str.

For duration, I went with cv.positive_float. Is there any reason we would want to support a negative duration?

@MartinHjelmare
Copy link
Member

There's no problem to turn eg "1" into a 1. So we can keep cv.string for tone in the service schema. It's up to each integration to feed the correct type to its library and device.

cv.positive_float is good for duration. 👍

@raman325
Copy link
Contributor

raman325 commented Apr 6, 2021

I thought I had pushed my changes last night but I hadn't. Added the suggestions here and re-pushed. I haven't fixed tests or docs yet, will do that once we settle on the approach

@raman325
Copy link
Contributor

raman325 commented Jul 4, 2021

Quick update here, we've gotten consensus and should be merging the new siren platform too. We simplified the siren platform design significantly from where we started.

There are five supported feature flags:

  • SUPPORT_TURN_ON: Determines whether the entity can be turned on within HA (a doorbell may not have this capability)
  • SUPPORT_TURN_OFF: Determines whether the entity can be turned off within HA (a doorbell may not have this capability)
  • SUPPORT_DURATION: Indicates that the turn on service will accept a duration parameter in seconds
  • SUPPORT_TONES: Indicates whether the entity can choose a specific tone to play. If it's enabled, the integration can provide a list of available tones (available_tones attribute) and the turn on service will accept a tone as input
  • SUPPORT_VOLUME_SET: Indicates whether the entity can be set to a specific volume level. If it's enabled, the turn on service will accept a volume level

There are three services:

  • turn on
  • turn off
  • toggle

What this does not add support for natively is setting defaults for tone, duration, or volume. These are things that the integration can provide if it's supported, or can be added to the base platform in a future iteration.

@MartinHjelmare anything I missed that you can think of?

@MartinHjelmare
Copy link
Member

Sounds good!

@raman325
Copy link
Contributor

Given that the PR has been merged, I think we can close this architecture issue 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests