-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
[tradfri] Add support for Air Purifier #14836
[tradfri] Add support for Air Purifier #14836
Conversation
Added Support for Tradfri Air Purifier: * Added documentation disambiguation Tradfri vs Dirigera * Added Tradfri Air Purifier - fanMode and fanSpeed * Workable Tradfri Air Purifier basic implementation * Tradfri: modified fanMode type and definition * Tradfri Air Purifier: Added disableLed * Tradfri Air Purifier: Added lockPhysicalButton * Tradfri Air Purifier: Added airQuality data * Tradfri Air Purifier: Added filterCheck data * Tradfri Air Purifier: Added translations * Tradfri Air Purifier: Added filter_uptime * Tradfri Air Purifier: code optimization * Documentation for supported Air Purifier channels Fixes openhab#14816 Signed-off-by: Vivien Boistuaud <[email protected]>
This pull request has been mentioned on openHAB Community. There might be relevant details there: https://community.openhab.org/t/tradfri-binding-support-for-air-purifiers/146119/1 |
* [tradfri] Updated README: * Reverts unnecessary formatting changes * Fixes centering of capabilities for non-lightning devices list * Removes 0202, 0203 and position from lightning devices list Signed-off-by: Vivien Boistuaud <[email protected]>
Made some updates to the README to fix some formatting and content-splitting issues (8f17e0c) |
* Fixes Alphabetical order of properties * Fixes a typo in some names Signed-off-by: Vivien Boistuaud <[email protected]>
Proactive fixes in 7f4accf:
Note: Additions of the ^M endings in tradfri.properties is coming from execution 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 your contribution! The code looks good, only a few minor comments.
bundles/org.openhab.binding.tradfri/src/main/resources/OH-INF/i18n/tradfri_de.properties
Outdated
Show resolved
Hide resolved
bundles/org.openhab.binding.tradfri/src/main/resources/OH-INF/i18n/tradfri_fi.properties
Outdated
Show resolved
Hide resolved
bundles/org.openhab.binding.tradfri/src/main/resources/OH-INF/i18n/tradfri_fr.properties
Outdated
Show resolved
Hide resolved
bundles/org.openhab.binding.tradfri/src/main/resources/OH-INF/i18n/tradfri_it.properties
Outdated
Show resolved
Hide resolved
...ding.tradfri/src/main/java/org/openhab/binding/tradfri/internal/TradfriBindingConstants.java
Outdated
Show resolved
Hide resolved
...ding.tradfri/src/main/java/org/openhab/binding/tradfri/internal/TradfriBindingConstants.java
Outdated
Show resolved
Hide resolved
bundles/org.openhab.binding.tradfri/src/main/resources/OH-INF/thing/thing-types.xml
Show resolved
Hide resolved
bundles/org.openhab.binding.tradfri/src/main/resources/OH-INF/thing/thing-types.xml
Show resolved
Hide resolved
bundles/org.openhab.binding.tradfri/src/main/resources/OH-INF/thing/thing-types.xml
Show resolved
Hide resolved
* README.md alignment update * reverting translation changes to comply with automation rules via Crowdin * refactor things definition * Simplify the Air Purifier thing name * Add categories for Time and Fan * Simplified binding constants * reverted tradfri.properties due to formatting issues * added default text in tradfri.properties Signed-off-by: Vivien Boistuaud <[email protected]>
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 a lot @jlaur for your code review, really appreciated.
I fixed everything according to your recommendation, built and tested it to confirm it is still totally functional. New test packages for 3.4.4 and 4.0.0 have been updated in the description.
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.
Before merging, perhaps you could have a look at these small findings in the README?
Updates to the documentation to: * Fix Item Types * Fix abreviations (min -> minutes) * Standardize `description` column of the `Channel Type ID` table * Align the `Channel Type ID` table * Improve the `Channels` > `Air Purifier` paragraph * Align the non-ZLL capabilities table to similar format as ZLL One (uppercase first letter of each word; Align capabilities labels to `Channel Type ID` labels) Signed-off-by: Vivien Boistuaud <[email protected]>
Hi @jlaur, I reviewed the additions to the documentation and standardization of the tables to:
Waiting for the build to complete and will ask for re-review. |
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.
Reviewed all changes and submitted new commit with further enhancements after rewriting the Air Purifier documentation.
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.
The documentation update looks good. 👍 Only some nitty gritty last details.
* Fixed for language consistency purposes Signed-off-by: Vivien Boistuaud <[email protected]>
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.
LGTM. 👍 @kaikreuzer, @cweitkamp - you are both code owners for this binding, would you like to review?
@kaikreuzer, @cweitkamp - gentle ping. 🙂 |
@kaikreuzer, @cweitkamp - I really just need to know if you'd like to have a look before merging - or not? 🙂 |
Thanks for the regular pings @jlaur, I somehow missed them. I'd say you can feel free to merge if you do not hear anything back after a ping within 2 weeks. :-) |
(and of course many thanks to @vivienbo for the contribution!) |
* [tradfri] Added Support for Air Purifier (openhab#7) Added Support for Tradfri Air Purifier: * Added documentation disambiguation Tradfri vs Dirigera * Added Tradfri Air Purifier - fanMode and fanSpeed * Workable Tradfri Air Purifier basic implementation * Tradfri: modified fanMode type and definition * Tradfri Air Purifier: Added disableLed * Tradfri Air Purifier: Added lockPhysicalButton * Tradfri Air Purifier: Added airQuality data * Tradfri Air Purifier: Added filterCheck data * Tradfri Air Purifier: Added translations * Tradfri Air Purifier: Added filter_uptime * Tradfri Air Purifier: code optimization * Documentation for supported Air Purifier channels Fixes openhab#14816 Signed-off-by: Vivien Boistuaud <[email protected]> Signed-off-by: Thomas Burri <[email protected]>
* [tradfri] Added Support for Air Purifier (openhab#7) Added Support for Tradfri Air Purifier: * Added documentation disambiguation Tradfri vs Dirigera * Added Tradfri Air Purifier - fanMode and fanSpeed * Workable Tradfri Air Purifier basic implementation * Tradfri: modified fanMode type and definition * Tradfri Air Purifier: Added disableLed * Tradfri Air Purifier: Added lockPhysicalButton * Tradfri Air Purifier: Added airQuality data * Tradfri Air Purifier: Added filterCheck data * Tradfri Air Purifier: Added translations * Tradfri Air Purifier: Added filter_uptime * Tradfri Air Purifier: code optimization * Documentation for supported Air Purifier channels Fixes openhab#14816 Signed-off-by: Vivien Boistuaud <[email protected]> Signed-off-by: Matt Myers <[email protected]>
* [tradfri] Added Support for Air Purifier (openhab#7) Added Support for Tradfri Air Purifier: * Added documentation disambiguation Tradfri vs Dirigera * Added Tradfri Air Purifier - fanMode and fanSpeed * Workable Tradfri Air Purifier basic implementation * Tradfri: modified fanMode type and definition * Tradfri Air Purifier: Added disableLed * Tradfri Air Purifier: Added lockPhysicalButton * Tradfri Air Purifier: Added airQuality data * Tradfri Air Purifier: Added filterCheck data * Tradfri Air Purifier: Added translations * Tradfri Air Purifier: Added filter_uptime * Tradfri Air Purifier: code optimization * Documentation for supported Air Purifier channels Fixes openhab#14816 Signed-off-by: Vivien Boistuaud <[email protected]> Signed-off-by: Jørgen Austvik <[email protected]>
Description
This PR adds support for Tradfri Air Purifier (Strakvind) into the tradfri binding.
It supports:
Note: you will still need to run the filter review process through the Tradfri Mobile App.
Testing
Tested on OpenHAB 4.0.0-M1 and on 3.4.4.
Changelog
General matter:
Tradfri Air Purifier features:
Related issues: