Add support for Tapo lock device DL110 and implement lock control features #1646
Conversation
cristian-rincon
commented
Jan 15, 2026
cristian-rincon
commented
- Introduced new device type for Tapo lock in device_type.py
- Updated device family to include Tapo lock in deviceconfig.py
- Added lock control commands in cli/lock.py
- Implemented lock and lock history modules in smartcam
- Created tests for lock module and history functionality
- Added JSON fixture for Tapo lock device
cristian-rincon
changed the title
|
@cristian-rincon Huge thanks for pulling this together. I would suggest instead of making it a SMARTCAM device, it be under just the SMART device category. You may need to make adjustments to the device_factory.py and discover.py to handle making sure the correct protocol and transport are used since it looks like this device_type uses the SSL AES Transport, but there should be a clear separation here since it is not a camera at all. Also, when submitting, making sure that all checks pass is best. To help with submissions, in your local repository folder run these commands following commands: uv sync --all-extras That will setup a local Python .venv and run all the test coverages and make sure the code is following existing guidelines. You can fix errors and make sure all lines of code are covered for CodeCov as well. |
|
@cristian-rincon Will this also support the DL100? If not, anything I can do to help make that so? |
rytilahti
left a comment
rytilahti
left a comment
There was a problem hiding this comment.
Thanks for the PR, @cristian-rincon! I added some initial feedback inline, please take a look and let me know what you think.
| # lock commands runnnable at top level | ||
| "unlock": "lock", | ||
| "history": "lock", |
There was a problem hiding this comment.
Let's avoid polluting the main command namespace given how specific these are to the locks (albeit history could be something similar to trigger logs on some devices, but that's for another discussion).
| "Lock", | ||
| "LockEvent", | ||
| "LockMethod", |
There was a problem hiding this comment.
Interfaces are used to provide common interface for different device families, but as there are currently no locks for other types (iot, smart), let's avoid abstracting things and adding these interfaces until we have a need for the level of abstraction.
| assert hasattr(lock, "is_locked") # type: ignore[attr-defined] | ||
| assert hasattr(lock, "battery_level") # type: ignore[attr-defined] | ||
| assert hasattr(lock, "auto_lock_enabled") # type: ignore[attr-defined] |
There was a problem hiding this comment.
These are already tested by the code below.
| assert lock | ||
|
|
||
| # Test properties exist and can be accessed | ||
| is_locked = lock.is_locked # type: ignore[attr-defined] |
There was a problem hiding this comment.
There should be no need to ignore the type for every individual check. Does not adding a type hint to the assignment fix the type for all the checks below?
Same applies to check in other test functions in this file.
| "SMART.TAPOLOCK": SmartCamDevice, | ||
| "SMART.TAPOLOCK.HTTPS": SmartCamDevice, |
There was a problem hiding this comment.
Isn't this always HTTPS as the test implies?
| Vacuum = "vacuum" | ||
| Chime = "chime" | ||
| Doorbell = "doorbell" | ||
| DoorLock = "doorlock" |
There was a problem hiding this comment.
I was thinking whether this should be Doorlock to keep the consistency with the doorbell above, but would Lock be enough? It is what you also use in the cli, and it is descriptive enough, IMO.
| def _get_lock_status(self) -> dict: | ||
| """Get lock status data.""" | ||
| return self.data.get(self.QUERY_GETTER_NAME, {}) | ||
|
|
||
| def _get_lock_config(self) -> dict: | ||
| """Get lock config data.""" | ||
| return self.data.get("getLockConfig", {}) |
There was a problem hiding this comment.
Would it be possible to use dataclasses for the responses to allow typing and letting mashumaro to deal with the validation?
| self._add_feature( | ||
| Feature( | ||
| device, | ||
| id="lock_battery_level", |
There was a problem hiding this comment.
If you would use battery_level here, this would be exposed correctly to homeassistant (and other downstreams) without extra configuration. Or would that collide with the battery module on this device?
4 participants
