Skip to content

Fix ambiguous loading of button maps#3918

Merged
manup merged 2 commits intodresden-elektronik:masterfrom
manup:master
Dec 14, 2020
Merged

Fix ambiguous loading of button maps#3918
manup merged 2 commits intodresden-elektronik:masterfrom
manup:master

Conversation

@manup
Copy link
Copy Markdown
Member

@manup manup commented Dec 14, 2020

  • The missing break, did load all entries which started with a modelid. So lumi.sensor_switch.aq2 was overwritten by lumi.sensor_switch;
  • Prevent single press button event to be emitted twice for lumi.sensor_switch.aq2;
  • Don't use QString::startsWith() always require exact modelId;
  • Put manufactuer name of Tuya switch in button map and use it as modelId;
  • Add helper isTuyaManufacturerName() to detect Tuya devices.

Issue: #3719

- The missing break, did load all entries which started with a modelid. So lumi.sensor_switch.aq2 was overwritten by lumi.sensor_switch;
- Don't use startsWith() always require exact modelId;
- Put manufactuer name of Tuya switch in button map and use it as modelId;
- Add helper isTuyaManufacturerName() to detect Tuya devices.

Issue: dresden-elektronik#3719
@SwoopX
Copy link
Copy Markdown
Collaborator

SwoopX commented Dec 14, 2020

You might want to add the isTuyaManufacturerName() here as well without any action

else if ((node->nodeDescriptor().manufacturerCode() == VENDOR_OSRAM_STACK) || (node->nodeDescriptor().manufacturerCode() == VENDOR_OSRAM))

to prevent any sensor assuming Heiman based on the MFC.

@manup
Copy link
Copy Markdown
Member Author

manup commented Dec 14, 2020

You might want to add the isTuyaManufacturerName() here as well without any action

else if ((node->nodeDescriptor().manufacturerCode() == VENDOR_OSRAM_STACK) || (node->nodeDescriptor().manufacturerCode() == VENDOR_OSRAM))

to prevent any sensor assuming Heiman based on the MFC.

Good point, yes for Tuya we absolutely need to query the actual manufacturer code name.

Edit: But at this place a Tuya device should never fall into the if statement since the vendor code isn't used by them irk?

@manup manup merged commit 515b361 into dresden-elektronik:master Dec 14, 2020
@SwoopX
Copy link
Copy Markdown
Collaborator

SwoopX commented Dec 14, 2020

Well, we have exactly that assignment a bit further below in the if-else jungle. The marked passage above was just to illustrate where it should be placed. Apparently, VENDOR_EMBER has already been silenced.

else if ( //node->nodeDescriptor().manufacturerCode() == VENDOR_EMBER ||
             node->nodeDescriptor().manufacturerCode() == VENDOR_HEIMAN)
    {
        sensorNode.setManufacturer("Heiman");
    }

Espacially that passage gave me a hard time back in the days changing the manufacturer out of the blue. From my perspective, the function should be applied and put on the very top of the if-else statements for consistency and to retain the original manufacturer name undoubtly.

@manup
Copy link
Copy Markdown
Member Author

manup commented Dec 14, 2020

I'd like to remove the manual manufacturer name setup for all vendor codes which are shared across different manufacturers.
And then if it is empty it needs to be queried from the device (which should already happen automatically).

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