[insteon] Rewrite with backward compatibility #17146

jsetton · 2024-07-25T02:26:39Z

This is a rewrite of the Insteon binding adding some much needed improvements while keeping some of its core code refactored. Apart from simplifying the user experience by retrieving all the configuration directly from the device when possible, and improving the way the Insteon things are configured in MainUI, here is an exhaustive list of the changes:

introduced device configuration automated determination
converted mode-based number items to string type with descriptive states
added number items uom support
added distinct scenes and x10 device things
added distinct bridge things for supported hub/plm devices
added button event trigger channels
added device products configuration layer
added device link database support
added device cache storage
added device operating flags controls
added modem configuration flags controls
added related devices synchronization feature
added heartbeat timeout monitor
added ability to link/unlink a device to the modem
added ability to add missing default links
added link database & scene management support
added scene state support
added new i3 devices basic support
added ezrain sprinkler device support
revamped console commands with auto-completion support
improved discovery service
improved thing status

Since the configuration process was revamp, this would require existing users of the binding to reconfigure their Insteon setup.
However, backward compatibility is provided as some of the legacy code was kept with some adaptation to interface with updated core components. Existing device legacy things connected to a network legacy bridge will be migrated to the legacy-device thing type while still keeping the same ids to prevent any breakage.

lsiepel · 2024-07-26T09:45:06Z

I thought you would wait for me to comment on the other PR, but youre going fast. I'll try to look at this and the other PR (might close that one) next week.

jsetton · 2024-07-26T17:00:32Z

As I mentioned in the other PR, I wanted to make sure that the binding would be maintainable by having the legacy classes use the updated folder structure with minimal redundancy. Anyway, I think the way I structured this PR might be easier to review.

openhab-bot · 2024-08-21T00:20:04Z

This pull request has been mentioned on openHAB Community. There might be relevant details there:

https://community.openhab.org/t/insteon-binding-beta-4-1-0-5-0-0/154128/1

lsiepel · 2024-08-29T10:05:15Z

Spend some time looking at this PR. Pretty impressed by the documentation. On a high level i think this is the way forward. There also seem to be changed in the legacy part that i can't directly link as essential to integrating. They also don;t seem wrong, but with a large rewrite as this regressions are expected.
I guess you already tested and upgrade your own setup, did you share this version and/or do you have test results from others?
I'll try to complete my review soon, but 157 files.. this might take some time.

jsetton · 2024-09-03T17:15:41Z

I guess you already tested and upgrade your own setup,

I did test the legacy code with my own setup without any issue. There are no change in behavior for existing users. As far as the new changes, I have been solely using my implementation for couple years now to test and fix any issues.

did you share this version

Yes, it is available in the marketplace as beta version.

do you have test results from others?

I have had several test results for the new changes from others over the previous PR comments and community forum posts. I have reached out to several of them asking for feedback and if possible to test the backward compatibility support.

openhab-bot · 2024-09-03T17:27:35Z

This pull request has been mentioned on openHAB Community. There might be relevant details there:

https://community.openhab.org/t/insteon-binding-beta-4-1-0-5-0-0/154128/8

lsiepel · 2024-09-03T19:36:03Z

Can you try not to force push as then it is hard to review / track changes.

jsetton · 2024-09-04T22:25:39Z

@lsiepel I did some refactoring around the transport message code improving the separation between a Msg object and the MsgDefinition object it is derived from. Let me know if you want me to push this change to this PR or wait until you finish reviewing the current changes.

lsiepel · 2024-09-05T07:10:03Z

@lsiepel I did some refactoring around the transport message code improving the separation between a Msg object and the MsgDefinition object it is derived from. Let me know if you want me to push this change to this PR or wait until you finish reviewing the current changes.

Let us first finish this PR before we do further refactoring.

lsiepel

Not covered everything yet (114 / 157 files), but made some progress revieweing this PR and don't want to hold it back.

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

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

...nsteon/src/main/java/org/openhab/binding/insteon/internal/device/feature/CommandHandler.java

...on/src/main/java/org/openhab/binding/insteon/internal/discovery/InsteonDiscoveryService.java

.../main/java/org/openhab/binding/insteon/internal/discovery/InsteonLegacyDiscoveryService.java

Signed-off-by: jsetton <[email protected]>

* introduced device configuration automated determination * converted mode-based number items to string type with descriptive states * added number items uom support * added scenes and x10 device things * added distinct bridge things for supported hub/plm devices * added button event trigger channels * added device products configuration layer * added device link database support * added device cache storage * added device operating flags controls * added modem configuration flags controls * added related devices synchronization feature * added heartbeat timeout monitor * added ability to link/unlink a device to the modem * added ability to add missing default links * added link database & scene management support * added scene state support * added new i3 devices basic support * added ezrain sprinkler device support * revamped console commands with auto-completion support * improved discovery service * improved thing status Signed-off-by: jsetton <[email protected]>

Signed-off-by: jsetton <[email protected]>

jsetton · 2024-10-01T01:01:00Z

Sorry again about the force push, I had to rebase my changes to clear another upstream merge conflict.

All the changes I made related to your code review starts from 2f25a14.

Signed-off-by: jsetton <[email protected]>

lsiepel

Reviewed the remaining files and looked back at the previous round. That was a lot to digest. Initialy my intention was to have a less integrated binding with old/new behaviour to prevent regression and make review easier. Hopefully i have not missed anything. Once these comments are finished, i strongly advice to get some more users to test this binding with the workflow of 1. use it 'as is' for one week, 2. migrate and 3. use it for another week with the new thing types.

Unfortunately it won't be in todays milestone release, but hopefully in the next to get some more test milage before the stable release around christmas.

...on/src/main/java/org/openhab/binding/insteon/internal/device/feature/BaseFeatureHandler.java

....insteon/src/main/java/org/openhab/binding/insteon/internal/device/feature/HandlerEntry.java

...src/main/java/org/openhab/binding/insteon/internal/device/feature/LegacyFeatureTemplate.java

...binding.insteon/src/main/java/org/openhab/binding/insteon/internal/InsteonLegacyBinding.java

...n/src/main/java/org/openhab/binding/insteon/internal/handler/InsteonLegacyDeviceHandler.java

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

Signed-off-by: jsetton <[email protected]>

jsetton · 2024-10-06T18:50:14Z

I strongly advice to get some more users to test this binding with the workflow of 1. use it 'as is' for one week, 2. migrate and 3. use it for another week with the new thing types.

I had one user test both already. Another one has been focused on first testing new thing on the side and am working with him to test the migration portion.

Signed-off-by: jsetton <[email protected]>

lsiepel · 2024-10-10T04:49:09Z

Thanks for your patience and the work you put in this PR. While it is more integrated then I initially aimed for, it seems pretty clean and a lot has been done to prevent regressions.

jsetton · 2024-10-10T14:47:28Z

@lsiepel I did some refactoring around the transport message code improving the separation between a Msg object and the MsgDefinition object it is derived from.

#17537

* [insteon] Restructure legacy code for rewrite Signed-off-by: jsetton <[email protected]>

jsetton requested a review from a team as a code owner July 25, 2024 02:26

jsetton mentioned this pull request Jul 25, 2024

[insteon] Integrate new binding features #17096

Closed

jsetton force-pushed the insteon branch 3 times, most recently from 41db1f9 to f096ded Compare July 26, 2024 05:08

jsetton force-pushed the insteon branch 2 times, most recently from 56505e5 to 70b99f3 Compare July 26, 2024 16:51

jsetton force-pushed the insteon branch 8 times, most recently from 7dfc222 to c45173e Compare August 1, 2024 14:18

jsetton force-pushed the insteon branch from c45173e to 127f5a1 Compare September 3, 2024 19:00

lsiepel requested changes Sep 14, 2024

View reviewed changes

jsetton force-pushed the insteon branch from 3efae9e to d5e65da Compare September 18, 2024 00:07

jsetton added 3 commits September 30, 2024 20:41

[insteon] Restructure legacy code for rewrite

a2ef429

Signed-off-by: jsetton <[email protected]>

[insteon] Code review changes

2f25a14

Signed-off-by: jsetton <[email protected]>

jsetton added 10 commits September 30, 2024 20:54

[insteon] Update channel labels and descriptions

befbfad

Signed-off-by: jsetton <[email protected]>

[insteon] Add migration guide and debug console commands

ed1f160

Signed-off-by: jsetton <[email protected]>

[insteon] Fix sat report logger static field

c1835e6

Signed-off-by: jsetton <[email protected]>

[Insteon] Fix some channel descriptions

a416cf1

Signed-off-by: jsetton <[email protected]>

[insteon] Extend legacy loader classes from resource loader

d44af3a

Signed-off-by: jsetton <[email protected]>

[insteon] Rename base feature handler class

fe78048

Signed-off-by: jsetton <[email protected]>

[insteon] Reduce debug logging

b1fbc58

Signed-off-by: jsetton <[email protected]>

[insteon] Update migration guide

df47a05

Signed-off-by: jsetton <[email protected]>

[insteon] Update discovery parameter labels and descriptions

dc5e52a

Signed-off-by: jsetton <[email protected]>

[insteon] Update thing status details per documentation

881e2f2

Signed-off-by: jsetton <[email protected]>

jsetton force-pushed the insteon branch from 21caffa to 881e2f2 Compare October 1, 2024 00:58

jsetton added 2 commits October 4, 2024 20:25

[insteon] Fix modem db reader restart on disconnect

4096e4e

Signed-off-by: jsetton <[email protected]>

[insteon] Add scene interface

e8a252f

Signed-off-by: jsetton <[email protected]>

lsiepel requested changes Oct 6, 2024

View reviewed changes

jsetton added 2 commits October 6, 2024 13:23

[insteon] Code review additional changes

b68b10a

Signed-off-by: jsetton <[email protected]>

[insteon] Fix markdown warnings

e69cbf9

Signed-off-by: jsetton <[email protected]>

[insteon] Update legacy classes meaningful names

f11860d

Signed-off-by: jsetton <[email protected]>

lsiepel added the enhancement An enhancement or new feature for an existing add-on label Oct 7, 2024

lsiepel mentioned this pull request Oct 9, 2024

[insteon] show Air Gapped and unplugged devices as "OFFLINE" on "Things" page with correct "lastHeardFrom" log entry. #12994

Closed

lsiepel linked an issue Oct 10, 2024 that may be closed by this pull request

[insteon] show Air Gapped and unplugged devices as "OFFLINE" on "Things" page with correct "lastHeardFrom" log entry. #12994

Closed

lsiepel approved these changes Oct 10, 2024

View reviewed changes

lsiepel merged commit d923eb9 into openhab:main Oct 10, 2024
5 checks passed

lsiepel added this to the 4.3 milestone Oct 10, 2024

jsetton deleted the insteon branch October 10, 2024 13:53

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

[insteon] Rewrite with backward compatibility (openhab#17146)

722dd75

* [insteon] Restructure legacy code for rewrite Signed-off-by: jsetton <[email protected]>

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

[insteon] Rewrite with backward compatibility (openhab#17146)

0e62885

* [insteon] Restructure legacy code for rewrite Signed-off-by: jsetton <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[insteon] Rewrite with backward compatibility #17146

[insteon] Rewrite with backward compatibility #17146

jsetton commented Jul 25, 2024 •

edited

Loading

lsiepel commented Jul 26, 2024

jsetton commented Jul 26, 2024

openhab-bot commented Aug 21, 2024

lsiepel commented Aug 29, 2024 •

edited

Loading

jsetton commented Sep 3, 2024

openhab-bot commented Sep 3, 2024

lsiepel commented Sep 3, 2024

jsetton commented Sep 4, 2024

lsiepel commented Sep 5, 2024

lsiepel left a comment •

edited

Loading

jsetton commented Oct 1, 2024

lsiepel left a comment •

edited

Loading

jsetton commented Oct 6, 2024

lsiepel commented Oct 10, 2024

jsetton commented Oct 10, 2024

[insteon] Rewrite with backward compatibility #17146

[insteon] Rewrite with backward compatibility #17146

Conversation

jsetton commented Jul 25, 2024 • edited Loading

lsiepel commented Jul 26, 2024

jsetton commented Jul 26, 2024

openhab-bot commented Aug 21, 2024

lsiepel commented Aug 29, 2024 • edited Loading

jsetton commented Sep 3, 2024

openhab-bot commented Sep 3, 2024

lsiepel commented Sep 3, 2024

jsetton commented Sep 4, 2024

lsiepel commented Sep 5, 2024

lsiepel left a comment • edited Loading

Choose a reason for hiding this comment

jsetton commented Oct 1, 2024

lsiepel left a comment • edited Loading

Choose a reason for hiding this comment

jsetton commented Oct 6, 2024

lsiepel commented Oct 10, 2024

jsetton commented Oct 10, 2024

jsetton commented Jul 25, 2024 •

edited

Loading

lsiepel commented Aug 29, 2024 •

edited

Loading

lsiepel left a comment •

edited

Loading

lsiepel left a comment •

edited

Loading