-
-
Notifications
You must be signed in to change notification settings - Fork 30.8k
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
Add fan support for KNX climate entities #126368
Conversation
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.
Hey there @Julius2342, @farmio, @marvin-w, mind taking a look at this pull request as it has been labeled with an integration ( Code owner commandsCode owners of
|
I was thinking about how to do this configuration in UI. With free-form integer modes like the current implementations If we used In [5]: def percentage_steps(steps: int) -> list[str]:
...: return [str(int(100/steps*(step+1))) for step in range(steps)]
...:
In [8]: percentage_steps(1)
Out[8]: ['100']
In [7]: percentage_steps(2)
Out[7]: ['50', '100']
In [6]: percentage_steps(3)
Out[6]: ['33', '66', '100']
In [9]: percentage_steps(4)
Out[9]: ['25', '50', '75', '100']
In [10]: percentage_steps(5)
Out[10]: ['20', '40', '60', '80', '100']
In [16]: percentage_steps(10)
Out[16]: ['10', '20', '30', '40', '50', '60', '70', '80', '90', '100'] which could also use built-in FAN_HIGH, FAN_LOW, FAN_MEDIUM for <4 steps. Alternatively A third option would be to let users configure key-value pairs instead of a list. This would be the most flexible option. fan_modes:
"Low": 20
"Bit more": 30
"Medium": 50
"Blow away": 100 What do you think? |
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.
Thank you very much for your contribution. 👍
Here are some thoughts and stylistic suggestions.
@farmio I followed your advice, number of steps apply to both |
I tested this locally as well with my thermostat. This PR depends on XKNX/xknx#1573 and a release of xknx. I will work on the rest of the checklist in the description. |
2a52311
to
98da39f
Compare
Co-authored-by: Matthias Alphart <[email protected]>
Co-authored-by: Matthias Alphart <[email protected]>
Co-authored-by: Matthias Alphart <[email protected]>
Co-authored-by: Matthias Alphart <[email protected]>
cb58c72
to
a9bf06c
Compare
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.
Nice! Thank you very much! 👍
* Add fan mode support to knx climate * fix linting errors * remove unneeded None protection from CONF_FAN_PERCENTAGES_MODES * Update homeassistant/components/knx/climate.py Co-authored-by: Matthias Alphart <[email protected]> * Update homeassistant/components/knx/climate.py Co-authored-by: Matthias Alphart <[email protected]> * Update homeassistant/components/knx/climate.py Co-authored-by: Matthias Alphart <[email protected]> * Update homeassistant/components/knx/schema.py Co-authored-by: Matthias Alphart <[email protected]> * find closest percentage when not in fan modes * new field for fan speed mode, max steps apply to both step and percentage * not picking FAN_OFF when the percentage is closest to zero * add fan zero mode to support auto mode * use StrEnum for FanZeroMode * change default to 'percent' * fix mypy errors --------- Co-authored-by: Matthias Alphart <[email protected]>
Breaking change
Proposed change
Add fan to knx climate device.
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
.To help with the load of incoming pull requests: