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

[danfossairunit] Add filter period channel #11371

Merged

Conversation

jlaur
Copy link
Contributor

@jlaur jlaur commented Oct 11, 2021

Fixes #11310

Signed-off-by: Jacob Laursen [email protected]

Add new channel filter_life for number of months between filter replacements.

@jlaur jlaur requested a review from pravussum as a code owner October 11, 2021 18:05
@jlaur jlaur marked this pull request as draft October 11, 2021 18:05
@jlaur jlaur marked this pull request as ready for review October 11, 2021 19:04
@jlaur jlaur marked this pull request as draft October 11, 2021 20:04
Copy link
Contributor

@pravussum pravussum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Besides the issue with retrieving the max filter period the changes look good to me. But under the circumstances (I think it might even be a bug in the CCM), using fixed values seems reasonable - at least they are the correct ones then.
Thanks also for giving the documentation samples a polish.

@jlaur
Copy link
Contributor Author

jlaur commented Oct 11, 2021

@pravussum - I left some more thoughts in the linked issue. Right now I'm thinking having the channel provides most flexibility, and it would be possible to write a rule with a RemainingFilterLife proxy item that could be updated only when receiving recalculated value having the desired filter period set.

@jlaur jlaur marked this pull request as ready for review October 11, 2021 20:46
@Skinah Skinah added the enhancement An enhancement or new feature for an existing add-on label Oct 13, 2021
@jlaur
Copy link
Contributor Author

jlaur commented Oct 14, 2021

@Skinah - I saw you added label two days ago, would it be possible for you to schedule a rebuild or add the rebuild tag as last build failed for reasons unrelated to changes in this PR?

lolodomo
lolodomo previously approved these changes Oct 17, 2021
Copy link
Contributor

@lolodomo lolodomo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@jlaur jlaur force-pushed the 11310-danfossairunit-filter-period-channel branch from 3fb6458 to d05a198 Compare October 17, 2021 21:12
@lolodomo lolodomo dismissed their stale review October 18, 2021 11:30

Changes after approval

@lolodomo lolodomo added rebuild Triggers Jenkins PR build and removed rebuild Triggers Jenkins PR build labels Oct 19, 2021
Copy link
Contributor

@lolodomo lolodomo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Skinah
Copy link
Contributor

Skinah commented Oct 20, 2021

All the new PR's are failing at the moment so it is probably a build system issue, when new PR's pass again it will be worth triggering a rebuild then otherwise it will just chew up the servers time if we expect it to fail not due to your code.

@lolodomo
Copy link
Contributor

lolodomo commented Oct 20, 2021

I tried after maintainers were informed the build problem could have been solved.

@kaikreuzer kaikreuzer added rebuild Triggers Jenkins PR build and removed rebuild Triggers Jenkins PR build labels Oct 20, 2021
@lolodomo lolodomo added the rebuild Triggers Jenkins PR build label Oct 21, 2021
@lolodomo lolodomo removed the rebuild Triggers Jenkins PR build label Oct 21, 2021
@jlaur
Copy link
Contributor Author

jlaur commented Oct 22, 2021

@lolodomo
Copy link
Contributor

@jlaur : I see you have discussion about the documentation in the issue. Is there still something to change in the documentation or is it finished and ready for a merge ?

@jlaur
Copy link
Contributor Author

jlaur commented Oct 22, 2021

@jlaur : I see you have discussion about the documentation in the issue. Is there still something to change in the documentation or is it finished and ready for a merge ?

No further documentation plans from my side, as only one channel is exposed and there's already a warning about value being overwritten. If @pravussum would have any ideas for further documentation improvements, I can prepare another PR.

@lolodomo lolodomo merged commit d4c9d6e into openhab:main Oct 22, 2021
@lolodomo
Copy link
Contributor

Thank you

@lolodomo lolodomo added this to the 3.2 milestone Oct 22, 2021
dschoepel pushed a commit to dschoepel/openhab-addons that referenced this pull request Nov 9, 2021
* Add filter period channel.

Fixes openhab#11310

Signed-off-by: Jacob Laursen <[email protected]>

* Round remaining filter life percentage to one decimal.

Signed-off-by: Jacob Laursen <[email protected]>

* Improve and extend example configuration.

Signed-off-by: Jacob Laursen <[email protected]>

* Mark filter period channel as advanced due to Link CC/Air Dial conflicts.

Signed-off-by: Jacob Laursen <[email protected]>

* Add comment about value getting overwritten.

Signed-off-by: Jacob Laursen <[email protected]>

* Remove redundant parentheses.

Signed-off-by: Jacob Laursen <[email protected]>

* Fix SCA report: First javadoc author should have 'Initial contribution' contribution description.

Signed-off-by: Jacob Laursen <[email protected]>

* Fix SCA issue: NoEmptyLineSeparatorCheck

Signed-off-by: Jacob Laursen <[email protected]>
Signed-off-by: Dave J Schoepel <[email protected]>
NickWaterton pushed a commit to NickWaterton/openhab-addons that referenced this pull request Dec 30, 2021
* Add filter period channel.

Fixes openhab#11310

Signed-off-by: Jacob Laursen <[email protected]>

* Round remaining filter life percentage to one decimal.

Signed-off-by: Jacob Laursen <[email protected]>

* Improve and extend example configuration.

Signed-off-by: Jacob Laursen <[email protected]>

* Mark filter period channel as advanced due to Link CC/Air Dial conflicts.

Signed-off-by: Jacob Laursen <[email protected]>

* Add comment about value getting overwritten.

Signed-off-by: Jacob Laursen <[email protected]>

* Remove redundant parentheses.

Signed-off-by: Jacob Laursen <[email protected]>

* Fix SCA report: First javadoc author should have 'Initial contribution' contribution description.

Signed-off-by: Jacob Laursen <[email protected]>

* Fix SCA issue: NoEmptyLineSeparatorCheck

Signed-off-by: Jacob Laursen <[email protected]>
Signed-off-by: Nick Waterton <[email protected]>
nemerdaud pushed a commit to nemerdaud/openhab-addons that referenced this pull request Jan 28, 2022
* Add filter period channel.

Fixes openhab#11310

Signed-off-by: Jacob Laursen <[email protected]>

* Round remaining filter life percentage to one decimal.

Signed-off-by: Jacob Laursen <[email protected]>

* Improve and extend example configuration.

Signed-off-by: Jacob Laursen <[email protected]>

* Mark filter period channel as advanced due to Link CC/Air Dial conflicts.

Signed-off-by: Jacob Laursen <[email protected]>

* Add comment about value getting overwritten.

Signed-off-by: Jacob Laursen <[email protected]>

* Remove redundant parentheses.

Signed-off-by: Jacob Laursen <[email protected]>

* Fix SCA report: First javadoc author should have 'Initial contribution' contribution description.

Signed-off-by: Jacob Laursen <[email protected]>

* Fix SCA issue: NoEmptyLineSeparatorCheck

Signed-off-by: Jacob Laursen <[email protected]>
marcfischerboschio pushed a commit to bosch-io/openhab-addons that referenced this pull request May 5, 2022
* Add filter period channel.

Fixes openhab#11310

Signed-off-by: Jacob Laursen <[email protected]>

* Round remaining filter life percentage to one decimal.

Signed-off-by: Jacob Laursen <[email protected]>

* Improve and extend example configuration.

Signed-off-by: Jacob Laursen <[email protected]>

* Mark filter period channel as advanced due to Link CC/Air Dial conflicts.

Signed-off-by: Jacob Laursen <[email protected]>

* Add comment about value getting overwritten.

Signed-off-by: Jacob Laursen <[email protected]>

* Remove redundant parentheses.

Signed-off-by: Jacob Laursen <[email protected]>

* Fix SCA report: First javadoc author should have 'Initial contribution' contribution description.

Signed-off-by: Jacob Laursen <[email protected]>

* Fix SCA issue: NoEmptyLineSeparatorCheck

Signed-off-by: Jacob Laursen <[email protected]>
@jlaur jlaur deleted the 11310-danfossairunit-filter-period-channel branch June 15, 2024 20:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement An enhancement or new feature for an existing add-on
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[danfossairunit] Filter lifetime management
5 participants