Skip to content

Followup LIDL/Silvercrest door bell (HG06668)#6073

Merged
manup merged 1 commit intodresden-elektronik:masterfrom
manup:master
May 24, 2022
Merged

Followup LIDL/Silvercrest door bell (HG06668)#6073
manup merged 1 commit intodresden-elektronik:masterfrom
manup:master

Conversation

@manup
Copy link
Copy Markdown
Member

@manup manup commented May 24, 2022

Additions for PR: #6071

The DDF didn't load due 0-100% "invalid utf8" in config/battery. It's actually valid but needs another fix...

Main problem is that we have different modelid/manufacturernames in the databases out there. The variants are added so the DDF is picked up for each of these. Note for later we need to have something like:

"apimodelid"
"apimanufacturername"

To express what should actually be exposed and be backward compatible, since the legacy code sometimes replaces the original Zigbee values (Lidl/Tuya).

The DDF didn't load due 0-100% "invalid utf8" in config/battery. It's actually valid but needs another fix...

Main problem is that we have different modelid/manufacturernames in the databases out there.

The variants are added so the DDF is picked up for each of these. Note for later we need to have something like:

"apimodelid"
"apimanufacturername"

To express what should actually be exposed and be backward compatible, since the legacy code sometimes replaces the original Zigbee values (Lidl/Tuya).
@manup manup added this to the v2.16.1 milestone May 24, 2022
@manup manup merged commit 3faac2e into dresden-elektronik:master May 24, 2022
@Smanar
Copy link
Copy Markdown
Collaborator

Smanar commented May 25, 2022

@manup

So the DDF can not work previously ? (I don't see the bad character)

And for the rest, I personally don't see the utility to change the real model id or the manufacture name, except create issues or make the device identification more complicated for users and devs, user are forced to use the GUI to have real one.

For me thoses 2 values need to be untouchable.

Or you are speaking for exemple for the button_map file ?

@manup
Copy link
Copy Markdown
Member Author

manup commented May 31, 2022

I tested with the device here, the legacy code did create the sensor with modified manufacturer name and modelid which caused the DDF to not be picked up.

It's a bit messy situation for some devices ;)

And for the rest, I personally don't see the utility to change the real model id or the manufacture name, except create issues or make the device identification more complicated for users and devs, user are forced to use the GUI to have real one.

Yeah back in the day it seemed to be a good idea to change these as many clients did (do?) expect unique modelids which doesn't hold true for Tuya.

But I agree it would be better in future to keep the original values and provide "changed" or more readable values with extra attributes like RAttrProduct and RAttrVendor which we already can set with DDF.

For clients I think it's best to uniquely identify a device with a combined string of manufacturername | modelid which also works with Tuya.

The biggest challenge is the backward compatibility so we need to be able to deal with legacy paired devices as the PR does.
Note there are also completely weird devices like the Trust remote control which doesn't provide a modelid at all.

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

Successfully merging this pull request may close these issues.

2 participants