dresden-elektronik / deconz-rest-plugin

deCONZ REST-API plugin to control ZigBee devices
BSD 3-Clause "New" or "Revised" License
1.9k stars 498 forks source link

DDF add support for Neo TS0601 #7832

Open ThiemeNL opened 2 months ago

ThiemeNL commented 2 months ago

Product name: Neo siren alarm with battery Manufacturer: _TZE200_t1blo2bj Model identifier: TS0601 Device type to add: Siren, but recognized as light

6112 with help of @Smanar

SwoopX commented 2 months ago

Took me a while to have a closer look. Thanks for the recent changes, the majority seems to look fine. I'm afraid this PR doesn't get my vote to get merged for a couple of reasons. Let me try to explain:

  1. The light should use state/alert as previously mentioned for consistency In contrast to Smanar's believe that anything for Tuya is blocked, anything is workable for any device. Especially the DDF code is so rediciously simple to have all light related resource items work with DDFs that I couldn't believe it at the beginning. What is adding complexity here, admittedly, are the various Tuya hacks already in the code. This gives me the creeps and they should go as soon as possible, even with the risk of breaking stuff imho. The longer that code stays, the more difficult wil it be to remove it.
  2. The DDF will only work for 1 of the 2 manufacturer/modelid combinations mentioned (siren usage more precisely) For the TZE204 manufacturer, it should not be possible to sound the alarm via DDF, as the hacks prohibit that in general by tumbling down a spaghetti code rabbit hole. @ThiemeNL I assume you have the TZE200 variant? Even with that, one need to know what to put into the DDF unfortunately

Having said that, @ThiemeNL could you please let us know with which value you test state/alert? Please redo with a type other than "warning device".

Smanar commented 2 months ago

In contrast to Smanar's believe that anything for Tuya is blocked, anything is workable for any device. Especially the DDF code is so rediciously simple to have all light related resource items work with DDFs that I couldn't believe it at the beginning. What is adding complexity here, admittedly, are the various Tuya hacks already in the code. This gives me the creeps and they should go as soon as possible, even with the risk of breaking stuff imho. The longer that code stays, the more difficult wil it be to remove it.

You don't remember ? with DDF to send request (all is working for incoming request), you can use only state/on state/bri (with this PR https://github.com/dresden-elektronik/deconz-rest-plugin/pull/5868 ), not possible to use state/lift for exemple, make a try on your side on a covering, you can reverse a return but not reverse the request. All is possible using "config" endpoint but not the "state".

And it's not locked for tuya, it's locked for ALL brands. You need to use defaut request or give up.

SwoopX commented 2 months ago

You don't remember ? with DDF to send request (all is working for incoming request), you can use only state/on state/bri (with this PR #5868 ), not possible to use state/lift for exemple, make a try on your side on a covering, you can reverse a return but not reverse the request. All is possible using "config" endpoint but not the "state".

And it's not locked for tuya, it's locked for ALL brands. You need to use defaut request or give up.

No, I indeed do not remember that one, thanks for sharing. That also finally helped me explaining why it is working at all with state/on, was about to say it's a coincidence and following the legacy code paths while using some unexplainable zigbee Tuya glitches 🙂 However, it's not here and a state change is truly used though located at a very unexpected location.

However, all this doesn't change anything on my overall position. Everything is possible with DDFs, so I'll neither go with the default requests nor will I give up. In essence, adding a single line of code would be sufficient but practically, it will be just a few more. The current state changes are created, but not set up correctly. Will raise some PRs for making sirens and covers work shortly.

github-actions[bot] commented 1 month ago

Hey @ThiemeNL, thanks for your pull request!

[!TIP] Modified bundles can be downloaded here. Relative expire date

DDB changes

Modified

Validation

[!CAUTION] Some errors have been reported. Please check the error annotations here and the logs here.

:clock130: Updated for commit 84492a2522bb14775e797dd223c7e76a7be79451

Zehir commented 1 month ago

Not sure what is the problem reported by the validator because the file exist. Maybe an update of the base branch could help