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

[mecmeter] Initial contribution of mecmeter binding #9075

Merged
merged 4 commits into from
Mar 28, 2021
Merged

[mecmeter] Initial contribution of mecmeter binding #9075

merged 4 commits into from
Mar 28, 2021

Conversation

kaikreuzer
Copy link
Member

This is a new binding for the din-rail power meter from MEC, see https://www.mec.at/produkte/#mecMeter.

Signed-off-by: Kai Kreuzer kai@openhab.org

@cpmeister cpmeister added the new binding If someone has started to work on a binding. For a new binding PR. label Nov 21, 2020
@wborn wborn added rebuild Triggers Jenkins PR build and removed rebuild Triggers Jenkins PR build labels Jan 22, 2021
@Skinah Skinah added the work in progress A PR that is not yet ready to be merged label Feb 23, 2021
@kaikreuzer kaikreuzer removed the work in progress A PR that is not yet ready to be merged label Mar 13, 2021
@kaikreuzer kaikreuzer changed the title [WIP][mecmeter] Initial contribution of mecmeter binding [mecmeter] Initial contribution of mecmeter binding Mar 13, 2021
@kaikreuzer kaikreuzer marked this pull request as ready for review March 13, 2021 11:54
@kaikreuzer kaikreuzer requested a review from a team as a code owner March 13, 2021 11:54
@kaikreuzer
Copy link
Member Author

@openhab/add-ons-maintainers This binding is finally ready for review. Should be a fairly simple one, not that much code altogether.

wborn
wborn requested changes Mar 24, 2021
View reviewed changes
Copy link
Member

@wborn wborn left a comment

Choose a reason for hiding this comment

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

Thank you for migrating the code to OH3. 👍 I placed a few review comments for my findings below.

...binding.mecmeter/src/main/java/org/openhab/binding/mecmeter/MecMeterDeviceConfiguration.java Show resolved Hide resolved
...binding.mecmeter/src/main/java/org/openhab/binding/mecmeter/MecMeterDeviceConfiguration.java Show resolved Hide resolved
...binding.mecmeter/src/main/java/org/openhab/binding/mecmeter/MecMeterDeviceConfiguration.java Outdated Show resolved Hide resolved
...binding.mecmeter/src/main/java/org/openhab/binding/mecmeter/MecMeterDeviceConfiguration.java Outdated Show resolved Hide resolved
bundles/org.openhab.binding.mecmeter/src/main/resources/OH-INF/thing/thing-types.xml Show resolved Hide resolved
...hab.binding.mecmeter/src/main/java/org/openhab/binding/mecmeter/handler/MecMeterHandler.java Outdated Show resolved Hide resolved
...hab.binding.mecmeter/src/main/java/org/openhab/binding/mecmeter/handler/MecMeterHandler.java Outdated Show resolved Hide resolved
...hab.binding.mecmeter/src/main/java/org/openhab/binding/mecmeter/handler/MecMeterHandler.java Outdated Show resolved Hide resolved
bundles/org.openhab.binding.mecmeter/src/main/resources/OH-INF/thing/thing-types.xml Outdated Show resolved Hide resolved
bundles/pom.xml Show resolved Hide resolved
@kaikreuzer
Initial contribution of mecmeter binding
84ff064 
Also-by: Klaus Berger <klaus.berger@verbund.com>
Also-by: Florian Pazour <florian.pazour@verbund.com>
Signed-off-by: Kai Kreuzer <kai@openhab.org>
@kaikreuzer
Copy link
Member Author

@wborn Thanks for your review! All comments are addressed now.

@kaikreuzer kaikreuzer requested a review from wborn March 24, 2021 18:48
wborn
wborn requested changes Mar 24, 2021
View reviewed changes
Copy link
Member

@wborn wborn 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 quickly fixing all my review comments!

After going through the code I have 2 more comments:

bundles/org.openhab.binding.mecmeter/README.MD Outdated Show resolved Hide resolved
...hab.binding.mecmeter/src/main/java/org/openhab/binding/mecmeter/handler/MecMeterHandler.java Show resolved Hide resolved
@kaikreuzer
addressed review feedback
bdf7e9a 
Signed-off-by: Kai Kreuzer <kai@openhab.org>
@kaikreuzer kaikreuzer requested a review from wborn March 24, 2021 22:30
wborn
wborn approved these changes Mar 24, 2021
View reviewed changes
Copy link
Member

@wborn wborn 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! If another @openhab/add-ons-maintainers also approves the PR then we can merge this. 🚀

@wborn
Copy link
Member

wborn commented Mar 24, 2021

While we wait for the second review, can you also add the binding dir to the CODEOWNERS file @kaikreuzer?

@kaikreuzer
added codeowners entry
717fa51 
Signed-off-by: Kai Kreuzer <kai@openhab.org>
@kaikreuzer
Copy link
Member Author

can you also add the binding dir to the CODEOWNERS file

Sure, done!

@kaikreuzer kaikreuzer requested a review from fwolter March 25, 2021 12:06
fwolter
fwolter requested changes Mar 27, 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 two compiler warnings left:

[WARNING] MecMeterHandler.java:[101,13] Potential null pointer access: this expression has a '@Nullable' type
[WARNING] MecMeterHandler.java:[117,13] Potential null pointer access: this expression has a '@Nullable' type

...hab.binding.mecmeter/src/main/java/org/openhab/binding/mecmeter/handler/MecMeterHandler.java Show resolved Hide resolved
...hab.binding.mecmeter/src/main/java/org/openhab/binding/mecmeter/handler/MecMeterHandler.java Outdated Show resolved Hide resolved
bundles/org.openhab.binding.mecmeter/src/main/resources/OH-INF/thing/thing-types.xml Outdated Show resolved Hide resolved
@kaikreuzer
addressed review comments
4582144 
Signed-off-by: Kai Kreuzer <kai@openhab.org>
@kaikreuzer kaikreuzer requested a review from fwolter March 27, 2021 18:38
fwolter
fwolter approved these changes Mar 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.

LGTM

@fwolter fwolter merged commit 1d8b0df into openhab:main Mar 28, 2021
@fwolter fwolter added this to the 3.1 milestone Mar 28, 2021
themillhousegroup pushed a commit to themillhousegroup/openhab2-addons that referenced this pull request May 10, 2021
@kaikreuzer @themillhousegroup
[mecmeter] Initial contribution of mecmeter binding (openhab#9075)
4b9c047 
* Initial contribution of mecmeter binding

Also-by: Klaus Berger <klaus.berger@verbund.com>
Also-by: Florian Pazour <florian.pazour@verbund.com>
Signed-off-by: Kai Kreuzer <kai@openhab.org>

* addressed review feedback

Signed-off-by: Kai Kreuzer <kai@openhab.org>

* added codeowners entry

Signed-off-by: Kai Kreuzer <kai@openhab.org>

* addressed review comments

Signed-off-by: Kai Kreuzer <kai@openhab.org>
Signed-off-by: John Marshall <john.marshall.au@gmail.com>
@kaikreuzer kaikreuzer deleted the mecmeter branch May 15, 2021 18:29
computergeek1507 pushed a commit to computergeek1507/openhab-addons that referenced this pull request Jul 13, 2021
@kaikreuzer @computergeek1507
[mecmeter] Initial contribution of mecmeter binding (openhab#9075)
2e9a229 
* Initial contribution of mecmeter binding

Also-by: Klaus Berger <klaus.berger@verbund.com>
Also-by: Florian Pazour <florian.pazour@verbund.com>
Signed-off-by: Kai Kreuzer <kai@openhab.org>

* addressed review feedback

Signed-off-by: Kai Kreuzer <kai@openhab.org>

* added codeowners entry

Signed-off-by: Kai Kreuzer <kai@openhab.org>

* addressed review comments

Signed-off-by: Kai Kreuzer <kai@openhab.org>
thinkingstone pushed a commit to thinkingstone/openhab-addons that referenced this pull request Nov 7, 2021
@kaikreuzer @thinkingstone
[mecmeter] Initial contribution of mecmeter binding (openhab#9075)
a25d5c4 
* Initial contribution of mecmeter binding

Also-by: Klaus Berger <klaus.berger@verbund.com>
Also-by: Florian Pazour <florian.pazour@verbund.com>
Signed-off-by: Kai Kreuzer <kai@openhab.org>

* addressed review feedback

Signed-off-by: Kai Kreuzer <kai@openhab.org>

* added codeowners entry

Signed-off-by: Kai Kreuzer <kai@openhab.org>

* addressed review comments

Signed-off-by: Kai Kreuzer <kai@openhab.org>
marcfischerboschio pushed a commit to bosch-io/openhab-addons that referenced this pull request May 5, 2022
@kaikreuzer
[mecmeter] Initial contribution of mecmeter binding (openhab#9075)
c463e0d 
* Initial contribution of mecmeter binding

Also-by: Klaus Berger <klaus.berger@verbund.com>
Also-by: Florian Pazour <florian.pazour@verbund.com>
Signed-off-by: Kai Kreuzer <kai@openhab.org>

* addressed review feedback

Signed-off-by: Kai Kreuzer <kai@openhab.org>

* added codeowners entry

Signed-off-by: Kai Kreuzer <kai@openhab.org>

* addressed review comments

Signed-off-by: Kai Kreuzer <kai@openhab.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wborn wborn wborn approved these changes

@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.
Projects
None yet
Milestone
3.1
Development

Successfully merging this pull request may close these issues.
5 participants
@kaikreuzer @wborn @fwolter @cpmeister @Skinah