search

openhab / org.openhab.binding.zwave

openHAB binding for Z-Wave
Eclipse Public License 2.0
170 stars 202 forks source link

Add support for COMMAND_CLASS_SOUND_SWITCH #1245

Closed bwosborne2 closed 2 years ago

bwosborne2 commented 4 years ago

This device needs significant binding updates.

"there are significant code changes required to support this device as it uses a new command class." from https://community.openhab.org/t/aeotech-doorbell-6/79278/42

Also, since the database maintenance, this device is no longer visible in the summary.

JimCzuprynski commented 3 years ago

So if this did get fixed, would we be able to include it in the OpenHAB 3.2 beta release? How does all that work, folks?

nza-dk commented 3 years ago

Sure @brydling, I called it version 3.1.1 to easily identify if it's the right one loaded in openHAB: https://www.dropbox.com/s/551p7y5z4gwhpmc/org.openhab.binding.zwave-3.1.1.jar?dl=1

I haven't gotten around all the requirements for a pull-request, I've never done one to openHAB before... But yes @JimCzuprynski, if it gets accepted as a pull-request, it gets included in the following beta release.

brydling commented 3 years ago

@nza-dk It works perfectly! Of course the sound configuration settings are still missing (being able to set the default tone and volume for different events) but I guess that's a new set of commands (SOUND_SWITCH_CONFIGURATION_SET/_GET/_REPORT) not part of the regular COMMAND_CLASS_CONFIGURATION, mainly important when using it as a doorbell, not as a siren, to set which tone and volume shall play when the button is pressed. I use the Z-wave PC Controller to configure this, and then your .jar does everything I need.

I actually modified the xml a bit (in the database). There were two notifications missing (notification_siren3 tells when button 1 is pressed but notification_siren4 and notification_siren5 were missing so you can't get a notification when button 2 or 3 is pressed). I also changed the names and texts of most channels to make it more clear when mapping the channels to the notifications in the product user manual.

If you want to I can send it to you to include in your pull request? I have a 4 month baby, which is great, but I unfortunately don't have the time to investigate how to make a pull-request of my own 😄

JimCzuprynski commented 3 years ago

I'm actually upgrading my OpenHAB configuration from 2.5.1 to 3.1.0 this coming week, so I'll be in a good position to help test this when the beta drops. Perhaps I can even help with expanding some of the other functions? I'd really like to be able to use the device as a siren for burglaries, as well as a "tone" when someone opens or closes a door (or forgets to shut the d*mn thing). ;)

nza-dk commented 3 years ago

@brydling xml updates needs to come from the source database at https://www.opensmarthouse.org/zwavedatabase/1089 🙂 Once (if) the pull-request gets accepted, the channels for playing tones and setting volume will also have to be added to this database.

@JimCzuprynski I did the same upgrade not so long ago, and I can't say it was all straight forward 😄

brydling commented 3 years ago

@nza-dk ah, didn't know that. Thanks for the info. I have found more problems in the database for this device. Have opened a question in the OpenHAB forums on how to fix it: https://community.openhab.org/t/aeotec-doorbell-6-button-power-notifications-not-working/124921

brydling commented 3 years ago

@nza-dk I checked your pull-request. Nice! I see you have set the volume using the configuration message instead of the "Play Command Tone Volume" field in the regular "Tone Play Set" command. I think this actually means that with your code it is possible to add this volume channel to every endpoint in the database, making it possible to set the default volume of each endpoint, but using a channel rather than as a device parameter in OpenHAB. My question is: Would it be possible to also add the default tone parameter (Default Tone Identifier) to the configuration message as a new channel, maybe using a different TYPE like TYPE=DEFAULT_TONE instead of TYPE=VOLUME that you have used to set the volume? So it will be like one channel with TYPE=VOLUME that send a config message with payload (volume, 0) like you do now and another channel with TYPE=DEFAULT_TONE that send a config message with payload (0, default_tone)? With that it would be possible to configure both volume and tone for each of the doorbell's buttons, the tamper alarm, etc. But again, using channels instead of device parameters which probably would have made more sense since this is device configuration. But I guess it would be more job to add support for the Sound Switch Configuration Set/Get/Report commands to the device configuration framework? (I know nothing about the OpenHAB and ZWave binding source code)

Edit: Oh, I just realized @kennetn is the original author of this code. Maybe he can answer the question then :)

openhab-bot commented 2 years ago

This issue has been mentioned on openHAB Community. There might be relevant details there:

https://community.openhab.org/t/z-wave-docker-users/130983/7

kennetn commented 2 years ago

Hi

I have being looking at this again in the hope of creating a more polished version that can be merged. I also found an issue that I have fixed but before I publish this I need to add some unit testing. It still only supports playing tones and setting the volume. To do this I have added two new channels in the ...\thing\channels.xml like this: ` 

<!-- Sound tone play Channel -->
<channel-type id="sound_tone_play">
    <item-type>Number</item-type>
    <label>Sound tone play</label>
    <description>The Sound tone that will play. The allowed tone numbers depend on the device</description>
    <category></category>
</channel-type>

<!-- Sound volume Channel -->
<channel-type id="sound_volume">
    <item-type>Dimmer</item-type>
    <label>Sound volume</label>
    <description>The sound volume channel allows control of the loudness</description>
    <category></category>
</channel-type>

` But it would be nice if @cdjackson or someone else could comment on if this is the right way to implement this?

bwosborne2 commented 2 years ago

I believe that node-zwave-js written in javascript & typescript supports this command. Perhaps it would be useful to look at their code for hints?

brydling commented 2 years ago

@kennetn Nice! What are your thoughts about my suggestion? That way the bindning can be used to fully configure the siren as a doorbell and also dynamically change the configuration, for example lower the volume of the doorbell endpoints when the baby is asleep 🙂 For the volumes I think it is just a matter of adding that to every endpoint in the database, but for the tone there is also code updates needed as I suggested above.

BR Niclas

kennetn commented 2 years ago

Hi

I create a new release that I think should be ok to merge. https://github.com/kennetn/org.openhab.binding.zwave/releases/tag/v3.3.0-v1

I will run it the next few days and then I will create a pull request. Therefore I suggest to delete you pull request @nza-dk.

I did not add support for setting the default tone. Honestly I do not see much point in that at the moment.

brydling commented 2 years ago

Hi!

The point is for the other endpoints. That is how you configure what tone the siren will play when one of the doorbell buttons are pressed or an alarm is received, for example. Currently this has to be configured using another gateway or with the Z-wave PC Controller application because this bindning does not support it.

Setting the default tone for the endpoint used to play tones from OH is pointless as you say as this endpoint is never triggered by anything else than OH.

Basically this patch makes the Siren 6 usable, but it does not make the Doorbell 6 usable.

kennetn commented 2 years ago

Is it not just because the parameters are incomplete in the z-wave database? There is a "Tone Index" for parameters 2 to 8 described in the "Engineering Specification - Aeotec Doorbell 6 .pdf". If that is changed would it not work then ?

brydling commented 2 years ago

I never noticed this while I read the specification before but now I see it. You are probably right. That should hopefully do the trick!

bwosborne2 commented 2 years ago

There is a "Tone Index" for parameters 2 to 8 described in the "Engineering Specification - Aeotec Doorbell 6 .pdf".

Where is that document? It is not in our database, I just used the manual for a reference. That was a LONG time ago though,

bwosborne2 commented 2 years ago

I found the specification & will attach it in the database, I never was very good with bitmap masks. @cdjackson or somebody else needs to update this. this was my first database attempt years ago anyway.

cdjackson commented 2 years ago

I'm happy to add "some" bitmap masks - just so long as it's not hundreds :) If someone can just specify what they want - eg something like parameter 23 needs to be split into 3, parameter 24 needs to be split into 4 - something so my simple mind can then do the bitmasks without having to think too much about the device and how it works since I don't have it...

brydling commented 2 years ago

@cdjackson https://doc.eedomus.com/files/Engineering%20Specification%20-%20Aeotec%20Doorbell%206%20%20.pdf On page 22 you can see the definition of parameter 0x02. On page 22/23 you can see the definition of parameter 0x03. Parameters 0x04 through 0x08 have the same definition as parameter 0x03, with one exception: Parameter 0x08: The valid values of Interval Between 2 Tones are only 0 and 15. The valid values of Continuous Play Count are only 0 and 31.

Is this the information you requested?

Except for this, there are a few other things that need to be changed for this device:

ZWaveAlarmConverter.java, add events.put(NotificationEvent.POWER_MANAGEMENT__REPLACE_BATTERY_SOON, OnOffType.ON); under Battery Alarms because this device sends REPLACE_BATTERY_SOON instead of REPLACE_BATTERY_NOW.

In the database, the channel called notification_siren3 shall be added at least for endpoints 4 and 5. Endpoint 3 is triggered when button 1 is pressed, endpoint 4 is triggered when button 2 is pressed and endpoint 5 is triggered when button 3 is pressed. Currently OH only has a channel for notification from button 1. (The other endpoints also support this channel but I personally can't see a use case there)

I hope that was all the changes I performed, tested and am currently using here at home. Unfortunately I can't find my updated xml-file right now, only the code diff in ZWaveAlarmConverter.java. (This is purely because of my OH incompetence, I know it must be somewhere since I am currently using it).

Also I don't think this device uses the alarm_burglar, as is in the database now. According to the engineering spec, this is the notification it sends for the tamper alarm: Tampering, product cover removed | 0x03 | Vibration event is triggered So I guess it should be alarm_tamper in the database instead, and possibly also a new line is needed in ZWaveAlarmConverter.java here too? I can't find the event "product cover removed" in the code in the revision I have cloned.

nza-dk commented 2 years ago

Great work @kennetn, can't wait to try it out 😃 I have closed my original pull-request.

bwosborne2 commented 2 years ago

A user modified this database entry since we started looking at the Engineering Specification. I have asked what exactly was changed since our system does not show us in this instance. I think I may have skipped masks I was uncertain of at that time. I am not sure I have even done bitmasks on the OSH site. This entry started on the old site.

Let me try & distill what masks are needed. I will concentrate on Parameter 2 since some of the others are similar. It is a 4 byte size masked into several parts. I added the Reserved bits for completeness. We obviously do not define them.

If you provide those bitmasks I can try & update the database entry after we find out what was changed by the user.

cdjackson commented 2 years ago

I want to make sure we're all talking about the same device here. Is it the ZW162 or ZW164 - or are they "the same".

In the database, we have a device named ZW164 -:

https://opensmarthouse.org/zwavedatabase/1089

I think from comments above this is the one you're all looking at - correct? However the link here is to a manual for the ZW162 and the configuration in the database for parameters seems to be completely different to what's in this manual (at least for a few parameters I looked at).

From a search online, the ZW162 and ZW164 seem completely different - at least in their parameter configuration so I'm a little confused what device it is you're working on. Do you need a separate entry for the ZW162, or are you really after the ZW164, but you are looking at the wrong manual (ie you referenced the ZW162)?

bwosborne2 commented 2 years ago

My understanding is that ZW162 Doorbell 6 is just ZW164 Siren 6 packaged with 2 433MHz push buttons. from Aeotec's description on Amazon for Siren 6:

Siren 6. Also Doorbell 6.
Siren 6 really is Z-Wave’s most connected alert system. Easy to set up as a siren, it’s just as easy to convert it
 to a doorbell. Paired with an optional accessory, Button, Siren 6 will offer the same functionality as 
Aeotec’s Z-Wave doorbell, Doorbell 6.

Aeotec Button is sold separately.
cdjackson commented 2 years ago

The manuals and configuration look completely different.

From the ZW162 manual posted above -:

image

From the ZW164 manual -:

image

Not to mention that the ZW164 configs start at 1, where ZW162 starts at 2.

bwosborne2 commented 2 years ago

Should we break them out separate? BTW Amazo actually has the button listed LOL

image

kennetn commented 2 years ago

I was very confused about this also on the back of my device it says ZW162-C however the device is identified as ZW164. So I guess they need to be separated but I can do some testing to verify that the ZW162 Engineering Specification corresponds to the device I have and somebody with a ZW164 would need to check that also. Anyway this should not affect the issues with adding support for COMMAND_CLASS_SOUND_SWITCH, so is it possible that you can have look at my pull request @cdjackson ?

bwosborne2 commented 2 years ago

I do not mind separating them in the database assuming I can identify them by device type / ID.

EDIT: I have identified all 6 devices in that entry so splitting should not be a problem.

I recommend creating 2 new entries, originally with a dummy manufacturer ID until the entries are complete. That will minimize impacting current users of these devices. After they are complete, the old entry can be deleted, the Manufacturer on the new ones corrected, and the new entries approved to the database.

I believe the original request was for Siren 6 and Doorbell 6 "came along for the ride". I was still a novice at entering new devices into the database back in August 2019.

brydling commented 2 years ago

I have two ZW162-C, purchased as a "Doorbell 6" packed with one button each. My understanding was also that this is exactly the same siren that can be bought separately as the "Siren 6". Aeotec even stated that somewhere IIRC, that you can buy a separate button to the "Siren 6" to make it a "Doorbell 6". Didn't know there were two different product IDs.

bwosborne2 commented 2 years ago

Didn't know there were two different product IDs.

Yup. I found the Button at Amazon in the US. for $16 US. They support up to 3 buttons total.

Here is the button for yours from Amazon UK

kennetn commented 2 years ago

I did a bit of testing with the "Zensys Tools - PC Controller 4.78" and things do not really match up for me. Maybe someone can help me :-) This is the output I get when I send CONFIGURATION_GET for 0 to 0x15

_10:29:43.473: Completed: 012 - COMMAND_CLASS_CONFIGURATION.CONFIGURATION_GET, data: 00 10:29:47.169: Completed: 012 - COMMAND_CLASS_CONFIGURATION.CONFIGURATION_GET, data: 01 10:29:47.183: Received: 012 - COMMAND_CLASS_CONFIGURATION.CONFIGURATION_REPORT, data: 01 04 01 00 00 00 10:29:50.722: Completed: 012 - COMMAND_CLASS_CONFIGURATION.CONFIGURATION_GET, data: 02 10:29:50.737: Received: 012 - COMMAND_CLASS_CONFIGURATION.CONFIGURATION_REPORT, data: 02 04 01 00 00 01 10:29:54.394: Completed: 012 - COMMAND_CLASS_CONFIGURATION.CONFIGURATION_GET, data: 03 10:29:54.409: Received: 012 - COMMAND_CLASS_CONFIGURATION.CONFIGURATION_REPORT, data: 03 04 02 00 00 01 10:29:57.675: Completed: 012 - COMMAND_CLASS_CONFIGURATION.CONFIGURATION_GET, data: 04 10:29:57.689: Received: 012 - COMMAND_CLASS_CONFIGURATION.CONFIGURATION_REPORT, data: 04 04 02 00 00 01 10:30:01.475: Completed: 012 - COMMAND_CLASS_CONFIGURATION.CONFIGURATION_GET, data: 05 10:30:01.489: Received: 012 - COMMAND_CLASS_CONFIGURATION.CONFIGURATION_REPORT, data: 05 04 02 00 00 01 10:30:05.280: Completed: 012 - COMMAND_CLASS_CONFIGURATION.CONFIGURATION_GET, data: 06 10:30:05.295: Received: 012 - COMMAND_CLASS_CONFIGURATION.CONFIGURATION_REPORT, data: 06 04 04 00 00 00 10:30:09.102: Completed: 012 - COMMAND_CLASS_CONFIGURATION.CONFIGURATION_GET, data: 07 10:30:09.116: Received: 012 - COMMAND_CLASS_CONFIGURATION.CONFIGURATION_REPORT, data: 07 04 04 00 00 00 10:30:12.967: Completed: 012 - COMMAND_CLASS_CONFIGURATION.CONFIGURATION_GET, data: 08 10:30:12.984: Received: 012 - COMMAND_CLASS_CONFIGURATION.CONFIGURATION_REPORT, data: 08 04 04 00 00 00 10:30:16.919: Completed: 012 - COMMAND_CLASS_CONFIGURATION.CONFIGURATION_GET, data: 09 10:30:22.984: Completed: 012 - COMMAND_CLASS_CONFIGURATION.CONFIGURATION_GET, data: 0A 10:30:27.296: Completed: 012 - COMMAND_CLASS_CONFIGURATION.CONFIGURATION_GET, data: 0B 10:30:31.609: Completed: 012 - COMMAND_CLASS_CONFIGURATION.CONFIGURATION_GET, data: 0C 10:30:35.370: Completed: 012 - COMMAND_CLASS_CONFIGURATION.CONFIGURATION_GET, data: 0D 10:30:39.247: Completed: 012 - COMMAND_CLASS_CONFIGURATION.CONFIGURATION_GET, data: 0E 10:30:43.507: Completed: 012 - COMMAND_CLASS_CONFIGURATION.CONFIGURATION_GET, data: 0F 10:30:48.475: Completed: 012 - COMMAND_CLASS_CONFIGURATION.CONFIGURATION_GET, data: 10 10:30:48.489: Received: 012 - COMMAND_CLASS_CONFIGURATION.CONFIGURATION_REPORT, data: 10 04 4B 19 14 03 10:30:56.428: Completed: 012 - COMMAND_CLASS_CONFIGURATION.CONFIGURATION_GET, data: 11 10:30:56.442: Received: 012 - COMMAND_CLASS_CONFIGURATION.CONFIGURATION_REPORT, data: 11 04 32 32 00 03 10:31:11.316: Completed: 012 - COMMAND_CLASS_CONFIGURATION.CONFIGURATION_GET, data: 12 10:31:11.330: Received: 012 - COMMAND_CLASS_CONFIGURATION.CONFIGURATION_REPORT, data: 12 04 00 21 01 03 10:31:17.246: Completed: 012 - COMMAND_CLASS_CONFIGURATION.CONFIGURATION_GET, data: 13 10:31:17.260: Received: 012 - COMMAND_CLASS_CONFIGURATION.CONFIGURATION_REPORT, data: 13 04 21 00 00 03 10:31:29.428: Completed: 012 - COMMAND_CLASS_CONFIGURATION.CONFIGURATION_GET, data: 14 10:31:29.443: Received: 012 - COMMAND_CLASS_CONFIGURATION.CONFIGURATION_REPORT, data: 14 04 00 00 00 0A 10:31:37.311: Completed: 012 - COMMAND_CLASS_CONFIGURATION.CONFIGURATION_GET, data: 15 10:31:37.325: Received: 012 - COMMAND_CLASS_CONFIGURATION.CONFIGURATIONREPORT, data: 15 04 00 00 0A 00

But the device is responding to parameter 1 that is not described and the values for 2 should have been default "34 07 00 00". The other default values are also different that what it says in the https://doc.eedomus.com/files/Engineering%20Specification%20-%20Aeotec%20Doorbell%206%20%20.pdf

What am I missing ?

kennetn commented 2 years ago

I think I found the issues, there is a another branch of what looks like the same document here: https://aeotec.freshdesk.com/helpdesk/attachments/6086176996 This match with my device and has version number 6 and the other one has version 8. But the date in version 6 is later then the other one in version 8.

brydling commented 2 years ago

What revision is your device reporting? Maybe I shall try the same thing with mine and see if my mappings are the same.

kennetn commented 2 years ago

I do not think that will be required. I am not sure the document on https://doc.eedomus.com/files... is even relevant. I am sorry I brought it up. I go the link from this post https://community.openhab.org/t/aeotech-doorbell-6/79278/25 and I guess I was to hasty and thought it was just a newer version. But I guess the best documentation is on these two pages: https://help.aeotec.com/support/solutions/articles/6000202226-doorbell-6-user-guide- https://help.aeotec.com/support/solutions/articles/6000202227-siren-6-user-guide- And they look identical so no reason to split it up in the database but I think @bwosborne2 or someone else should remove the Reference to "Doorbell 6 Engineering Specification" and add the other two documents instead.

bwosborne2 commented 2 years ago

Many times vendors add features to new firmware levels. When that happens we make a new database entry fir the newer firmware.

But I guess the best documentation is on these two pages: https://help.aeotec.com/support/solutions/articles/6000202226-doorbell-6-user-guide- https://help.aeotec.com/support/solutions/articles/6000202227-siren-6-user-guide- And they look identical so no reason to split it up in the database but I think @bwosborne2 or someone else should remove the Reference to "Doorbell 6 Engineering Specification" and add the other two documents instead. I added the engineering specification due it being mentioned here.

I will be interesting to see how the content of the guides you linked to compare to the one from 2019 in the database.

EDIT: The Engineering Specification has been deleted from the database entry.

brydling commented 2 years ago

Looking at the version 6 specification it seems we are back at the issue where the default tone for the different endpoints can't be configured using the regular configuration class, but must be configured using the sound switch configuration message implemented in this PR?

Then I guess we're back at my original suggestion, or something similar?

bwosborne2 commented 2 years ago

i will move the primary database discussions to email to avoid clogging up this Issue further.

bwosborne2 commented 2 years ago

Sorry to bother, but roe original device xml we have is the 1.4 firmware. and our documentation appears to be for 1.6.

It appears 1.6 (1.06) was released in March 2019 & is available as an upgrade, following Aeotec's directions. Could @brydling or @kennetn please post the openHAB XML for a firmware 1.6 device? That would be a large help to me.

bwosborne2 commented 2 years ago

I got the xml file, thanks but the website logic (that I helped write) is choking on the zml and nothing useful in debug logs. Sorry for the extra delay on the database part.

kennetn commented 2 years ago

Hi @cdjackson

Do you think you will have time to look at my pull request some time in the future ? I do not think it should be a lot of work it is very similar to the existing code. I do not mind if there is something you think should be done differently. I know that I could add more features to the SOUND_SWITCH class but if it will not be merged I do not think it makes a lot of sense.