Fix adv_parser for Switchbot Lock Pro #243
Conversation
- because of course they are different, why didn't I caught it earlier?!
- also add notes on which fields are still pending
|
#233 (comment) |
|
Yup, I'll remove the "haven't been tested" comment real quick. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #243 +/- ##
==========================================
+ Coverage 58.59% 58.78% +0.19%
==========================================
Files 34 35 +1
Lines 1495 1536 +41
==========================================
+ Hits 876 903 +27
- Misses 619 633 +14 ☔ View full report in Codecov by Sentry. |
Let me know how I can help with checking |
According to switchbot's documentation it's used when you group two locks together. |
Can not help here. Only have 1 lock. |
Yeah me neither lol. I guess we can just return False for both of them for now? As these features are kinda niche and I believe people really want the other part to work. We can always reintroduce these fields later, when proper data shows up. |
|
@szclsya I'm still unsure if we really should ignore When unlatching and when unlocking w/o unlatching the status is bouncing between unlocking->unlocked->not_fully_locked instead of straight forward going from unlocking->not_fully_locked. I think this should be debounced to not show up weirdly in the logs (also shows up this way in HA logs). You'll see in the logs that the I trigger
I trigger
I trigger
|
|
@jsprotky That's because the old I was confused by it too, but since it seems that 3-5 bits do correctly parsed into lock status it should be just a rearrange. |
|
According to the website, the Pro does not support Dual Lock (yet?): https://support.switch-bot.com/hc/en-us/articles/22917404813207-Can-Two-SwitchBot-Lock-Pro-Devices-Be-Set-up-for-Dual-Lock I have found two issues with the current state, which might be solved by this PR, or maybe there is something else missing:
I'm happy to assist with testing or might try a fix myself, if I have the time. |
Had the same issue and this is now solved with this PR. |
I had this as well but it got fixed at some point. My lock did not have the door sensor attached at first. Once I added that and ran a re-calibration, HA picked up the lock as being a European lock with latch. This was prior to this PR. |
- thus mark both field as False at all time
|
@szclsya So I guess this PR is not a draft anymore but rather finalized. |
switchbot/adv_parsers/lock.py
Outdated
| res = { | ||
| "battery": data[2] & 0b01111111 if data else None, | ||
| "calibration": bool(mfr_data[7] & 0b10000000), | ||
| "status": LockStatus(int(mfr_data[7] & 0b00111000) / 8), |
There was a problem hiding this comment.
Wouldn't >> 3 instead of / 8 more clearly convey what is happening here?
There was a problem hiding this comment.
Ah it was some leftover of debugging to see if they changed how the lock status is encoded (thankfully they didn't). I'll change it to >>3.
|
The new |
|
@szclsya do you know if the integration can detect the state of both batteries when using the rechargeable Dual Power Pack? The app does report each separately. Please let me know if you need any info. |
|
@mwolter805 As mentioned in the issue please provide passive scan data. But I doubt this data is actually sent passively. It's more likely to be a response of a active mode command. |
I already tried to search this info in the passive data but I didn't find any related information as I reported here. Obviously would be great if I'm wrong 😉 |
|
@szclsya Please forgive my ignorance, can I create and run that python script in the home assistant container? Edit: I believe I was able to grab the logs through the debug command in the HA integration Battery A 100%, Battery B RemovedLocked from App Unlocked from App Battery B 99%, Battery A removedLocked from App Unlocked from App Battery A 100%, Battery B 99%Locked from App Unlocked from App |
|
@mwolter805 Just as @ollo69 speculated the detailed battery info is indeed not sent in passive mode, so I'd assume there's a separate active mode command to check these information. Anyhow it will be out of scope of this PR (since it's about paring passive mode data). Maybe open a separate issue or asking in the general Lock Pro issue? |
|
Any more changes, or is this ready to merge? |
|
@bdraco Not sure if we should add the manual field (it's incorrect when locked by auto lock) but other than that we should be good. |
I'd keep it out for now unless its strictly needed for HA |
- as it's not needed by HA for now and is actually inaccurate
|
@bdraco All good now then. |
|
This could also use a test with a few different advertisements so we know it won't break in the future. |
|
@bdraco True. Will add some in another PR when I have time. |
|
if you don't have time I can create bump PR in HA for you, let me know.? |
|
@ollo69 Yeah sure, go ahead! |
Turns out the lock pro indeed has a different passive mode data format (duh), and we will need a separate parser for it.
Currently only these fields are tested to work:
And these are taken directly from the original lock, thus needs more testing on appropriate hardware:
Put it in
adv_parsers/lock.pysince code for lock pro active mode also lives side by side with the original lock atdevices/lock.py.