search

golles / ha-kamstrup_403

Custom component that integrates the Kamstrup 403 heating system into Home Assistant. This component does also support a few other heating systems
MIT License
70 stars 10 forks source link

Support reading of multiple values at once #56

Closed golles closed 1 year ago

golles commented 1 year ago

@adabrandt for your code suggestion in https://github.com/bsdphk/PyKamstrup/pull/6

codecov[bot] commented 1 year ago

Codecov Report

Base: 16.89% // Head: 73.86% // Increases project coverage by +56.96% :tada:

Coverage data is based on head (bfbe837) compared to base (cbc618f). Patch coverage: 52.41% of modified lines in pull request are covered.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #56 +/- ## =========================================== + Coverage 16.89% 73.86% +56.96% =========================================== Files 6 7 +1 Lines 290 329 +39 =========================================== + Hits 49 243 +194 + Misses 241 86 -155 ``` | [Impacted Files](https://codecov.io/gh/golles/ha-kamstrup_403/pull/56?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Sander) | Coverage Δ | | |---|---|---| | [custom\_components/kamstrup\_403/\_\_init\_\_.py](https://codecov.io/gh/golles/ha-kamstrup_403/pull/56?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Sander#diff-Y3VzdG9tX2NvbXBvbmVudHMva2Ftc3RydXBfNDAzL19faW5pdF9fLnB5) | `85.00% <50.00%> (+55.73%)` | :arrow_up: | | [...tom\_components/kamstrup\_403/pykamstrup/kamstrup.py](https://codecov.io/gh/golles/ha-kamstrup_403/pull/56?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Sander#diff-Y3VzdG9tX2NvbXBvbmVudHMva2Ftc3RydXBfNDAzL3B5a2Ftc3RydXAva2Ftc3RydXAucHk=) | `51.16% <51.16%> (ø)` | | | [custom\_components/kamstrup\_403/config\_flow.py](https://codecov.io/gh/golles/ha-kamstrup_403/pull/56?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Sander#diff-Y3VzdG9tX2NvbXBvbmVudHMva2Ftc3RydXBfNDAzL2NvbmZpZ19mbG93LnB5) | `95.74% <100.00%> (+95.74%)` | :arrow_up: | | [custom\_components/kamstrup\_403/pykamstrup/const.py](https://codecov.io/gh/golles/ha-kamstrup_403/pull/56?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Sander#diff-Y3VzdG9tX2NvbXBvbmVudHMva2Ftc3RydXBfNDAzL3B5a2Ftc3RydXAvY29uc3QucHk=) | `100.00% <100.00%> (ø)` | | | [custom\_components/kamstrup\_403/sensor.py](https://codecov.io/gh/golles/ha-kamstrup_403/pull/56?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Sander#diff-Y3VzdG9tX2NvbXBvbmVudHMva2Ftc3RydXBfNDAzL3NlbnNvci5weQ==) | `94.11% <0.00%> (+94.11%)` | :arrow_up: | Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Sander). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Sander)

:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

StevenLooman commented 1 year ago

Great work! You can consider moving kampstrup.py to its own package and publish it to PyPI even. This way others can use it even more easily. It also keeps the home assistant parts (this repo) and the kampstrup interfacing separate.

Also, parentheses are only needed when catching multiple exception types. E.g., you can omit the parentheses on this line: https://github.com/golles/ha-kamstrup_403/pull/56/files#diff-b42daff407aa09858d366cd76d1042a12e82de32c92ea02115b6e6dd8393bae1R141

golles commented 1 year ago

Great work! You can consider moving kampstrup.py to its own package and publish it to PyPI even. This way others can use it even more easily. It also keeps the home assistant parts (this repo) and the kampstrup interfacing separate.

I have thought about it, and I'm not entirely sure if I want to take public ownership of it, also because it's not really my code. But with the change I made in this PR, I agree it's pretty standalone. I'll give it another thought...

Also, parentheses are only needed when catching multiple exception types. E.g., you can omit the parentheses on this line: https://github.com/golles/ha-kamstrup_403/pull/56/files#diff-b42daff407aa09858d366cd76d1042a12e82de32c92ea02115b6e6dd8393bae1R141

Thanks, I found a few more as well, fixed in 8192f0d96c3025b5ff06153889a73482ccf64bdd

StevenLooman commented 1 year ago

I have thought about it, and I'm not entirely sure if I want to take public ownership of it, also because it's not really my code. But with the change I made in this PR, I agree it's pretty standalone. I'll give it another thought...

Perhaps asking @bsdphk directly helps, maybe he can give his opinion?