Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add basic support for lock pro #241

Merged
merged 16 commits into from
Jun 18, 2024
Merged

Add basic support for lock pro #241

merged 16 commits into from
Jun 18, 2024

Conversation

szclsya
Copy link
Contributor

@szclsya szclsya commented Jun 7, 2024

Note that since I've only tried manually creating SwitchbotLock interface, the adv_parser part and the model name is mostly made up for now.

Also since we only need to change some constants to make it work, I just added a enum inside the SwitchbotLock to differentiate between the two models.

@szclsya
Copy link
Contributor Author

szclsya commented Jun 7, 2024

Looks like the third last number in the lock/unlock command (i.e 0f4e0101000180) encodes the origin of such command:

  • 0: Unknown (Shows up as blank in the app)
  • 1: App Control
  • 2: Widget
  • 3: Apple Watch
  • 4: Scene
  • 5: NFC
  • 6: OpenAPI
  • 7: Unknown
  • 8: Alexa
  • 9: Google Assistant
  • a: SmartThings
  • b: IFTTT
  • c: Matter
  • d: Siri
  • e: Manual
  • f: Keypad

Maybe we should use 6: OpenAPI?

switchbot/devices/lock.py Outdated Show resolved Hide resolved
@bdraco
Copy link
Member

bdraco commented Jun 8, 2024

I removed python 3.10 since it doesn't support StrEnum and fixed the lint issues.

This needs test coverage before merging as well

@szclsya
Copy link
Contributor Author

szclsya commented Jun 8, 2024

Added active mode test case. Any idea how can I grab some passive mode data? I ran discover in a loop for a while but only ever got active mode data from the lock.

@bdraco
Copy link
Member

bdraco commented Jun 8, 2024

You can pull it out of diagnostics in Home Assistant from a local Bluetooth adapter or an esphome remote proxy

@szclsya
Copy link
Contributor Author

szclsya commented Jun 8, 2024

Hmm weird, from my esphome bluetooth proxy it's still active mode data. Any idea?

@szclsya szclsya marked this pull request as ready for review June 8, 2024 21:48
@schummar
Copy link

schummar commented Jun 8, 2024

I have the EU lock and logged my bluetooth traffic. COMMAND_UNLOCK_WITHOUT_UNLATCH should be 0f4e01010001a0

@bdraco
Copy link
Member

bdraco commented Jun 8, 2024

Hmm weird, from my esphome bluetooth proxy it's still active mode data. Any idea?

What does your yaml look like?

@szclsya
Copy link
Contributor Author

szclsya commented Jun 9, 2024

Should be good now. Didn't know I need to trim the b'\x0f\xffi\t' out of the advertisement data gathered from passive ble monitor log.

@codecov-commenter
Copy link

codecov-commenter commented Jun 9, 2024

Codecov Report

Attention: Patch coverage is 64.28571% with 5 lines in your changes missing coverage. Please review.

Project coverage is 58.78%. Comparing base (45bf527) to head (c5ec00b).
Report is 12 commits behind head on master.

Current head c5ec00b differs from pull request most recent head 30ded48

Please upload reports for the commit 30ded48 to get more accurate results.

Files Patch % Lines
switchbot/discovery.py 0.00% 3 Missing ⚠️
switchbot/devices/lock.py 71.42% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #241      +/-   ##
==========================================
+ Coverage   58.59%   58.78%   +0.18%     
==========================================
  Files          34       35       +1     
  Lines        1495     1531      +36     
==========================================
+ Hits          876      900      +24     
- Misses        619      631      +12     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

switchbot/devices/lock.py Outdated Show resolved Hide resolved
switchbot/devices/lock.py Outdated Show resolved Hide resolved
@bdraco
Copy link
Member

bdraco commented Jun 10, 2024

I think this is close to ready to merge.

Do you have a Home Assistant PR to match up to this?

I'll try to take a look tomorrow, but it might not be until Tuesday that I come back to this as I have a busy day tomorrow and its super late here.

@ynsgnr
Copy link

ynsgnr commented Jun 10, 2024

Thank you so much for the work @szclsya, tested it with EU Lock Pro and it works well! Tested unlocking, locking, status and night latch unlocking!

I think model detection can be improved by detecting Lock or Lock Pro automatically. Probably by using the same enum (SwitchbotModel) instead of two different enums (SwitchbotModel , LockModel)

That would allow a lock construction like this:

wolock = await GetSwitchbotDevices().get_locks()['32:C0:00...']
lock.SwitchbotLock(wolock.device, "....", "....", model=wolock.data["modelName"])

@szclsya
Copy link
Contributor Author

szclsya commented Jun 10, 2024

@ynsgnr Good idea! Updated.

@szclsya
Copy link
Contributor Author

szclsya commented Jun 10, 2024

@bdraco I have a Home Assistant patch working locally now. Will send a pull request after this pull request is merged.

@bdraco
Copy link
Member

bdraco commented Jun 10, 2024

@bdraco I have a Home Assistant patch working locally now. Will send a pull request after this pull request is merged.

It would be super helpful for review purposes if you linked the branch, or opened a draft PR to HA core so I can look at the whole change.

switchbot/__init__.py Outdated Show resolved Hide resolved
@szclsya
Copy link
Contributor Author

szclsya commented Jun 10, 2024

@bdraco See Home Assistant PR at home-assistant/core#119326

@ynsgnr
Copy link

ynsgnr commented Jun 17, 2024

@bdraco gentle reminder to take a look at the PR, just wanted to remind 😇

@bdraco
Copy link
Member

bdraco commented Jun 18, 2024

Haven't forgotten. Just haven't had any free time to look

switchbot/devices/lock.py Outdated Show resolved Hide resolved
Copy link
Member

@bdraco bdraco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @szclsya

@bdraco bdraco merged commit 48398d0 into sblibs:master Jun 18, 2024
3 checks passed
@bdraco
Copy link
Member

bdraco commented Jun 18, 2024

Please be sure to name future branches something besides master as it means I have to do a lot more work to pull them down to review which makes reviews take longer.

Thanks

@bdraco
Copy link
Member

bdraco commented Jun 18, 2024

released as 0.48.0

Please bump the library in a separate PR from home-assistant/core#119326 and than merge in dev to home-assistant/core#119326 once the bump PR is merged

@szclsya
Copy link
Contributor Author

szclsya commented Jun 19, 2024

Please be sure to name future branches something besides master as it means I have to do a lot more work to pull them down to review which makes reviews take longer.

Thanks

Roger that. Sorry for the inconvenience.

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.

5 participants