[energenie] oh1 migration #6461

hmerk · 2019-11-27T13:52:01Z

Signed-off-by: Hans-Jörg Merk hans-joerg.merk@t-online.de

TravisBuddy · 2019-11-27T13:56:01Z

Travis tests were successful

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

...enie/src/main/java/org/openhab/binding/energenie/internal/config/EnergenieConfiguration.java

...energenie/src/main/java/org/openhab/binding/energenie/internal/handler/EnergenieHandler.java

...rgenie/src/main/java/org/openhab/binding/energenie/internal/handler/EnergeniePWMHandler.java

...les/org.openhab.binding.energenie/src/main/resources/ESH-INF/i18n/energenie_xx_XX.properties

bundles/org.openhab.binding.energenie/src/main/resources/ESH-INF/thing/thing-types.xml

...rgenie/src/main/java/org/openhab/binding/energenie/internal/handler/EnergeniePWMHandler.java

Hilbrand

I've done a quick review. Didnt' catch everything. But this should you get started 😄

hmerk · 2019-11-27T19:50:15Z

Thanks @Hilbrand for you first review comments. Will make the changes tomorrow.
According to default password, „1“ is indeed the factory default.

TravisBuddy · 2019-11-28T07:57:31Z

Travis tests were successful

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

TravisBuddy · 2019-11-28T13:01:52Z

Travis tests were successful

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

TravisBuddy · 2019-11-28T13:02:34Z

Travis tests were successful

Hey @hmerk,
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

Ok here are some more comments. This covers most of the issues. Only comments left after that are related to the use of the Socket. If you're ok with it I can make the changes related to usage of Socket after you've updated from to my review.

Sorry to give you a hard time, while all you are doing is copying most of the original OH1 code 😉 This is something I expect will happen with other older bindings.

....energenie/src/main/java/org/openhab/binding/energenie/internal/EnergenieHandlerFactory.java

...enie/src/main/java/org/openhab/binding/energenie/internal/config/EnergenieConfiguration.java

bundles/org.openhab.binding.energenie/src/main/resources/ESH-INF/thing/thing-types.xml

bundles/org.openhab.binding.energenie/README.md

...rgenie/src/main/java/org/openhab/binding/energenie/internal/handler/EnergeniePWMHandler.java

...energenie/src/main/java/org/openhab/binding/energenie/internal/handler/EnergenieHandler.java

bundles/org.openhab.binding.energenie/src/main/resources/ESH-INF/thing/thing-types.xml

hmerk · 2019-11-30T12:32:29Z

Thanks @Hilbrand for your second review. I have included most of your change requests.
According to the socket usage, your suggestion would be fine for me.
But I still don't understand how to add the identical channels to all devices without repeating them, cause that's what you asked for.
And don't worry gving me a hard time. Even better, I learn a lot from your valuable comments 👍

TravisBuddy · 2019-11-30T12:35:06Z

Travis tests were successful

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

TravisBuddy · 2019-11-30T14:18:58Z

Travis tests were successful

Hey @hmerk,
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 · 2019-12-18T13:09:30Z

@hmerk I've updated the code to move part of the code into it's own class. Apparently I have to much power 😄 I checkout out your branch and added a commit to this and then pushed. Which ended up directly to this pull request. Might not be a problem, but I expected it to want to create a branch in my own repository. So can you review my added commit and if possible test to see if I didn't broke anything.

TravisBuddy · 2019-12-18T13:10:35Z

Travis tests were successful

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

hmerk · 2019-12-18T19:29:15Z

@Hilbrand thanks for your suggested changes, much appreciated.
I will try to test the new version intensively before Christmas and provide feedback.

hmerk · 2019-12-19T07:20:59Z

@Hilbrand I started testing the new oH3.0 Snapshot Version by copying the jar from jfrog into addons folder of a fresh oH2.5 Release VM.
Result: Configured Things won't get ONLINE:

I will check the code changes later today to see if I can find the issue....

hmerk · 2019-12-19T07:26:45Z

@Hilbrand I enabled trace log and it seems that your code changes broke the solution calculation :-(

`
2019-12-19 07:24:27.506 [TRACE] [nie.internal.handler.EnergenieSocket] - EG responded with task (int) '[111, -1, -4, 124]' (hex) '6FFFFC7C'

2019-12-19 07:24:27.506 [TRACE] [nie.internal.handler.EnergenieSocket] - Solution '[0, 0, 0, 0]' send to EG

2019-12-19 07:24:27.517 [TRACE] [nie.internal.handler.EnergenieSocket] - Start Condition '[17]' send to EG

2019-12-19 07:24:27.519 [TRACE] [nie.internal.handler.EnergenieSocket] - EG responded with task (int) '[52, -86, 82, 65]' (hex) '34AA5241'

2019-12-19 07:24:27.520 [TRACE] [nie.internal.handler.EnergenieSocket] - Solution '[0, 0, 0, 0]' send to EG

2019-12-19 07:24:27.525 [TRACE] [nie.internal.handler.EnergenieSocket] - Start Condition '[17]' send to EG

2019-12-19 07:24:27.527 [TRACE] [nie.internal.handler.EnergenieSocket] - EG responded with task (int) '[-67, -81, 107, -54]' (hex) 'BDAF6BCA'

2019-12-19 07:24:27.528 [TRACE] [nie.internal.handler.EnergenieSocket] - Solution '[0, 0, 0, 0]' send to EG

2019-12-19 07:24:29.009 [DEBUG] [ie.internal.handler.EnergenieHandler] - Couldn't get I/O for the connection to: 192.168.0.130:5000.

2019-12-19 07:24:29.021 [DEBUG] [ie.internal.handler.EnergenieHandler] - Couldn't get I/O for the connection to: 192.168.0.132:5000.

2019-12-19 07:24:29.031 [DEBUG] [ie.internal.handler.EnergenieHandler] - Couldn't get I/O for the connection to: 192.168.0.130:5000.

2019-12-19 07:24:30.569 [DEBUG] [ie.internal.handler.EnergenieHandler] - Couldn't get I/O for the connection to: 192.168.0.131:5000.

2019-12-19 07:25:29.013 [TRACE] [nie.internal.handler.EnergenieSocket] - Start Condition '[17]' send to EG

2019-12-19 07:25:29.015 [TRACE] [nie.internal.handler.EnergenieSocket] - EG responded with task (int) '[87, 110, 40, 100]' (hex) '576E2864'

2019-12-19 07:25:29.015 [TRACE] [nie.internal.handler.EnergenieSocket] - Solution '[0, 0, 0, 0]' send to EG

2019-12-19 07:25:29.024 [TRACE] [nie.internal.handler.EnergenieSocket] - Start Condition '[17]' send to EG

2019-12-19 07:25:29.026 [TRACE] [nie.internal.handler.EnergenieSocket] - EG responded with task (int) '[-95, -102, 5, -82]' (hex) 'A19A05AE'

2019-12-19 07:25:29.027 [TRACE] [nie.internal.handler.EnergenieSocket] - Solution '[0, 0, 0, 0]' send to EG

2019-12-19 07:25:29.035 [TRACE] [nie.internal.handler.EnergenieSocket] - Start Condition '[17]' send to EG

2019-12-19 07:25:29.037 [TRACE] [nie.internal.handler.EnergenieSocket] - EG responded with task (int) '[-92, 24, 123, -79]' (hex) 'A4187BB1'

2019-12-19 07:25:29.038 [TRACE] [nie.internal.handler.EnergenieSocket] - Solution '[0, 0, 0, 0]' send to EG

2019-12-19 07:25:30.518 [DEBUG] [ie.internal.handler.EnergenieHandler] - Couldn't get I/O for the connection to: 192.168.0.130:5000.

2019-12-19 07:25:30.529 [DEBUG] [ie.internal.handler.EnergenieHandler] - Couldn't get I/O for the connection to: 192.168.0.132:5000.

2019-12-19 07:25:30.540 [DEBUG] [ie.internal.handler.EnergenieHandler] - Couldn't get I/O for the connection to: 192.168.0.130:5000.

2019-12-19 07:25:33.641 [DEBUG] [ie.internal.handler.EnergenieHandler] - Couldn't get I/O for the connection to: 192.168.0.131:5000.
`

hmerk · 2019-12-19T10:04:13Z

@Hilbrand After a first quick check, the handler never uses socket.authorize() since your PR, therefore no task is retrieved and could be handled. According to the Data Exchange Protocol documentation, a task is just valid for 4 seconds. If no solution is received in this time, the device falls back to start condition. Therefore polling the device or handling a command always needs to authorize first.
I will try to solve this by providing a new commit.

Hilbrand · 2019-12-19T15:37:21Z

Ah I see. Some leftover from my refactoring. The task should be passed to the calculateSolution method.

TravisBuddy · 2019-12-19T15:40:52Z

Travis tests were successful

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

Signed-off-by: Hans-Jörg Merk <hans-joerg.merk@t-online.de>

TravisBuddy · 2020-05-06T06:34:04Z

Travis tests were successful

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

...ng.energenie/src/main/java/org/openhab/binding/energenie/internal/EnergenieProtocolEnum.java

Signed-off-by: Hans-Jörg Merk <hans-joerg.merk@t-online.de>

TravisBuddy · 2020-05-06T06:45:46Z

Travis tests were successful

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

cpmeister

LGTM. Thanks for taking the time to migrate this!

hmerk · 2020-05-06T06:52:51Z

@cpmeister Thanks for your patience and your valuable review comments.

@J-N-K @Hilbrand Could we get your approval as well as all comments have been adressed and the Binding is working in my environment. Travis and Jenkins are fine as well.

hmerk · 2020-05-06T07:14:23Z

@armitagemderivitec

Very much looking forward to this integration. Is there any news?

Should be merged soon and will be available in SNAPSHOT and 2.5.5 Release.

m17kea · 2020-05-06T11:40:10Z

@hmerk great thanks!

J-N-K

@hmerk, good job. Only some minor comments.

bundles/org.openhab.binding.energenie/NOTICE

bundles/org.openhab.binding.energenie/README.md

...rgenie/src/main/java/org/openhab/binding/energenie/internal/handler/EnergeniePWMHandler.java

bundles/org.openhab.binding.energenie/README.md

Signed-off-by: Hans-Jörg Merk <hans-joerg.merk@t-online.de>

hmerk · 2020-05-07T06:14:31Z

@J-N-K Thanks, but I have to give back the cudos to you, @Hilbrand and @cpmeister .
Without all your outstanding support, this would not have been possible.

TravisBuddy · 2020-05-07T06:16:24Z

Travis tests were successful

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

Signed-off-by: Hans-Jörg Merk <hans-joerg.merk@t-online.de>

TravisBuddy · 2020-05-07T06:24:19Z

Travis tests were successful

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

* [energenie] oh1 migration Signed-off-by: Hans-Jörg Merk <hans-joerg.merk@t-online.de> Co-authored-by: Hilbrand Bouwkamp <hilbrand@h72.nl>

* [energenie] oh1 migration Signed-off-by: Hans-Jörg Merk <hans-joerg.merk@t-online.de> Co-authored-by: Hilbrand Bouwkamp <hilbrand@h72.nl> Signed-off-by: CSchlipp <christian@schlipp.de>

* [energenie] oh1 migration Signed-off-by: Hans-Jörg Merk <hans-joerg.merk@t-online.de> Co-authored-by: Hilbrand Bouwkamp <hilbrand@h72.nl>

* [energenie] oh1 migration Signed-off-by: Hans-Jörg Merk <hans-joerg.merk@t-online.de> Co-authored-by: Hilbrand Bouwkamp <hilbrand@h72.nl> Signed-off-by: Daan Meijer <daan@studioseptember.nl>

* [energenie] oh1 migration Signed-off-by: Hans-Jörg Merk <hans-joerg.merk@t-online.de> Co-authored-by: Hilbrand Bouwkamp <hilbrand@h72.nl>

hmerk 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 Nov 27, 2019

hmerk requested a review from a team as a code owner November 27, 2019 13:52

This was referenced Nov 27, 2019

openHAB 1 addon compatibility overview with openHAB 3 #6179

Closed

[energenie] Energenie Mi|Home Binding initial contribution #3890

Closed

Hilbrand reviewed Nov 27, 2019

View reviewed changes

Hilbrand requested changes Nov 27, 2019

View reviewed changes

hmerk added rebuild Triggers Jenkins PR build and removed rebuild Triggers Jenkins PR build labels Nov 29, 2019

Hilbrand requested changes Nov 29, 2019

View reviewed changes

hmerk requested review from 3cky, 9037568, alexf2015 and andrewfg as code owners December 31, 2019 11:40

Move protocol to own Enum class

e9dd91e

Signed-off-by: Hans-Jörg Merk <hans-joerg.merk@t-online.de>

cpmeister reviewed May 6, 2020

View reviewed changes

...ng.energenie/src/main/java/org/openhab/binding/energenie/internal/EnergenieProtocolEnum.java Outdated Show resolved Hide resolved

Add newline at end of EnergenieProtocolEnum

27b8227

Signed-off-by: Hans-Jörg Merk <hans-joerg.merk@t-online.de>

cpmeister approved these changes May 6, 2020

View reviewed changes

Hilbrand requested a review from J-N-K May 6, 2020 09:50

Hilbrand approved these changes May 6, 2020

View reviewed changes

J-N-K requested changes May 6, 2020

View reviewed changes

Small fixes after review

4029768

Signed-off-by: Hans-Jörg Merk <hans-joerg.merk@t-online.de>

Correction to protocol version for PM2LAN device

1942dd4

Signed-off-by: Hans-Jörg Merk <hans-joerg.merk@t-online.de>

J-N-K approved these changes May 7, 2020

View reviewed changes

J-N-K merged commit c8b7bbe into openhab:2.5.x May 7, 2020

J-N-K added this to the 2.5.5 milestone May 7, 2020

hmerk deleted the energenie_migration branch May 7, 2020 18:06

J-N-K pushed a commit to J-N-K/openhab-addons that referenced this pull request Jul 14, 2020

[energenie] Initial contribution (openhab#6461)

399b608

* [energenie] oh1 migration Signed-off-by: Hans-Jörg Merk <hans-joerg.merk@t-online.de> Co-authored-by: Hilbrand Bouwkamp <hilbrand@h72.nl>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[energenie] oh1 migration #6461

[energenie] oh1 migration #6461

hmerk commented Nov 27, 2019

TravisBuddy commented Nov 27, 2019

Hilbrand left a comment

hmerk commented Nov 27, 2019

TravisBuddy commented Nov 28, 2019

TravisBuddy commented Nov 28, 2019

TravisBuddy commented Nov 28, 2019

Hilbrand left a comment

hmerk commented Nov 30, 2019 •

edited

Loading

TravisBuddy commented Nov 30, 2019

TravisBuddy commented Nov 30, 2019

Hilbrand commented Dec 18, 2019

TravisBuddy commented Dec 18, 2019

hmerk commented Dec 18, 2019

hmerk commented Dec 19, 2019 •

edited

Loading

hmerk commented Dec 19, 2019 •

edited

Loading

hmerk commented Dec 19, 2019 •

edited

Loading

Hilbrand commented Dec 19, 2019

TravisBuddy commented Dec 19, 2019

TravisBuddy commented May 6, 2020

TravisBuddy commented May 6, 2020

cpmeister left a comment

hmerk commented May 6, 2020 •

edited

Loading

hmerk commented May 6, 2020 •

edited

Loading

m17kea commented May 6, 2020

J-N-K left a comment

hmerk commented May 7, 2020

TravisBuddy commented May 7, 2020

TravisBuddy commented May 7, 2020

[energenie] oh1 migration #6461

[energenie] oh1 migration #6461

Conversation

hmerk commented Nov 27, 2019

TravisBuddy commented Nov 27, 2019

Travis tests were successful

Hilbrand left a comment

Choose a reason for hiding this comment

hmerk commented Nov 27, 2019

TravisBuddy commented Nov 28, 2019

Travis tests were successful

TravisBuddy commented Nov 28, 2019

Travis tests were successful

TravisBuddy commented Nov 28, 2019

Travis tests were successful

Hilbrand left a comment

Choose a reason for hiding this comment

hmerk commented Nov 30, 2019 • edited Loading

TravisBuddy commented Nov 30, 2019

Travis tests were successful

TravisBuddy commented Nov 30, 2019

Travis tests were successful

Hilbrand commented Dec 18, 2019

TravisBuddy commented Dec 18, 2019

Travis tests were successful

hmerk commented Dec 18, 2019

hmerk commented Dec 19, 2019 • edited Loading

hmerk commented Dec 19, 2019 • edited Loading

hmerk commented Dec 19, 2019 • edited Loading

Hilbrand commented Dec 19, 2019

TravisBuddy commented Dec 19, 2019

Travis tests were successful

TravisBuddy commented May 6, 2020

Travis tests were successful

TravisBuddy commented May 6, 2020

Travis tests were successful

cpmeister left a comment

Choose a reason for hiding this comment

hmerk commented May 6, 2020 • edited Loading

hmerk commented May 6, 2020 • edited Loading

m17kea commented May 6, 2020

J-N-K left a comment

Choose a reason for hiding this comment

hmerk commented May 7, 2020

TravisBuddy commented May 7, 2020

Travis tests were successful

TravisBuddy commented May 7, 2020

Travis tests were successful

hmerk commented Nov 30, 2019 •

edited

Loading

hmerk commented Dec 19, 2019 •

edited

Loading

hmerk commented Dec 19, 2019 •

edited

Loading

hmerk commented Dec 19, 2019 •

edited

Loading

hmerk commented May 6, 2020 •

edited

Loading

hmerk commented May 6, 2020 •

edited

Loading