-
-
Notifications
You must be signed in to change notification settings - Fork 29.9k
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 Tado fan mode #120809
Fix Tado fan mode #120809
Conversation
Hey there @chiefdragon, @erwindouna, mind taking a look at this pull request as it has been labeled with an integration ( Code owner commandsCode owners of
|
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.
Thanks for providing a quick fix, @EtienneSOU!
for option in options | ||
if tado_to_ha_mapping.get(option) is not None | ||
] | ||
if not supported_fanmodes: |
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.
Put this above, to keep the early-return principle. It saves some unnecessary processing done above, if its None.
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.
I don't think I can put it above as this if statement is to return a None if I get a list[None] .
The main goal of this function is to avoid duplicate code between fanLevel and fanSpeed and return the good type for supported fan modes that should be a list[str] | None.
If i don't keep this if statement here, I will return list[str | None].
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.
I see, makes sense. Thanks for the quick fix, let's see if we can get this in Beta!
@frenck Can this please be added in Beta? |
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.
Thanks, @EtienneSOU 👍
../Frenck
Breaking change
Proposed change
Fix fan levels over 3.
Type of change
Additional information
Checklist
ruff format 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
.To help with the load of incoming pull requests: