-
-
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
[comfoair] Initial contribution for comfoair v2 binding #7052
Conversation
Travis tests have failedHey @boehan, 1st BuildExpand here
### 2nd Build Expand here
|
1 similar comment
Travis tests have failedHey @boehan, 1st BuildExpand here
### 2nd Build Expand here
|
Travis tests were successfulHey @boehan, |
1 similar comment
Travis tests were successfulHey @boehan, |
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 migrating this binding over. Here are some things I found after my first pass.
...inding.comfoair/src/main/java/org/openhab/binding/comfoair/internal/ComfoAirCommandType.java
Outdated
Show resolved
Hide resolved
...inding.comfoair/src/main/java/org/openhab/binding/comfoair/internal/ComfoAirCommandType.java
Show resolved
Hide resolved
...inding.comfoair/src/main/java/org/openhab/binding/comfoair/internal/ComfoAirCommandType.java
Outdated
Show resolved
Hide resolved
...ding.comfoair/src/main/java/org/openhab/binding/comfoair/internal/ComfoAirConfiguration.java
Outdated
Show resolved
Hide resolved
...ab.binding.comfoair/src/main/java/org/openhab/binding/comfoair/internal/ComfoAirHandler.java
Outdated
Show resolved
Hide resolved
...g.comfoair/src/main/java/org/openhab/binding/comfoair/internal/datatypes/DataTypeNumber.java
Show resolved
Hide resolved
...ding.comfoair/src/main/java/org/openhab/binding/comfoair/internal/datatypes/DataTypeRPM.java
Show resolved
Hide resolved
...ding.comfoair/src/main/java/org/openhab/binding/comfoair/internal/datatypes/DataTypeRPM.java
Show resolved
Hide resolved
...foair/src/main/java/org/openhab/binding/comfoair/internal/datatypes/DataTypeTemperature.java
Show resolved
Hide resolved
...ing.comfoair/src/main/java/org/openhab/binding/comfoair/internal/datatypes/DataTypeVolt.java
Show resolved
Hide resolved
@cpmeister thx for your review (besides all the other work you seem to put into OH development these days, btw). I will try to address your comments as asap. |
2b915a1
to
5236137
Compare
Travis tests have failedHey @boehan, 2nd BuildExpand here
|
Travis tests have failedHey @boehan, 2nd BuildExpand here
|
Travis tests were successfulHey @boehan, |
...ab.binding.comfoair/src/main/java/org/openhab/binding/comfoair/internal/ComfoAirCommand.java
Outdated
Show resolved
Hide resolved
...ab.binding.comfoair/src/main/java/org/openhab/binding/comfoair/internal/ComfoAirHandler.java
Outdated
Show resolved
Hide resolved
....comfoair/src/main/java/org/openhab/binding/comfoair/internal/datatypes/DataTypeBoolean.java
Outdated
Show resolved
Hide resolved
....comfoair/src/main/java/org/openhab/binding/comfoair/internal/datatypes/DataTypeMessage.java
Outdated
Show resolved
Hide resolved
...g.comfoair/src/main/java/org/openhab/binding/comfoair/internal/datatypes/DataTypeNumber.java
Outdated
Show resolved
Hide resolved
ac3d5e1
to
9b989c4
Compare
I removed the rules from the README since they were just ported over from the v1 binding and completely untested as I'm not using them myself. |
Travis tests were successfulHey @boehan, |
1 similar comment
Travis tests were successfulHey @boehan, |
Travis tests have failedHey @boehan, 1st BuildExpand here
### 2nd Build Expand here
|
Travis tests were successfulHey @boehan, |
3 similar comments
Travis tests were successfulHey @boehan, |
Travis tests were successfulHey @boehan, |
Travis tests were successfulHey @boehan, |
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.
Since you mentioned you wanted some thorough feedback, I decided to give you thorough feedback. 😁
....comfoair/src/main/java/org/openhab/binding/comfoair/internal/datatypes/DataTypeMessage.java
Outdated
Show resolved
Hide resolved
bundles/org.openhab.binding.comfoair/src/main/resources/ESH-INF/thing/thing-types.xml
Outdated
Show resolved
Hide resolved
bundles/org.openhab.binding.comfoair/src/main/resources/ESH-INF/thing/thing-types.xml
Outdated
Show resolved
Hide resolved
bundles/org.openhab.binding.comfoair/src/main/resources/ESH-INF/thing/thing-types.xml
Outdated
Show resolved
Hide resolved
bundles/org.openhab.binding.comfoair/src/main/resources/ESH-INF/thing/thing-types.xml
Outdated
Show resolved
Hide resolved
...inding.comfoair/src/main/java/org/openhab/binding/comfoair/internal/ComfoAirCommandType.java
Outdated
Show resolved
Hide resolved
...inding.comfoair/src/main/java/org/openhab/binding/comfoair/internal/ComfoAirCommandType.java
Outdated
Show resolved
Hide resolved
...inding.comfoair/src/main/java/org/openhab/binding/comfoair/internal/ComfoAirCommandType.java
Outdated
Show resolved
Hide resolved
...inding.comfoair/src/main/java/org/openhab/binding/comfoair/internal/ComfoAirCommandType.java
Outdated
Show resolved
Hide resolved
...inding.comfoair/src/main/java/org/openhab/binding/comfoair/internal/ComfoAirCommandType.java
Outdated
Show resolved
Hide resolved
Now you start challenging my (still limited) java skills 😄. I will try to implement your suggestions, if it improves the code. Though, I will need your guidance on some of them. |
Signed-off-by: Hans Böhm <h.boehm@gmx.at>
Signed-off-by: Hans Böhm <h.boehm@gmx.at>
Signed-off-by: Hans Böhm <h.boehm@gmx.at> fix connector Signed-off-by: Hans Böhm <h.boehm@gmx.at>
Signed-off-by: Hans Böhm <h.boehm@gmx.at>
Signed-off-by: Hans Böhm <h.boehm@gmx.at>
Signed-off-by: Hans Böhm <h.boehm@gmx.at> fix channel names 2 Signed-off-by: Hans Böhm <h.boehm@gmx.at>
Signed-off-by: Hans Böhm <h.boehm@gmx.at>
Signed-off-by: Hans Böhm <h.boehm@gmx.at>
Signed-off-by: Hans Böhm <h.boehm@gmx.at>
Signed-off-by: Hans Böhm <h.boehm@gmx.at>
Signed-off-by: Hans Böhm <h.boehm@gmx.at>
Signed-off-by: Hans Böhm <h.boehm@gmx.at>
Signed-off-by: Hans Böhm <h.boehm@gmx.at>
Signed-off-by: Hans Böhm <h.boehm@gmx.at>
Signed-off-by: Hans Böhm <h.boehm@gmx.at>
Signed-off-by: Hans Böhm <h.boehm@gmx.at>
Signed-off-by: Hans Böhm <h.boehm@gmx.at>
Travis tests were successfulHey @boehan, |
Any changes this could make it into 2.5.9? I know there are a lot of other PRs to be reviewed, but since this is already partly approved it would be nice if this could make it into the distro before the 2.5 feature freeze. |
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.
Sorry for the wait. Everything looks good now.
* comfoair - first version Signed-off-by: Hans Böhm <h.boehm@gmx.at> Signed-off-by: Daan Meijer <daan@studioseptember.nl>
Cool to have this merged. |
* comfoair - first version Signed-off-by: Hans Böhm <h.boehm@gmx.at>
* comfoair - first version Signed-off-by: Hans Böhm <h.boehm@gmx.at>
* comfoair - first version Signed-off-by: Hans Böhm <h.boehm@gmx.at>
This binding is a migration of the v1 comfoair binding.
Logic is mainly re-used from v1 version with some straightforward changes/additions:
Since, this is my first bigger java project, any suggestions on code quality improvements are highly appreciated.
Also thanks to @HolgerHees and @gieemek for their valuable work on the v1 binding.
JAR-Files are available from (after initial build):
https://openhab.jfrog.io/openhab/libs-pullrequest-local/org/openhab/addons/bundles/org.openhab.binding.comfoair/
Community thread