Shelly cover position update when moving
Proposed change
This change forces Shelly cover integration to regularly update the cover position while it is moving.
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:
- Link to developer documentation pull request:
- Link to frontend pull request:
Checklist
- [x] The code change is tested and works locally.
- [x] Local tests pass. Your PR cannot be merged unless tests pass
- [x] There is no commented out code in this PR.
- [x] I have followed the development checklist
- [x] I have followed the perfect PR recommendations
- [x] 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.
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 @balloob, @bieniu, @thecode, @chemelli74, @bdraco, mind taking a look at this pull request as it has been labeled with an integration (shelly) you are listed as a code owner for? Thanks!
Code owner commands
Code owners of shelly 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 shellyRemoves 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.
While the idea is nice and I would personally love to have a status update for covers while they are moving, I have asked Shelly developers in the past why they don't send status updates while moving and they explained that this put too much pressure on the device. I don't like adding more to it by constantly polling the device. I will ask Shelly developers again if this may be changed in the device as it would be the correct solution.
I did try to contact them through their forum here : https://community.shelly.cloud/topic/2554-shelly-plus2pm-doesnt-send-its-position-to-mqtt-or-rpc-while-moving/ Note that the polling only occurs once per second, I believe it won't be too much work for the device ?
I was wondering if maybe newer devices (mine is the Plus2PM) would be more capable of frequent position update ? Anyway thanks for asking the shelly devs, if they can do it on their side, this PR would not be useful anymore but that would make more sense to me
There hasn't been any activity on this pull request recently. This pull request has been automatically marked as stale because of that and will be closed if no further activity occurs within 7 days. If you are the author of this PR, please leave a comment if you want to keep it open. Also, please rebase your PR onto the latest dev branch to ensure that it's up to date with the latest changes. Thank you for your contribution!
@thecode Hi, sorry for the ping, do you have any news from the shelly developpers ?
@thecode Hi, sorry for the ping, do you have any news from the shelly developpers ?
Shelly replied that they can't send the status constantly as currently any status update from the device is sent to all channels which will spam their cloud.
They are working on a way to separate update channels but no timeline for this.
Oh I see, my use case doesn't rely on the servers at all but I can understand that it represent a lot of additional data for them
I think since Shelly team doesn't know when they will implement another solution we can continue with this PR.
Tests are needed.
I think since Shelly team doesn't know when they will implement another solution we can continue with this PR.
Tests are needed.
I updated the PR according to the latest commit on dev. What kind of tests would you expect? I have to admit I am not used to python testing so any help would be welcome
What kind of tests would you expect?
We need a test that proves the new code works. You can extend this test https://github.com/home-assistant/core/blob/d0fe7de501620a5cd5c2e8b134adb097b7f6287b/tests/components/shelly/test_cover.py#L112 by changing the position when closing or opening, like here for slat_pos https://github.com/home-assistant/core/blob/d0fe7de501620a5cd5c2e8b134adb097b7f6287b/tests/components/shelly/test_cover.py#L259
What kind of tests would you expect?
We need a test that proves the new code works. You can extend this test
https://github.com/home-assistant/core/blob/d0fe7de501620a5cd5c2e8b134adb097b7f6287b/tests/components/shelly/test_cover.py#L112 by changing the position when closing or opening, like here for
slat_poshttps://github.com/home-assistant/core/blob/d0fe7de501620a5cd5c2e8b134adb097b7f6287b/tests/components/shelly/test_cover.py#L259
I ended up creating separate tests. Coverage seems ok with the new code.
Please mark PR as "Ready for review" if ready.
Any chance will be merged soon?
Sorry for the copilot noise, I wasn't expected it to show on the PR directly when asking it to review my code on visual studio code. That said it had a valid point in adding more information to the update task.
Tests should be good now !