dresden-elektronik / deconz-rest-plugin

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

FIX permit the Profalux covering Support. #7510

Closed Smanar closed 6 months ago

Smanar commented 10 months ago

See https://forum.phoscon.de/t/adding-profalux-zigbee-roller-shutters-via-legacy-code/4372

Ok So I m agree this device is "hacky", but without this code no way to support. It just have no model id and no Manufacture Name, so the legacy code can support it with this hack

        //Add missing values for Profalux device
        if (existDevicesWithVendorCodeForMacPrefix(node->address(), VENDOR_PROFALUX))
        {
            //Shutter ?
            if (i->deviceId() == DEV_ID_ZLL_COLOR_LIGHT)
            {
                lightNode.setManufacturerName(QLatin1String("Profalux"));
                lightNode.setModelId(QLatin1String("PFLX Shutter"));
                lightNode.setNeedSaveDatabase(true);
            }
        }

But this one is disabled whith the recent code.

PS : Profalux have only this device and since years, with same problem, and no support using DDF planned/possible.

SwoopX commented 10 months ago

Just thinking here: maybe instead of letting the device pass there, assining a manufacturer name and modelID could allow using a DDF regardless?

Smanar commented 10 months ago

Just thinking here: maybe instead of letting the device pass there, assining a manufacturer name and modelID could allow using a DDF regardless?

Honnestly, this device need so much hack, more than tuya stuff, realy.

I can deplace the hack from the bottom code to put it on top of the check, it will make sepraration from code and hack. But not possible using DDF for this device, with DDF we can "custom" return but not request (we have same problem for exotic covering/siren)

SwoopX commented 10 months ago

Hm, yeah. It's easy to assume things but if you can't verify them...

Guess this one's ok, if Manuel agrees

Smanar commented 10 months ago

Hm, yeah. It's easy to assume things but if you can't verify them...

You mean about "hack", I m sure of that, I m the guilty (and at 100% so I know well this device), just make a search on the code with "profalux"

If it's about DDF, you can make all you want for "config" but for "state" you have limited action possibles, you can make something for "state/on" request but nothing for "state/open" (except on return ofc)

BEskandari commented 10 months ago

Any chance that this PR would be included in the next version 2.25.2?

manup commented 9 months ago

Hi I had a look into the Proflux devices and also how other projects try to deal with them.

+++ rant on +++ To be frank they should be outright banned from the community and send back to the manufacturer. Not even providing manufacturer name and modelid should be a no go. This totally messes up the integration approach to identify the device which is used by all projects and requires addition hacks for a niche device. +++ rant off +++

I've post boned the PR to v2.26.x since I'm not willing to introduce yet another hack which will be forgotten soon and becomes a legacy which is hard to remove, especially since no developer has the device (?).

In v2.26.x there will be additions to query and provide more data for DDF matching via matchexp, like in this case the JS expression can test the manufacturer code and cluster details and than assign a DDF and use the static manufacturer name and modelid.

BEskandari commented 6 months ago

Hello @Smanar, if I understand we are not going to include your change in Deconz? and I presume that no chance to have a DDF with the matchexp to support those devices?

Smanar commented 6 months ago

Ha fuck, my bad. I have cleaned my fork and deleted all PR, I have deleted this one by error.

Manup have added it on milestone V2.26.0.

I will remake one, give me 10 mn

New PR https://github.com/dresden-elektronik/deconz-rest-plugin/pull/7735

Smanar commented 6 months ago

And haven't checked yet for new option for matchexp

We need one that check Manufacture Number, and haven't see information yet about it.