dresden-elektronik / deconz-rest-plugin

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

Sensors do not get loaded or assume a different ID than in the DB #5965

Closed SwoopX closed 2 years ago

SwoopX commented 2 years ago

Describe the bug

To be updated, as this is complex to describe. However, the consequences are easy to describe, as sensor IDs get assigned more than once. This could lead to sensors not being shown in REST API or sensors getting stored with wrong IDs.

Note that in the screenshots below, sensor ID 154 is used twice. First occurance of 154 would be correct (see database screenshots)

Root cause is that when using DDFs, "legacy" sensors are not loaded via sqliteLoadAllSensorsCallback(), but skipped. Therefore, vector plugin->sensors has an inadequate size, which should reflect all sensors, not just a few.

Interestingly, the DDF code tries to load the subdevices a little later and to determine if a new sensor ID is required or not and this is where it gets bad. Due to that wrong size, getFreeSensorId() assumes any left out sensor ID is free and available, although this is not (necessarily) the case. Once the subdevice is loaded/created, the sensor gets pushed to the sensors vector via DEV_AddResource(), which is too late to have been previously considered as it should be.

Steps to reproduce the behavior

Conditions to reproduce are not fully clear However, it requires 2 DDF devices having valid sensors in the sensors DB table. One of those devices must have the correct amount of subdevices in table sub_devices, the oser one must have more than 1 sub device according to DDF, where one of the subdevices is non-existent in table sensors. It is important for that missing sensor, a new "free" sensor ID will get selected. Additionally, some further debug output is required, otherwise it is very hard to implicitly spot the issue.

Expected behavior

Screenshots

grafik

grafik

Respective DB entries:

grafik

Overall DB entries in sensors, illustrating sensors vector is populated too late:

grafik

Sensors getting overwritten by each other:

grafik

Environment

deCONZ Logs

Additional context

The same issue or something similar might eventually apply for light resources as well (hasn't been checked from my end).

manup commented 2 years ago

PR is updated and should fix the issue. It also removes duplicated sensors and keeps the one with the lowest id.

SwoopX commented 2 years ago

Indeed. As far as I can tell, that resolves the issue

grafik

grafik

grafik

The missing sensor got the true first free ID

grafik

manup commented 2 years ago

Nice so remaining todo is only the case insensitive checks for manufacturer name. We might also ditch the 0006/1000 separation of Sunricher DDFs in favor of 0006, if a ZHASwitch exists with 1000 in the db it will be picked up automatically.

SwoopX commented 2 years ago

Yeah, I guess that would aid in keeping things clean(er).