Skip to content
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

Climate: Added humidity support #1586

Merged
merged 3 commits into from
Oct 20, 2024
Merged

Climate: Added humidity support #1586

merged 3 commits into from
Oct 20, 2024

Conversation

alexkn
Copy link
Contributor

@alexkn alexkn commented Oct 19, 2024

Description

Humidity support for climate device.
I want to add it to Home Assistant.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Code quality improvements to existing code or addition of tests

Checklist

  • The documentation has been adjusted accordingly
  • Tests have been added that prove the fix is effective or that the feature works
  • The changes are documented in the changelog (docs/changelog.md)

Copy link

codecov bot commented Oct 19, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.92%. Comparing base (7f6bcfb) to head (43218e1).
Report is 1 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #1586   +/-   ##
=======================================
  Coverage   96.92%   96.92%           
=======================================
  Files         157      157           
  Lines       10569    10571    +2     
=======================================
+ Hits        10244    10246    +2     
  Misses        325      325           
Files with missing lines Coverage Δ
xknx/devices/climate.py 100.00% <100.00%> (ø)

test/devices_tests/climate_test.py Outdated Show resolved Hide resolved
xknx/devices/climate.py Outdated Show resolved Hide resolved
xknx/devices/climate.py Outdated Show resolved Hide resolved
test/devices_tests/climate_test.py Outdated Show resolved Hide resolved
docs/climate.md Outdated Show resolved Hide resolved
@farmio
Copy link
Member

farmio commented Oct 19, 2024

Hi 👋!
Thank you very much for your contribution!

Since this humidity value has purely informational character and no additional value calculations are needed, I wonder if it would be an option to just use an additional xknx.devices.Sensor class in HAs ClimateEntity implementation instead of piping it through xknx.devices.Climate. A multi-xknx-device entity could be used in some other places in the HA integration too. Just a thought, let me know what you think...

- requested changes
@alexkn
Copy link
Contributor Author

alexkn commented Oct 20, 2024

Thank you for your quick feedback.

The humidity is part Home Assistant Climate entity.
Also the sensor is build into the same hardware.
So I think it is consistent to add it to the xknx climate device.

@alexkn alexkn requested a review from farmio October 20, 2024 08:52
- requested changes
Copy link
Member

@farmio farmio left a 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 👍

@farmio
Copy link
Member

farmio commented Oct 20, 2024

Are you interested in adding climate to UI configurable entities in HA too?

@alexkn
Copy link
Contributor Author

alexkn commented Oct 20, 2024

Are you interested in adding climate to UI configurable entities in HA too?

Maybe. But with my limited Python/HomeAssistant code knowledge, I guess I don't have enough time for that at the moment.

@farmio farmio merged commit 90dbacb into XKNX:main Oct 20, 2024
11 checks passed
@farmio
Copy link
Member

farmio commented Oct 20, 2024

Alright. If you have any questions, feel free to ping me on Discord.

@alexkn alexkn deleted the climate-humidity branch October 20, 2024 09:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants