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.

Sign up for GitHub

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

Jump to bottom

[anel] Initial contribution of the Anel NET-PwrCtrl binding for OH3 #10952

Merged
merged 4 commits into from
Nov 29, 2021
Merged

[anel] Initial contribution of the Anel NET-PwrCtrl binding for OH3 #10952

merged 4 commits into from
Nov 29, 2021

Conversation

paphko
Copy link
Contributor

@paphko paphko commented Jul 6, 2021

I re-wrote the binding for Anel NET-PwrCtrl devices which only exists for OH1 so far.

It is already tested by some users, see: https://community.openhab.org/t/anel-net-pwrctrl-binding-for-oh3/123378

There are also some unit tests for the core functionality.

The readme explains the usage and gives examples.

@paphko paphko requested a review from a team as a code owner July 6, 2021 15:56
@paphko paphko changed the title [anel] Initial contribution of the Anel NET-PwrCtrl binding for OH3 [WIP][anel] Initial contribution of the Anel NET-PwrCtrl binding for OH3 Jul 6, 2021
@paphko paphko force-pushed the anel branch 3 times, most recently from e541598 to ea62d11 Compare July 6, 2021 21:43
@paphko paphko changed the title [WIP][anel] Initial contribution of the Anel NET-PwrCtrl binding for OH3 [anel] Initial contribution of the Anel NET-PwrCtrl binding for OH3 Jul 7, 2021
@openhab-bot

This comment has been minimized.

Sign in to view
@openhab-bot
Copy link
Collaborator

This pull request has been mentioned on openHAB Community. There might be relevant details there:

https://community.openhab.org/t/anel-net-pwrctrl-binding-for-oh3/123378/1

@fwolter fwolter added new binding If someone has started to work on a binding. For a new binding PR. oh1 migration Relates to migrating an openHAB 1 addon to openHAB 2 labels Jul 7, 2021
@fwolter fwolter mentioned this pull request Jul 7, 2021
Closed
@paphko
Copy link
Contributor Author

paphko commented Jul 12, 2021

Do I have to do anything else now?
(the Jenkins build is instable due to test failures in other bundles)

@hmerk hmerk added the rebuild Triggers Jenkins PR build label Aug 1, 2021
@fwolter fwolter removed the rebuild Triggers Jenkins PR build label Aug 1, 2021
@paphko
Copy link
Contributor Author

paphko commented Sep 7, 2021

@fwolter could you maybe tell me what needs to be done so that this PR is reviewed and merged?

@fwolter
Copy link
Member

fwolter commented Sep 7, 2021

You simply need to wait. There are some other new binding PRs in the queue before yours is at the tip.

fwolter
fwolter requested changes Sep 12, 2021
View reviewed changes
Copy link
Member

@fwolter fwolter left a 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! I reviewed your code and here is my feedback.

There are some checkstyle warnings left. You could take a look at target/code-analysis/report.html after building it with mvn clean install.

bundles/org.openhab.binding.anel/README.md Show resolved Hide resolved
....openhab.binding.anel/src/main/java/org/openhab/binding/anel/internal/AnelConfiguration.java Outdated Show resolved Hide resolved
....openhab.binding.anel/src/main/java/org/openhab/binding/anel/internal/AnelConfiguration.java Outdated Show resolved Hide resolved
...es/org.openhab.binding.anel/src/main/java/org/openhab/binding/anel/internal/AnelHandler.java Outdated Show resolved Hide resolved
...es/org.openhab.binding.anel/src/main/java/org/openhab/binding/anel/internal/AnelHandler.java Outdated Show resolved Hide resolved
bundles/org.openhab.binding.anel/src/main/resources/OH-INF/thing/thing-types.xml Outdated Show resolved Hide resolved
bundles/org.openhab.binding.anel/src/main/resources/OH-INF/thing/thing-types.xml Outdated Show resolved Hide resolved
...enhab.binding.anel/src/test/java/org/openhab/binding/anel/internal/AnelUdpConnectorTest.java Show resolved Hide resolved
...enhab.binding.anel/src/test/java/org/openhab/binding/anel/internal/AnelUdpConnectorTest.java Show resolved Hide resolved
...hab.binding.anel/src/main/java/org/openhab/binding/anel/internal/state/AnelStateUpdater.java Outdated Show resolved Hide resolved
@paphko
Copy link
Contributor Author

paphko commented Nov 9, 2021

Thank you @fwolter for the code review!
I hope that I fixed all your remarks to your satisfaction.
However, there is one remaining warning from the Code Analysis Tool which I don't know how to solve:

[WARNING] org.openhab.binding.anel.internal.state.AnelStateUpdater.java:[30]
The package tech.units.indriya.unit.Units should not be used.

@paphko
Adjustments based on code review.
078f166 
Signed-off-by: Patrick Koenemann <[email protected]>
@paphko paphko requested a review from fwolter November 10, 2021 07:20
@fwolter
Copy link
Member

fwolter commented Nov 20, 2021

You can use SIUnits.CELSIUS with import org.openhab.core.library.unit.SIUnits;.

@paphko
Copy link
Contributor Author

paphko commented Nov 25, 2021

@fwolter FYI, I fixed your remaining remarks 😃

fwolter
fwolter requested changes Nov 28, 2021
View reviewed changes
Copy link
Member

@fwolter fwolter left a comment

Choose a reason for hiding this comment

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

There are some checkstyle warnings left. You could take a look at target/code-analysis/report.html after building it with mvn clean install.

@paphko
Checkstyle warnings revmoed.
2029974 
Signed-off-by: Patrick Koenemann <[email protected]>
fwolter
fwolter approved these changes Nov 29, 2021
View reviewed changes
Copy link
Member

@fwolter fwolter left a comment

Choose a reason for hiding this comment

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

LGTM

Now, you could add your binding's logo to the openHAB website. See https://next.openhab.org/docs/developer/bindings/#add-your-binding-s-logo-to-the-openhab-website

@fwolter fwolter merged commit 0adacaf into openhab:main Nov 29, 2021
@fwolter fwolter added this to the 3.2 milestone Nov 29, 2021
NickWaterton pushed a commit to NickWaterton/openhab-addons that referenced this pull request Dec 30, 2021
@paphko @NickWaterton
[anel] Initial contribution of the Anel NET-PwrCtrl binding for OH3 (o…
bff3312 
…penhab#10952)

* Initial contribution of the Anel NET-PwrCtrl binding for OH3.

Signed-off-by: Patrick Koenemann <[email protected]>

* Adjustments based on code review.

Signed-off-by: Patrick Koenemann <[email protected]>

* Further adjustments according to second review.

Signed-off-by: Patrick Koenemann <[email protected]>

* Checkstyle warnings revmoed.

Signed-off-by: Patrick Koenemann <[email protected]>
Signed-off-by: Nick Waterton <[email protected]>
mischmidt83 pushed a commit to mischmidt83/openhab-addons that referenced this pull request Jan 9, 2022
@paphko @mischmidt83
[anel] Initial contribution of the Anel NET-PwrCtrl binding for OH3 (o…
b544e1c 
…penhab#10952)

* Initial contribution of the Anel NET-PwrCtrl binding for OH3.

Signed-off-by: Patrick Koenemann <[email protected]>

* Adjustments based on code review.

Signed-off-by: Patrick Koenemann <[email protected]>

* Further adjustments according to second review.

Signed-off-by: Patrick Koenemann <[email protected]>

* Checkstyle warnings revmoed.

Signed-off-by: Patrick Koenemann <[email protected]>
Signed-off-by: Michael Schmidt <[email protected]>
nemerdaud pushed a commit to nemerdaud/openhab-addons that referenced this pull request Jan 28, 2022
@paphko
[anel] Initial contribution of the Anel NET-PwrCtrl binding for OH3 (o…
1aae8a4 
…penhab#10952)

* Initial contribution of the Anel NET-PwrCtrl binding for OH3.

Signed-off-by: Patrick Koenemann <[email protected]>

* Adjustments based on code review.

Signed-off-by: Patrick Koenemann <[email protected]>

* Further adjustments according to second review.

Signed-off-by: Patrick Koenemann <[email protected]>

* Checkstyle warnings revmoed.

Signed-off-by: Patrick Koenemann <[email protected]>
marcfischerboschio pushed a commit to bosch-io/openhab-addons that referenced this pull request May 5, 2022
@paphko
[anel] Initial contribution of the Anel NET-PwrCtrl binding for OH3 (o…
a82cdfe 
…penhab#10952)

* Initial contribution of the Anel NET-PwrCtrl binding for OH3.

Signed-off-by: Patrick Koenemann <[email protected]>

* Adjustments based on code review.

Signed-off-by: Patrick Koenemann <[email protected]>

* Further adjustments according to second review.

Signed-off-by: Patrick Koenemann <[email protected]>

* Checkstyle warnings revmoed.

Signed-off-by: Patrick Koenemann <[email protected]>
andan67 pushed a commit to andan67/openhab-addons that referenced this pull request Nov 6, 2022
@paphko @andan67
[anel] Initial contribution of the Anel NET-PwrCtrl binding for OH3 (o…
bb5d58a 
…penhab#10952)

* Initial contribution of the Anel NET-PwrCtrl binding for OH3.

Signed-off-by: Patrick Koenemann <[email protected]>

* Adjustments based on code review.

Signed-off-by: Patrick Koenemann <[email protected]>

* Further adjustments according to second review.

Signed-off-by: Patrick Koenemann <[email protected]>

* Checkstyle warnings revmoed.

Signed-off-by: Patrick Koenemann <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fwolter fwolter fwolter approved these changes

Assignees
No one assigned
Labels
new binding If someone has started to work on a binding. For a new binding PR. oh1 migration Relates to migrating an openHAB 1 addon to openHAB 2
Projects
None yet
Milestone
3.2
Development

Successfully merging this pull request may close these issues.
4 participants
@paphko @openhab-bot @fwolter @hmerk