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

[elroconnects] New binding for Elro Connects #11189

Merged
merged 17 commits into from
Mar 20, 2022
Merged

[elroconnects] New binding for Elro Connects #11189

merged 17 commits into from
Mar 20, 2022

Conversation

mherwege
Copy link
Contributor

@mherwege mherwege commented Sep 1, 2021 •
edited
Loading

Signed-off-by: Mark Herwege mark.herwege@telenet.be

This PR introduces a binding for the Elro Connects smarthome system.

The binding code is complete. I have had it running for a while without issues. I am aware of a few people using it as well, and have not received issues.

@mherwege mherwege requested a review from a team as a code owner September 1, 2021 20:28
@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/new-binding-for-elro-connects-devices-looking-for-testers/126125/1

@wborn wborn added additional testing preferred The change works for the pull request author. A test from someone else is preferred though. 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 Sep 2, 2021
@Skinah
Copy link
Contributor

Skinah commented Sep 3, 2021

Binding looks pretty good, thanks for creating it... When you ready, just remove the WIP tag and post you want someone to take a look/review.

@mherwege mherwege changed the title [elroconnects] New binding for Elro Connects [WIP] [elroconnects] New binding for Elro Connects Sep 8, 2021
@mherwege
Copy link
Contributor Author

mherwege commented Sep 8, 2021

@Skinah Many thanks for your review. I did the requested changes except for the constants, which I kept in the separate classes. Unfortunately, these constants can be different by device type.
I would still prefer more testing by other people, but I have not seen anyone picking up on it. I tested with the devices I have. I don't have all the devices the binding should support.
From my perspective, I removed the WIP label. I am OK to wait a bit before merging in the hope someone steps up. The alternative is to merge and react on feedback we may get.

@mherwege mherwege removed the work in progress A PR that is not yet ready to be merged label Sep 8, 2021
@mherwege mherwege requested a review from Skinah September 8, 2021 10:51
@mherwege
Some fixes and removed redundant null checks.
e277912 
Signed-off-by: Mark Herwege <mark.herwege@telenet.be>
@mherwege
Proper thread naming.
89e55be 
Signed-off-by: Mark Herwege <mark.herwege@telenet.be>
@mherwege
Adjust brand name capitalization. Some README adjustments.
58c47d0 
Signed-off-by: Mark Herwege <mark.herwege@telenet.be>
@mherwege
Fix format issue.
dc5030d 
Signed-off-by: Mark Herwege <mark.herwege@telenet.be>
@mherwege
Fix threadname.
000e4cc 
Signed-off-by: Mark Herwege <mark.herwege@telenet.be>
@mherwege
Update development cycle
2f9bcf1 
Signed-off-by: Mark Herwege <mark.herwege@telenet.be>
@mherwege
Review fixes
9508bf4 
Signed-off-by: Mark Herwege <mark.herwege@telenet.be>
@mherwege
Avoid communication restart when disposing
fbbe42b 
Signed-off-by: Mark Herwege <mark.herwege@telenet.be>
@mherwege
Update CODEOWNERS
fa4c731 
Signed-off-by: Mark Herwege <mark.herwege@telenet.be>
@mherwege
Code review.
518575d 
Signed-off-by: Mark Herwege <mark.herwege@telenet.be>
@mherwege
Background discovery
d4476f1 
Signed-off-by: Mark Herwege <mark.herwege@telenet.be>
@mherwege
Fix formatting
12e918e 
Signed-off-by: Mark Herwege <mark.herwege@telenet.be>
@mherwege
Create i18n properties file
35582b6 
Signed-off-by: Mark Herwege <mark.herwege@telenet.be>
@mherwege
Add IP Adress parameter
b0655c5 
Signed-off-by: Mark Herwege <mark.herwege@telenet.be>
@mherwege
Copy link
Contributor Author

mherwege commented Mar 15, 2022 •
edited
Loading

@Skinah @fwolter I believe I did all requested changes and provided feedback so far. Is there anything left on this for me?
In the mean time, I have been working on further enhancements to this binding, beyond what has been submitted for review here. In this PR, I only added minimal things requested by users to avoid creating a lot of extra review work. If you feel it would be better to include that directly in this PR, and not as a follow-up PR, let me know and I will do so. Otherwise, what is left for this PR, so I can build upon it?

@fwolter
Copy link
Member

fwolter commented Mar 19, 2022

In this PR, I only added minimal things requested by users to avoid creating a lot of extra review work.

This is exactly the right approach! After the last issue is fixed, this should be ready to merge.

@mherwege
Moved hostname resolving out of initialize
d20b7f8 
Signed-off-by: Mark Herwege <mark.herwege@telenet.be>
@mherwege
Copy link
Contributor Author

@fwolter Thank you for your review. I think the new commit resolves your finding.

@mherwege mherwege requested a review from fwolter March 20, 2022 11:39
fwolter
fwolter approved these changes Mar 20, 2022
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://www.openhab.org/docs/developer/bindings/#add-your-binding-s-logo-to-the-openhab-website

@fwolter fwolter merged commit 7c29e4d into openhab:main Mar 20, 2022
@fwolter fwolter added this to the 3.3 milestone Mar 20, 2022
@Confectrician Confectrician mentioned this pull request Mar 21, 2022
Merged
@mherwege mherwege deleted the elroconnects branch April 22, 2022 15:47
NickWaterton pushed a commit to NickWaterton/openhab-addons that referenced this pull request Apr 27, 2022
@mherwege @NickWaterton
[elroconnects] New binding for Elro Connects (openhab#11189)
6588c02 
* Adjustments after review.

Signed-off-by: Mark Herwege <mark.herwege@telenet.be>

* Fix pom.xml formatting.

Signed-off-by: Mark Herwege <mark.herwege@telenet.be>

* Some fixes and removed redundant null checks.

Signed-off-by: Mark Herwege <mark.herwege@telenet.be>

* Proper thread naming.

Signed-off-by: Mark Herwege <mark.herwege@telenet.be>

* Adjust brand name capitalization. Some README adjustments.

Signed-off-by: Mark Herwege <mark.herwege@telenet.be>

* Fix format issue.

Signed-off-by: Mark Herwege <mark.herwege@telenet.be>

* Fix threadname.

Signed-off-by: Mark Herwege <mark.herwege@telenet.be>

* Update development cycle

Signed-off-by: Mark Herwege <mark.herwege@telenet.be>

* Review fixes

Signed-off-by: Mark Herwege <mark.herwege@telenet.be>

* Avoid communication restart when disposing

Signed-off-by: Mark Herwege <mark.herwege@telenet.be>

* Update CODEOWNERS

Signed-off-by: Mark Herwege <mark.herwege@telenet.be>

* Code review.

Signed-off-by: Mark Herwege <mark.herwege@telenet.be>

* Background discovery

Signed-off-by: Mark Herwege <mark.herwege@telenet.be>

* Fix formatting

Signed-off-by: Mark Herwege <mark.herwege@telenet.be>

* Create i18n properties file

Signed-off-by: Mark Herwege <mark.herwege@telenet.be>

* Add IP Adress parameter

Signed-off-by: Mark Herwege <mark.herwege@telenet.be>

* Moved hostname resolving out of initialize

Signed-off-by: Mark Herwege <mark.herwege@telenet.be>
Signed-off-by: Nick Waterton <n.waterton@outlook.com>
andan67 pushed a commit to andan67/openhab-addons that referenced this pull request Nov 6, 2022
@mherwege @andan67
[elroconnects] New binding for Elro Connects (openhab#11189)
a3f33e0 
* Adjustments after review.

Signed-off-by: Mark Herwege <mark.herwege@telenet.be>

* Fix pom.xml formatting.

Signed-off-by: Mark Herwege <mark.herwege@telenet.be>

* Some fixes and removed redundant null checks.

Signed-off-by: Mark Herwege <mark.herwege@telenet.be>

* Proper thread naming.

Signed-off-by: Mark Herwege <mark.herwege@telenet.be>

* Adjust brand name capitalization. Some README adjustments.

Signed-off-by: Mark Herwege <mark.herwege@telenet.be>

* Fix format issue.

Signed-off-by: Mark Herwege <mark.herwege@telenet.be>

* Fix threadname.

Signed-off-by: Mark Herwege <mark.herwege@telenet.be>

* Update development cycle

Signed-off-by: Mark Herwege <mark.herwege@telenet.be>

* Review fixes

Signed-off-by: Mark Herwege <mark.herwege@telenet.be>

* Avoid communication restart when disposing

Signed-off-by: Mark Herwege <mark.herwege@telenet.be>

* Update CODEOWNERS

Signed-off-by: Mark Herwege <mark.herwege@telenet.be>

* Code review.

Signed-off-by: Mark Herwege <mark.herwege@telenet.be>

* Background discovery

Signed-off-by: Mark Herwege <mark.herwege@telenet.be>

* Fix formatting

Signed-off-by: Mark Herwege <mark.herwege@telenet.be>

* Create i18n properties file

Signed-off-by: Mark Herwege <mark.herwege@telenet.be>

* Add IP Adress parameter

Signed-off-by: Mark Herwege <mark.herwege@telenet.be>

* Moved hostname resolving out of initialize

Signed-off-by: Mark Herwege <mark.herwege@telenet.be>
andrasU pushed a commit to andrasU/openhab-addons that referenced this pull request Nov 12, 2022
@mherwege @andrasU
[elroconnects] New binding for Elro Connects (openhab#11189)
88ab97c 
* Adjustments after review.

Signed-off-by: Mark Herwege <mark.herwege@telenet.be>

* Fix pom.xml formatting.

Signed-off-by: Mark Herwege <mark.herwege@telenet.be>

* Some fixes and removed redundant null checks.

Signed-off-by: Mark Herwege <mark.herwege@telenet.be>

* Proper thread naming.

Signed-off-by: Mark Herwege <mark.herwege@telenet.be>

* Adjust brand name capitalization. Some README adjustments.

Signed-off-by: Mark Herwege <mark.herwege@telenet.be>

* Fix format issue.

Signed-off-by: Mark Herwege <mark.herwege@telenet.be>

* Fix threadname.

Signed-off-by: Mark Herwege <mark.herwege@telenet.be>

* Update development cycle

Signed-off-by: Mark Herwege <mark.herwege@telenet.be>

* Review fixes

Signed-off-by: Mark Herwege <mark.herwege@telenet.be>

* Avoid communication restart when disposing

Signed-off-by: Mark Herwege <mark.herwege@telenet.be>

* Update CODEOWNERS

Signed-off-by: Mark Herwege <mark.herwege@telenet.be>

* Code review.

Signed-off-by: Mark Herwege <mark.herwege@telenet.be>

* Background discovery

Signed-off-by: Mark Herwege <mark.herwege@telenet.be>

* Fix formatting

Signed-off-by: Mark Herwege <mark.herwege@telenet.be>

* Create i18n properties file

Signed-off-by: Mark Herwege <mark.herwege@telenet.be>

* Add IP Adress parameter

Signed-off-by: Mark Herwege <mark.herwege@telenet.be>

* Moved hostname resolving out of initialize

Signed-off-by: Mark Herwege <mark.herwege@telenet.be>
Signed-off-by: Andras Uhrin <andras.uhrin@gmail.com>
psmedley pushed a commit to psmedley/openhab-addons that referenced this pull request Feb 23, 2023
@mherwege @psmedley
[elroconnects] New binding for Elro Connects (openhab#11189)
67b37d8 
* Adjustments after review.

Signed-off-by: Mark Herwege <mark.herwege@telenet.be>

* Fix pom.xml formatting.

Signed-off-by: Mark Herwege <mark.herwege@telenet.be>

* Some fixes and removed redundant null checks.

Signed-off-by: Mark Herwege <mark.herwege@telenet.be>

* Proper thread naming.

Signed-off-by: Mark Herwege <mark.herwege@telenet.be>

* Adjust brand name capitalization. Some README adjustments.

Signed-off-by: Mark Herwege <mark.herwege@telenet.be>

* Fix format issue.

Signed-off-by: Mark Herwege <mark.herwege@telenet.be>

* Fix threadname.

Signed-off-by: Mark Herwege <mark.herwege@telenet.be>

* Update development cycle

Signed-off-by: Mark Herwege <mark.herwege@telenet.be>

* Review fixes

Signed-off-by: Mark Herwege <mark.herwege@telenet.be>

* Avoid communication restart when disposing

Signed-off-by: Mark Herwege <mark.herwege@telenet.be>

* Update CODEOWNERS

Signed-off-by: Mark Herwege <mark.herwege@telenet.be>

* Code review.

Signed-off-by: Mark Herwege <mark.herwege@telenet.be>

* Background discovery

Signed-off-by: Mark Herwege <mark.herwege@telenet.be>

* Fix formatting

Signed-off-by: Mark Herwege <mark.herwege@telenet.be>

* Create i18n properties file

Signed-off-by: Mark Herwege <mark.herwege@telenet.be>

* Add IP Adress parameter

Signed-off-by: Mark Herwege <mark.herwege@telenet.be>

* Moved hostname resolving out of initialize

Signed-off-by: Mark Herwege <mark.herwege@telenet.be>
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

@Skinah Skinah Skinah 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.3
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@mherwege @openhab-bot @Skinah @fwolter @wborn