search

home-assistant / architecture

Repo to discuss Home Assistant architecture
314 stars 100 forks source link

New platform for siren/chime devices #375

Closed marcelveldt closed 3 years ago

marcelveldt commented 4 years ago

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:

Functions:

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 commented 4 years ago

Seems reasonable .

blakadder commented 4 years ago

+1 There are sirens in Tasmota/MQTT and potentially under Tuya Smarthome integrations too

ranrinc commented 3 years ago

+1 cause I own a tuya sirens with all the security glory.. just need it to be implemented to HA

MartinHjelmare commented 3 years ago

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 commented 3 years ago

@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 commented 3 years ago

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

MartinHjelmare commented 3 years ago

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

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

chrismdann commented 3 years ago

Any movement. Have the Wink Chime and Coolcam Alarm. Would love to use it.

MartinHjelmare commented 3 years ago

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

peterjobrien commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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

frenck commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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

frenck commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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:

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

MartinHjelmare commented 3 years ago

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 commented 3 years ago

Fair point Martin, so then how about this:

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

MartinHjelmare commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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

Crinisen commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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

raman325 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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. :+1:

raman325 commented 3 years ago

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 commented 3 years ago

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:

There are three services:

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 commented 3 years ago

Sounds good!

raman325 commented 3 years ago

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