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

[gce] GCE Binding initial contribution #5457

Merged
merged 17 commits into from
Sep 15, 2020
Merged

[gce] GCE Binding initial contribution #5457

merged 17 commits into from
Sep 15, 2020

Conversation

clinique
Copy link
Contributor

This is a rewrite of the IPX binding for OH1. Renamed to GCE, the producer of the equipments because we may implement more devices in the future.
Seems to be working fine right now. Documentation as to be done.

@clinique clinique requested a review from a team as a code owner April 14, 2019 17:27
@wborn
Copy link
Member

wborn commented Apr 14, 2019

Initially I thought this PR was for a Google Compute Engine Binding. But apparently it's for something completely else. I've probably been using the GCE too much. 😉

@wborn wborn added new binding If someone has started to work on a binding. For a new binding PR. work in progress A PR that is not yet ready to be merged labels Apr 14, 2019
@wborn wborn changed the title [WIP] GCE Binding [gce][WIP] GCE Binding initial contribution May 25, 2019
@clinique clinique mentioned this pull request Oct 22, 2019
Closed
@Hilbrand Hilbrand added the oh1 migration Relates to migrating an openHAB 1 addon to openHAB 2 label Oct 22, 2019
@Hilbrand
Copy link
Member

@clinique is this still work in progress?

@clinique
Copy link
Contributor Author

@Hilbrand : yes, it is still WIP, the binding is fine and operational, I'm currently working on solving a new issue.
This leaves documentation to be adressed.

@openhab openhab deleted a comment from TravisBuddy Sep 11, 2020
@openhab openhab deleted a comment from TravisBuddy Sep 11, 2020
@clinique
Spotless corrections
3b40994 
Signed-off-by: clinique <gael@lhopital.org>
@clinique
Test correction
954a22c 
Signed-off-by: clinique <gael@lhopital.org>
@clinique
Correcting tests
c3f7a2e 
Signed-off-by: clinique <gael@lhopital.org>
@clinique
REmoving pointless tests
21386d8 
Signed-off-by: clinique <gael@lhopital.org>
@openhab openhab deleted a comment from TravisBuddy Sep 11, 2020
fwolter
fwolter requested changes Sep 11, 2020
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.

Did you put the model package at the same level as the internal package intentionally?

...g.openhab.binding.gce/src/main/java/org/openhab/binding/gce/model/StatusFileInterpreter.java Outdated Show resolved Hide resolved
bundles/org.openhab.binding.gce/README.md Outdated Show resolved Hide resolved
bundles/org.openhab.binding.gce/README.md Outdated Show resolved Hide resolved
bundles/org.openhab.binding.gce/README.md Outdated Show resolved Hide resolved
@clinique clinique changed the title [gce][WIP] GCE Binding initial contribution [gce] GCE Binding initial contribution Sep 12, 2020
@clinique
Copy link
Contributor Author

Did you put the model package at the same level as the internal package intentionally?

No, you're right, this is a mistake.

@clinique
Code review corrections
d8fa6ca 
Signed-off-by: clinique <gael@lhopital.org>
@openhab openhab deleted a comment from TravisBuddy Sep 12, 2020
fwolter
fwolter approved these changes Sep 12, 2020
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

@clinique
Forgot bom/openhab-addons/pom.xml
672267a 
Signed-off-by: clinique <gael@lhopital.org>
@TravisBuddy
Copy link

Travis tests were successful

Hey @clinique,
we found no major flaws with your code. Still you might want to look at this logfile, as we usually suggest some optional improvements.

Hilbrand
Hilbrand requested changes Sep 13, 2020
View reviewed changes
Copy link
Member

@Hilbrand Hilbrand left a comment

Choose a reason for hiding this comment

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

Just some minor comments and we could merge this binding.

bundles/org.openhab.binding.gce/README.md Outdated Show resolved Hide resolved
bundles/org.openhab.binding.gce/src/main/resources/ESH-INF/thing/ipx800v3.xml Outdated Show resolved Hide resolved
...nhab.binding.gce/src/main/java/org/openhab/binding/gce/internal/handler/Ipx800v3Handler.java Outdated Show resolved Hide resolved
....binding.gce/src/main/java/org/openhab/binding/gce/internal/model/StatusFileInterpreter.java Outdated Show resolved Hide resolved
@clinique
Code review corrections
4394d2f 
Signed-off-by: clinique <gael@lhopital.org>
Hilbrand
Hilbrand approved these changes Sep 15, 2020
View reviewed changes
Copy link
Member

@Hilbrand Hilbrand left a comment

Choose a reason for hiding this comment

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

Thanks. LGTM

@Hilbrand Hilbrand removed the work in progress A PR that is not yet ready to be merged label Sep 15, 2020
@Hilbrand Hilbrand merged commit 1229ca3 into openhab:2.5.x Sep 15, 2020
@Hilbrand Hilbrand added this to the 2.5.9 milestone Sep 15, 2020
@clinique clinique deleted the IPX800 branch September 15, 2020 11:28
markus7017 pushed a commit to markus7017/openhab-addons that referenced this pull request Sep 19, 2020
@clinique @markus7017
[gce] GCE Binding initial contribution (openhab#5457)
149aa3b 
Signed-off-by:  Gaël L'hopital <gael@lhopital.org>
andrewfg pushed a commit to andrewfg/openhab-addons that referenced this pull request Oct 8, 2020
@clinique @andrewfg
[gce] GCE Binding initial contribution (openhab#5457)
3b9bdbf 
Signed-off-by:  Gaël L'hopital <gael@lhopital.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Hilbrand Hilbrand Hilbrand approved these changes

@fwolter fwolter fwolter approved these changes

Assignees
No one assigned
Labels
cre Coordinated Review Effort 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
2.5.9
Development

Successfully merging this pull request may close these issues.
5 participants
@clinique @wborn @Hilbrand @TravisBuddy @fwolter