[insteon] Integrate new binding features

[lsiepel]

This is an attempt to integrate the features developed by @jsetton in #16761 into the existing insteon binding maintained by @robnielsen.

The goal of this PR is to add the new features to the binding with full backwards compatibility. Users should not need to adapt their configuration. They should be able to choose to migrate and use these new features whenever they want to. A list of features is available here: #16761 (comment)

Basically, i moved all the new classes and other files into the existing binding. For most collisions i tried to:

1. when backwards compatbility is in play, add V2 to the new object.
2. when backwards comp[atbility is not in play, add legacy to the existing object
3. Some parts where combined , like the factory and discovery.
4. Each Thing remained to have their existing (dedicated) handler, so the functionality and logic of the Thing (once instanciated) should not have changed.

At the time of writing there are multiple areas to work on:

1. The old part of the binding has had minimal changes, no logic should have been changed. But i might have made some reference mistake, so it needs some basis tests to confirm the two old Thing's work as before.
2. The new part of the binding also needs additional testing, just like any other new PR/binding.
3. Ther readme needs a revamp, but we can do that as last part of this PR.
4. There are 33 conflicts in channel type id's we need to determine if we can integrate those further and deduplicate those. I have not looked deeply into this, but i expect we can reduce those collisions. You can find them in the channsl.xml they have a V2 added to them so they are unique.
5. There are more parts that can be further enhanced / deduplicated like the Util classes.
6. The discovery of devices also needs attention. It should work as before.
7. There will be some more choices and topics to cover, will see how we handle them, we have plenty of time to see how this evolves and this integration matures before we finally merge it (or not), as long as we stay constructive and look for solutions.

A 4.3.0-SNAPSHOT test version is here: https://1drv.ms/u/s!AnMcxmvEeupwjvpEhMjkvONJ47bpPA?e=K76OGm (should be compatible with 4.2.x

Supersedes: #16761

[jsetton]

Thanks for working on this. However, I had already started to work on the merge on my end. There are already issues that I spotted and I think the binding needs to better maintainable with minimal redundancy. A lot of the legacy functions should be able to use the updated core functions.

[lsiepel]

Thanks for working on this. However, I had already started to work on the merge on my end. There are already issues that I spotted and I think the binding needs to better maintainable with minimal redundancy. A lot of the legacy functions should be able to use the updated core functions.

You can create a PR to this branch or suggest changes in the comments (abstract) or by a review (details).
Without knowing the changes you propose, be aware of robnielsen’s wish to keep changes small when they affect the existing part of the binding. We can always create follow up PR’s.

[jsetton]

I will probably layer my changes on top of yours so you can see the difference. Anyway like I said before, I do appreciate your initial effort but there are functional issues with what you are proposing. It might compile but it won't work. For example, the legacy device type won't load because it doesn't have the expected XML resources or no IOStream will be started for the network bridge.

[lsiepel]

I will probably layer my changes on top of yours so you can see the difference. Anyway like I said before, I do appreciate your initial effort but there are functional issues with what you are proposing. It might compile but it won't work. For example, the legacy device type won't load because it doesn't have the expected XML resources or no IOStream will be started for the network bridge.

It would be helpfull if you point me to the missing XML-resource. No resource has been removed. The same for the IOStream you mention. Please point me to the issue and we can work together to get this to the finish line.

[lsiepel]

I'm busy trying to get some test coverage. Not easy with this serialport.
@jsetton you probably meant the msg_definitions.xml ? This file is available, but when testing, it does not load. I don't understand yet why it is not loading as this logic has not changed and the file is available.

Nevermind, it is because Msg relies on: FrameworkUtil.getBundle(Msg.class); and when performing unittests that will return null. Let's see if we can mock or fix this somehow.

[jsetton]

@lsiepel I am referring to device_*.xml resources. These defines each device, from the channel it supports to how messages for that device should be handled.

Anyway, I do appreciate your effort and framing how the merge should happen but I am also working on my side to complete this. I would ask if you can hold off making additional changes until I provide my changes and we can work from there.

[lsiepel]

device_*.xml

The mentioned resource files are not changed by this PR, so the legacy devices should still work. Only changes that i make are test related. I won;t have much time the next few days, so i'l leave it for some time now.

Can you outline the changes you are making? Just to know what to expect and see how that aligns to this PR.

[jsetton]

The mentioned resource files are not changed by this PR, so the legacy devices should still work.

Correct but the "V2" changes won't work. These files are basically defining the majority of the integration. Two separate resource pipelines are needed with their own loaders and handlers. This is not an easy task as you can see.

[lsiepel]

The mentioned resource files are not changed by this PR, so the legacy devices should still work.

Correct but the "V2" changes won't work. These files are basically defining the majority of the integration. Two separate resource pipelines are needed with their own loaders and handlers. This is not an easy task as you can see.

On my phone so can’t check, but if the v2 has similar files they can be added with different names?! I’ll look into this. It doesn’t sound as a difficult task.

[jsetton]

I’ll look into this. It doesn’t sound as a difficult task.

Don't worry about it. I have already done some work on this. I am basically moving all the current implementation source code into a legacy folder with the exception of the handlers and some of other OH framework classes. The bigger issue is the console support. I may have to add a legacy command menu although I haven't look into it closely.

[jsetton]

@lsiepel How would you resolve the thing-type id conflict for device? I assume the new version will need to be renamed which I am reluctant to do. I couldn't find a way to re-map Thing ID to a given Thing Type ID, the same way it can be achieved at the channel level.

[lsiepel]

@lsiepel How would you resolve the thing-type id conflict for device? I assume the new version will need to be renamed which I am reluctant to do. I couldn't find a way to re-map Thing ID to a given Thing Type ID, the same way it can be achieved at the channel level.

Unofrtunately, I don't see any other option then to rename the new variant as we must maintain backwards compatbility. Integrating both devices into one handler / thing type is difficult, might not even possible and would break the promise to @robnielsen that the current code should not be affected / introduce new bugs as the code is very stable.

[jsetton]

@lsiepel I submitted PR #17146 structuring the commits to show the changes that were made to the original code. Some adaptation was done to connect the legacy code to the refactored IOStream and Msg transport core components and the new DeviceAddress interface. Feel free to merge the tests you added in this PR.

The backward compatibility is achieved by migrating existing device things connected to a network bridge to the legacy-device thing type while still keeping the same ids to prevent any breakage. Subsequent, legacy device things discovered or added in MainUI will directly use the legacy-device thing allowing the device thing to be used by the new workflow.

The documentation is also up to date including some collapsible sections with the legacy information.

All these changes were tested on an Insteon hub successfully.

As far as conflicts, here is what I have done:

• All remaining legacy classes were renamed adding Legacy in their name.
• All legacy channel type ids and configuration description uris were prefixed with legacy. Since the channels are dynamically added during the InsteonLegacyDeviceHandler initialization, existing channels will also be migrated with their current configuration.
• The console command extension will automatically fallback to the legacy command extension whenever a network bridge is active.
• When a user adds a new bridge with a configuration related to a legacy network bridge, the latter will be disabled (via ThingManager) in favor of the former. This is to prevent having two bridges connecting to the same modem at the same time. The same will happened if a user tries to enable a legacy bridge when a related new bridge is currently enabled.