core
core copied to clipboard
Add august open action
Breaking change
Proposed change
Added support for opening locks
Type of change
- [ ] Dependency upgrade
- [ ] Bugfix (non-breaking change which fixes an issue)
- [ ] New integration (thank you!)
- [x] New feature (which adds functionality to an existing integration)
- [ ] Deprecation (breaking change to happen in the future)
- [ ] Breaking change (fix/feature causing existing functionality to break)
- [ ] Code quality improvements to existing code or addition of tests
Additional information
- This PR fixes or closes issue: fixes #
- This PR is related to issue:
- Link to documentation pull request:
Checklist
- [ ] The code change is tested and works locally.
- [ ] Local tests pass. Your PR cannot be merged unless tests pass
- [ ] There is no commented out code in this PR.
- [ ] I have followed the development checklist
- [ ] I have followed the perfect PR recommendations
- [ ] The code has been formatted using Ruff (
ruff format homeassistant tests) - [ ] Tests have been added to verify that the new code works.
If user exposed functionality or configuration variables are added/changed:
- [ ] Documentation added/updated for www.home-assistant.io
If the code communicates with devices, web services, or third-party tools:
- [ ] The manifest file has all fields filled out correctly.
Updated and included derived files by running:python3 -m script.hassfest. - [ ] New or updated dependencies have been added to
requirements_all.txt.
Updated by runningpython3 -m script.gen_requirements_all. - [ ] For the updated dependencies - a link to the changelog, or at minimum a diff between library versions is added to the PR description.
- [ ] Untested files have been added to
.coveragerc.
To help with the load of incoming pull requests:
- [ ] I have reviewed two other open pull requests in this repository.
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks :+1:
Hey there @bdraco, mind taking a look at this pull request as it has been labeled with an integration (august) you are listed as a code owner for? Thanks!
Code owner commands
Code owners of august can trigger bot actions by commenting:
@home-assistant closeCloses the pull request.@home-assistant rename Awesome new titleRenames the pull request.@home-assistant reopenReopen the pull request.@home-assistant unassign augustRemoves the current integration label and assignees on the pull request, add the integration domain after the command.@home-assistant add-label needs-more-informationAdd a label (needs-more-information, problem in dependency, problem in custom component) to the pull request.@home-assistant remove-label needs-more-informationRemove a label (needs-more-information, problem in dependency, problem in custom component) on the pull request.
Works flawlessly in my fresh dev docker and my yale linus L2. The question is, what if a smart lock only has lock/unlock but no open action
Hi, when will this be included in the stable version? Thank you!
Hi, when will this be included in the stable version? Thank you!
There are 2 problems afaik that need to be solved before that:
-
Coverage for one method is missing (https://github.com/home-assistant/core/pull/113795#discussion_r1530762028) I added a PR for that, but one of the checks is failing and I don't get why.
-
It should only be set for devices that support the open action (https://github.com/home-assistant/core/pull/113795#discussion_r1530761482) However, I have no idea what the best way is to solve this.
I'm waiting for feedback on these 2 problems
- missing
Can I somehow integrate this into my system now? Do you perhaps have a hint? I only have devices that support open
It still seems to take a while.
Thank you!
I'm eagerly waiting for it to be merged, too. I can see if it's possible to create a new repo and add the required files to turn It into a HACS compatible Integration. That way it might be easy to add it as custom Repository in HACS, so we can use it that way until this PR gets merged. However, I've never done this before, so not sure if it works.
2. It should only be set for devices that support the open action (Add august open action #113795 (comment)) However, I have no idea what the best way is to solve this.
Its likely going to be a PR to yalexs to add a property if it has open or not.
Hopefully the device details from the api can tell us if it supports open or not
seeing whether the door is open or not would of course also be great. is this already being worked on?
thanks!
2. It should only be set for devices that support the open action (Add august open action #113795 (comment)) However, I have no idea what the best way is to solve this.
Its likely going to be a PR to yalexs to add a property if it has open or not.
Hopefully the device details from the api can tell us if it supports open or not
I looked at the device Details and other API Request, however I could not see any info about whether opening is supported or not. We could go by the lock model and compare it to a list of models that Support opening.
We could go by the lock model and compare it to a list of models that Support opening.
If we hide that behind a property or function in yalexs we can always switch it out later to something better if we find a way to get it from the device details
We could go by the lock model and compare it to a list of models that Support opening.
If we hide that behind a property or function in
yalexswe can always switch it out later to something better if we find a way to get it from the device details
Created a PR for this https://github.com/bdraco/yalexs/pull/105 (the new assertion in test_async_get_lock_with_unlatch is failing. I don't get why atm)
I've updated my 2 PRs to yalexs https://github.com/bdraco/yalexs/pull/105 https://github.com/bdraco/yalexs/pull/103 and I've already prepared the update to this PR (PR should be finished then). If the yalexs PRs are merged and a new version is published, I can test and push my update here.
lib bump in https://github.com/home-assistant/core/pull/116511
I'm not sure yet how to access the new unlatch_supported attribute. I though that I can do something like
def __init__(self, data: AugustData, device: Lock) -> None:
[...]
if self._unlatch_supported:
self._attr_supported_features = LockEntityFeature.OPEN
but the attribute _unlatch_supported is unknown
Edit: oh, it's if self._detail.unlatch_supported:
Code looks good. Should be good to merge once tests are added
Code looks good. Should be good to merge once tests are added
Ok great. I'll Look into that tomorrow
I've added a test for the hass lock.open action. Is this sufficient or are more tests needed?
It looks like you might be manually trying to fix ruff issues.
I'd recommend installing pre-commit
https://developers.home-assistant.io/docs/development_testing/
The one check is failing because homeassistant.exceptions.HomeAssistantError: Entity lock.online_with_doorsense_name does not support this service.. That's correct because currently only devices with the Type 17 get the Unlock Support Flag set. So I guess we need a new fixture (like https://github.com/bdraco/yalexs/blob/master/tests/fixtures/lock_with_unlatch.online.json) and use that. However I currently don't get how the fixtures are loaded. It test_lock uses lock.online_with_doorsense_name however there is no fixture with that exact name. Also it would be good to assert that the current fixture lock does not have the unlock support flag set. However, that's another thing I currently don't get
You'll need to copy the fixture and than add a new one in https://github.com/home-assistant/core/blob/63484fdaddaca7ec92ed95376a94b8fb2b1daa97/tests/components/august/mocks.py#L365
Well, I just found out that I can run the tests with pytest locally... Should have read the whole Development Workflow in the docs earlier, sorry
Wow, thank you a lot, I was already frustrated, as the L1 could open the door..
Wow, thank you a lot, I was already frustrated, as the L1 could open the door..
You're welcome! Currently only the L2 will be getting the open feature, because I created a whitelist of lock models that support unlatching in the yalexs api (https://github.com/bdraco/yalexs) this component is using. Currently only model 17 (Yale Linus L2) is included. So, if you would like the L1 to have this feature too, I'll need the model number in order to add it to yalexs (or you can create a PR yourself).
The easiest way to get the model number is probably to turn on debug logging for the august component. Then you can search the logs for "Type":. You can use e.g. Studio Code Server to open /homeassistant/home-assistant.log.
If the model id is not in diagnostics, it would be nice to add it there as well
If the model id is not in diagnostics, it would be nice to add it there as well
It is already. One can search for "Type": in the debug logs, I just found out. I've updated the command above for the easier way
@bdraco The negative test is working. A lock that does not support unlatching (according to yalexs) does not have the open feature flag set. However, the positive test (https://github.com/home-assistant/core/pull/113795/commits/5996c5445f9b00bb2857f1872a5bfa17bbdf22c1) leads to the error message FAILED tests/components/august/test_lock.py::test_open_lock_operation - TypeError: object MagicMock can't be used in 'await' expression. I'm not able to find out why. Is there a mock missing in mocks.py?
I guess the positive test is the only missing thing until this can be merged?
@bdraco The negative test is working. A lock that does not support unlatching (according to yalexs) does not have the open feature flag set. However, the positive test (5996c54) leads to the error message
FAILED tests/components/august/test_lock.py::test_open_lock_operation - TypeError: object MagicMock can't be used in 'await' expression. I'm not able to find out why. Is there a mock missing in mocks.py? I guess the positive test is the only missing thing until this can be merged?
@bdraco sorry for tagging you again :) The test for devices which don't support unlatching works. The test for devices which support it does not work and I don't know how to fix that.
could someone please be so kind and help @m10x ? it has been dragging on for two months now and no end in sight :( thanks a lot!
It's on my review list. There are still ~30 PRs ahead of it. Please be patient; there are a lot of people who need help. If the test gets solved before I have a large enough time block to be able to analyze whats needed, I will come back to it sooner as I have much more small time blocks than large ones.