-
-
Notifications
You must be signed in to change notification settings - Fork 30k
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
Fix RTS device delays in Overkiz integration #67124
Conversation
Hey there @vlebourl, @tetienne, mind taking a look at this pull request as it has been labeled with an integration ( |
13f03ef
to
33630fa
Compare
from pyoverkiz.models import Command, Device | ||
from pyoverkiz.types import StateType as OverkizStateType | ||
|
||
from .coordinator import OverkizDataUpdateCoordinator | ||
|
||
# Commands that don't support setting | ||
# the delay to another value | ||
COMMANDS_WITHOUT_DELAY = [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This would be slightly more efficient if it was a set()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good suggestions @bdraco. I will have a look if we have other places where we don't need an ordered list and thus can use a set. Will do this in a follow up PR some day :).
self.device.protocol == Protocol.RTS | ||
and command_name not in COMMANDS_WITHOUT_DELAY | ||
): | ||
args = args + (0,) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could be args += (0,)
Proposed change
Currently users face issues when the execute a command for an RTS device (stateless). When they for example open their cover, they can only stop it after 30 seconds. By default, Overkiz sets a duration of 30 seconds on every command, while we want to have this set to 0.
This PR will fix this by setting the duration argument (last arg) to zero on every command, except on a few commands that don't support this.
Type of change
Additional information
Checklist
black --fast homeassistant tests
)If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
Updated and included derived files by running:
python3 -m script.hassfest
.requirements_all.txt
.Updated by running
python3 -m script.gen_requirements_all
..coveragerc
.The integration reached or maintains the following Integration Quality Scale:
To help with the load of incoming pull requests: