Fix IKEA double battery percentage for old/new firmware #2948
Conversation
We always double the battery pct first if the firmware is unknown (for devices which have the doubling clusters, very new IKEA devices do not at all). But if the firmware is unknown, we cause a read to be sent (as the device is hopefully awake). If that read now returns a new firmware, we "undouble" the battery pct again, so it's correct. With the previous commit, it would only be correct if the device sends another update. With this commit, we have a short flash of the doubled battery pct, then go to the correct value IF the device responded to our read.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## dev #2948 +/- ##
==========================================
+ Coverage 87.64% 87.67% +0.03%
==========================================
Files 297 297
Lines 9055 9079 +24
==========================================
+ Hits 7936 7960 +24
Misses 1119 1119 ☔ View full report in Codecov by Sentry. |
|
"New": The device is very likely to be awake when |
This is for reading the attribute on a new join/repairing/reconfiguration, or refreshing the attribute in the two later cases. The `sw_build_id` attribute is not updated after zigpy performed an OTA update. It will still be in cache, so our existing code won't update it in that case, only a reconfiguration will with this code.
|
If the controller is on current production firmware its Zigbee3 and have / using pull control. |
|
This seems to be working nicely. |
|
I've added another small commit to guard against possible future version formatting where Again, this should be good to go and if nobody has anything against this, I'll merge it tomorrow and release/bump quirks for the next HA beta. |
|
I can normally testing 90% of IKEA controllers with current production firmware (and reversing to old if needed with SWD flashing if very needed) but im 810 Km from my systems so cant doing testing and im going back in one week if all is going well. Great work done !! |
|
Just a side note: This version is running on my side since the release and still I see double battery. Still same issue. Will wait for some more time and if it does not change will report in a new ticket with logs. |
|
Were you able to read Please open a new issue and provide the diagnostic information for the affected devices. |
|
Yes. I could read with success with the help of a press on the button(s). And rebinding is anyhow completely waking the device. Since you are so quick answering I will try to get you the diagnostics this evening. :) |
|
Nice. New issue would indeed be good for that, but do you roughly remember the version that it showed off the top of your head? (e.g. 2.3.075 or 24.4.5) And which device is affected? |
We always double the battery pct first if the firmware is unknown (for devices which have the doubling clusters, very new IKEA devices do not at all). But if the firmware is unknown, we cause a read to be sent (as the device is hopefully awake). If that read now returns a new firmware, we "undouble" the battery pct again, so it's correct. We also always read `sw_build_id` on `bind()`
Proposed change
This fixes the issue of some battery-powered IKEA devices having double the percentage reported.
The "double battery pct" cluster is still used on some devices which can have new or old firmware (where doubling may or may not be needed depending on the installed firmware version).
The fix would be simple if ZHA/zigpy cached the
sw_build_idattribute when pairing/reconfiguring a device.But that doesn't happen. So, we need to read the attribute once to cache it while the device is awake.
(That would also be required if we want to use
sw_build_idwith quirks v2, because we can't magically read an attribute of battery-powered devices that are already paired and asleep.)The diff might look huge at first (but ignore the tests) and the fix is mostly somewhat simple actually.
-> Start by looking at
_update_attribute()The "fix"
The "fix" works as follows:
sw_build_idattribute value from cache like:24.0.0), we split on.and check the first part, here: 24sw_build_idin the backgroundsw_build_idbeing unknown AND the read is successful:Note: If the attribute read for
sw_build_idis successful (or it's already known), no (further) read calls will ever be made.EDIT: For (new) pairings or reconfigurations,
sw_build_idis now also read on pairing/bind().Additional information
Note part 1 / TODO:
I did not yet test if an IKEA device responds to attribute reads when it sends an attribute report with the remaining battery percentage. I assume it would, as it should be "awake" at that point, but I still have to test this.(So far, I only wrote the unit tests to test this behavior.)
If, for some reason, it does not wake up to receive message in that case, I'd move the logic to read thesw_build_idattribute once (until it's in cache) to happen when a button is pressed (for a remote).Because then, a remote is definitely ready to receive commands for a short period.
Seems to work as expected. A button push during longer inactivity of the remote also triggers an attribute report, thus causing us to read
sw_build_idwhile the device is awake.Note part 2:
Or, if we do not want this "read schedule logic" at all (for now), we can also just check the
sw_build_idfrom attribute cache and if it's unknown, not schedule a read call.This might cause weird behavior though if a user who previously manually read
sw_build_idand has correct percentages then removes and re-pairs the device, only now to have doubled percentages (becausesw_build_idisn't in the attribute cache).(That's fixed by scheduling the read at that point.)
Note part 3 (EDIT):
We're now also reading
sw_build_idduring pairing/reconfiguration, so duringbind().This will not automatically fix the issue for existing users who currently have doubled percentages, but it will fix it if the device is paired again or reconfigured.
I'm hoping that the "task to schedule a read" will still work to fix existing installations automatically.
If it doesn't work, the "making users reconfigure their device to read
sw_build_idand fix the issue"-approach is also fine I guess.(I'm waiting for an attribute report from the IKEA remote for now to see if it also receives commands for a short period then.)Initial approach
My initial approach did not re-update the attribute cache after a successful attribute read if the initial doubling was wrong.
The successful attribute read only updated the attribute cache, so only next time the device sends something, the percentage would be right. You can still see this approach in my initial commit.
cc @dmulcahey just to know if this approach/PR would be somewhat okay. I know it might not be ideal to have to read attributes in the background, but it's only done once (if successful) and we need the
sw_build_idsooner or later anyway.A similar-ish approach with the background tasks is used for some Tuya devices and EU Xiaomi plugs (although not for firmware checking).
Checklist
pre-commitchecks pass / the code has been formatted using Black