[nikohomecontrol] Energy meters and access control #12893

mherwege · 2022-06-05T19:32:49Z

This PR adds:

NHC I energy meters
NHC II access control

To reduce code duplication with the added functionality, it also contains code refactoring to lift common logic to base classes. This makes the changes quite substantial. I am no longer able to split it in smaller pieces, as there are too many dependencies on previous PR's only just reviewed or still in review, which makes rebasing a nightmare. I appologize for that. I was able to rebase this on PR #12885, but had to squash the detailed commits to make it feasible.

This PR complements PR #11963, improving thermostats, starting from the same base PR. After review and merge of PR #12885 I will rebase both PR's.

mherwege · 2022-06-12T12:54:57Z

@lolodomo rebased as well.

lolodomo · 2022-06-15T11:13:45Z

There is no hurry but a rebase of your branch will be required to solve current conflicted files.

mherwege · 2022-06-15T12:27:11Z

@lolodomo Rebased

lolodomo

Review part 1 (resources, constants, handler factory and discovery)

.../main/java/org/openhab/binding/nikohomecontrol/internal/NikoHomeControlBindingConstants.java

...rg.openhab.binding.nikohomecontrol/src/main/resources/OH-INF/i18n/nikohomecontrol.properties

bundles/org.openhab.binding.nikohomecontrol/src/main/resources/OH-INF/thing/thing-types.xml

lolodomo · 2022-08-08T09:12:50Z

This is a breaking change due to changes in energyMeter thing type.

lolodomo · 2022-08-08T09:42:23Z

Remains to review: README, handler package, protocol package, NikoHomeControlCommunication1 class and NikoHomeControlCommunication2 class.

mherwege · 2022-08-16T09:46:15Z

@lolodomo Thank you for your review so far. I believe I have responded to your requests.

lolodomo · 2022-11-06T09:08:54Z

@lolodomo Thank you for your review so far. I believe I have responded to your requests.

This is stil in my TODO list to finish the review (with the 3.4 final release as deadline for that).

lolodomo · 2022-12-11T19:00:49Z

Sorry @mherwege , I finally did not yet fully review your PR.

PS: there is now a conflict that will need to be fixed.

mherwege · 2022-12-11T21:45:25Z

I think I resolved the conflict. That required a force push.

mherwege · 2024-05-05T17:34:24Z

@lolodomo I am sorry about insisting. Is there anything I need to do to make this progress? I would like to get this done before this PR’s second anniversary.

lolodomo

Few minor comments for the protocol package, mainly javadoc, except my comment about method visibility in interfaces.

...trol/src/main/java/org/openhab/binding/nikohomecontrol/internal/protocol/NhcAccessEvent.java

...ontrol/src/main/java/org/openhab/binding/nikohomecontrol/internal/protocol/NhcBaseEvent.java

...omecontrol/src/main/java/org/openhab/binding/nikohomecontrol/internal/protocol/NhcVideo.java

...omecontrol/src/main/java/org/openhab/binding/nikohomecontrol/internal/protocol/NhcMeter.java

...mecontrol/src/main/java/org/openhab/binding/nikohomecontrol/internal/protocol/NhcAccess.java

...java/org/openhab/binding/nikohomecontrol/internal/protocol/NikoHomeControlCommunication.java

...in/java/org/openhab/binding/nikohomecontrol/internal/handler/NikoHomeControlBaseHandler.java

.../java/org/openhab/binding/nikohomecontrol/internal/handler/NikoHomeControlActionHandler.java

...in/java/org/openhab/binding/nikohomecontrol/internal/handler/NikoHomeControlBaseHandler.java

lolodomo · 2024-05-19T11:29:23Z

Remains to review: README and 4 classes: NikoHomeControlAccessHandler, NikoHomeControlMeterHandler, NikoHomeControlCommunication1 and NikoHomeControlCommunication2

lolodomo

End of my review of the handler package

.../java/org/openhab/binding/nikohomecontrol/internal/handler/NikoHomeControlAccessHandler.java

...n/java/org/openhab/binding/nikohomecontrol/internal/handler/NikoHomeControlMeterHandler.java

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

lolodomo · 2024-05-19T20:04:14Z

I will try to finish tomorrow. README and 2 last classes. There is no open comments.

lolodomo

Review of class NikoHomeControlCommunication2

...rg/openhab/binding/nikohomecontrol/internal/protocol/nhc2/NikoHomeControlCommunication2.java

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

lolodomo

Review of NikoHomeControlCommunication1 : just one ultra minor comment

...rg/openhab/binding/nikohomecontrol/internal/protocol/nhc1/NikoHomeControlCommunication1.java

lolodomo · 2024-05-21T20:13:15Z

Just remain to have a look to README changes.

lolodomo

Review of README and end of review

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

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

mherwege · 2024-05-22T22:03:07Z

@lolodomo Many thanks for your thorough review. I believe I have addressed your comments. I will try to do a final smoke test on Friday, if all still works (I am not home at the moment). If you don't have other remarks and that test is OK, I believe this should be good to go.

lolodomo

LGTM, thank you

mherwege · 2024-05-24T15:40:13Z

@lolodomo I did a quick smoke test in my installation and it looks good for me. So this would be ready as far as I am concerned.

lolodomo · 2024-05-24T15:43:51Z

Thank you for your patience @mherwege

* NHCI energy meters and NHCII access control Signed-off-by: Mark Herwege <mark.herwege@telenet.be> Signed-off-by: LeeC77 <lee.charlton00@gmail.com>

* NHCI energy meters and NHCII access control Signed-off-by: Mark Herwege <mark.herwege@telenet.be> Signed-off-by: Paul Smedley <paul@smedley.id.au>

* NHCI energy meters and NHCII access control Signed-off-by: Mark Herwege <mark.herwege@telenet.be> Signed-off-by: Patrik Gfeller <patrik.gfeller@proton.me>

* NHCI energy meters and NHCII access control Signed-off-by: Mark Herwege <mark.herwege@telenet.be>

* NHCI energy meters and NHCII access control Signed-off-by: Mark Herwege <mark.herwege@telenet.be> Signed-off-by: Ciprian Pascu <contact@ciprianpascu.ro>

mherwege added the awaiting other PR Depends on another PR label Jun 5, 2022

mherwege changed the title ~~[nikohomecontrol] Energy access~~ [nikohomecontrol] Energy meters and access control Jun 5, 2022

mherwege added the enhancement An enhancement or new feature for an existing add-on label Jun 6, 2022

mherwege force-pushed the energy_access branch from 61499c5 to 22f3a14 Compare June 12, 2022 12:54

mherwege removed the awaiting other PR Depends on another PR label Jun 12, 2022

mherwege force-pushed the energy_access branch from 5a6bb6a to 01e674d Compare June 15, 2022 12:26

lolodomo requested changes Aug 8, 2022

View reviewed changes

lolodomo added the (potentially) not backward compatible label Aug 8, 2022

mherwege force-pushed the energy_access branch from 26eeb20 to 4e0c64c Compare August 23, 2022 11:43

mherwege force-pushed the energy_access branch from 4e0c64c to f7015c2 Compare December 11, 2022 21:44

mherwege force-pushed the energy_access branch from 447e413 to 0ea4927 Compare January 18, 2023 18:04

mherwege requested review from hmerk, cweitkamp, kaikreuzer, svilenvul, marcelrv, wborn, davidgraeff, ssalonen, BjoernLange and gerrieg as code owners January 18, 2023 18:04

lolodomo requested changes May 19, 2024

View reviewed changes

mherwege added 4 commits May 19, 2024 20:42

adjust to reviewer feedback

a28f472

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

fix

faa9587

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

OnOffType from

1857837

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

fix DateTime type

6d74ff1

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

lolodomo requested changes May 20, 2024

View reviewed changes

reviewer feedback

18b1f0c

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

lolodomo requested changes May 21, 2024

View reviewed changes

...rg/openhab/binding/nikohomecontrol/internal/protocol/nhc1/NikoHomeControlCommunication1.java Outdated Show resolved Hide resolved

lolodomo requested changes May 22, 2024

View reviewed changes

review adjustments

4a412a7

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

lolodomo approved these changes May 22, 2024

View reviewed changes

lolodomo merged commit a3fad33 into openhab:main May 24, 2024
5 checks passed

lolodomo added this to the 4.2 milestone May 24, 2024

mherwege deleted the energy_access branch May 26, 2024 10:17

mherwege mentioned this pull request Jun 1, 2024

[nikohomecontrol] Alarm systems #16834

Merged

joni1993 pushed a commit to joni1993/openhab-addons that referenced this pull request Oct 15, 2024

[nikohomecontrol] Energy meters and access control (openhab#12893)

df5376b

* NHCI energy meters and NHCII access control Signed-off-by: Mark Herwege <mark.herwege@telenet.be>

matchews pushed a commit to matchews/openhab-addons that referenced this pull request Oct 18, 2024

[nikohomecontrol] Energy meters and access control (openhab#12893)

93c4bde

* NHCI energy meters and NHCII access control Signed-off-by: Mark Herwege <mark.herwege@telenet.be>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[nikohomecontrol] Energy meters and access control #12893

[nikohomecontrol] Energy meters and access control #12893

mherwege commented Jun 5, 2022 •

edited

Loading

mherwege commented Jun 12, 2022

lolodomo commented Jun 15, 2022 •

edited

Loading

mherwege commented Jun 15, 2022 •

edited

Loading

lolodomo left a comment

lolodomo commented Aug 8, 2022

lolodomo commented Aug 8, 2022 •

edited

Loading

mherwege commented Aug 16, 2022

lolodomo commented Nov 6, 2022

lolodomo commented Dec 11, 2022 •

edited

Loading

mherwege commented Dec 11, 2022

mherwege commented May 5, 2024

lolodomo left a comment •

edited

Loading

lolodomo commented May 19, 2024

lolodomo left a comment

lolodomo commented May 19, 2024

lolodomo left a comment

lolodomo left a comment

lolodomo commented May 21, 2024

lolodomo left a comment

mherwege commented May 22, 2024

lolodomo left a comment

mherwege commented May 24, 2024

lolodomo commented May 24, 2024

[nikohomecontrol] Energy meters and access control #12893

[nikohomecontrol] Energy meters and access control #12893

Conversation

mherwege commented Jun 5, 2022 • edited Loading

mherwege commented Jun 12, 2022

lolodomo commented Jun 15, 2022 • edited Loading

mherwege commented Jun 15, 2022 • edited Loading

lolodomo left a comment

Choose a reason for hiding this comment

lolodomo commented Aug 8, 2022

lolodomo commented Aug 8, 2022 • edited Loading

mherwege commented Aug 16, 2022

lolodomo commented Nov 6, 2022

lolodomo commented Dec 11, 2022 • edited Loading

mherwege commented Dec 11, 2022

mherwege commented May 5, 2024

lolodomo left a comment • edited Loading

Choose a reason for hiding this comment

lolodomo commented May 19, 2024

lolodomo left a comment

Choose a reason for hiding this comment

lolodomo commented May 19, 2024

lolodomo left a comment

Choose a reason for hiding this comment

lolodomo left a comment

Choose a reason for hiding this comment

lolodomo commented May 21, 2024

lolodomo left a comment

Choose a reason for hiding this comment

mherwege commented May 22, 2024

lolodomo left a comment

Choose a reason for hiding this comment

mherwege commented May 24, 2024

lolodomo commented May 24, 2024

mherwege commented Jun 5, 2022 •

edited

Loading

lolodomo commented Jun 15, 2022 •

edited

Loading

mherwege commented Jun 15, 2022 •

edited

Loading

lolodomo commented Aug 8, 2022 •

edited

Loading

lolodomo commented Dec 11, 2022 •

edited

Loading

lolodomo left a comment •

edited

Loading