[insteon] Rewrite with backward compatibility

[jsetton]

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]

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]

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]

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]

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]

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]

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]

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

[jsetton]

@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]

@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.

[lsiepel]

Updating the channel type id's for existing thing's would require upgrade instructions. (applies to all changed type id's in this file.

[jsetton]

This is handled by InsteonLegacyDeviceHandler during the initialization as it creates all the channels using the legacy types. There is no need for upgrade instructions.

[lsiepel]

I would expect this to be a breaking change.
If you have a legacy device configured, and you upgrade to this binding, the configuration uri is not updated in the store (json db) resulting in an unconfigured device. Noit 100% if this can be fixed with upgrade instructions, so depending on your test result you might 1. add it to the ugprade instructions or 2. revert this change.

[jsetton]

This is why I added a way to store the existing channel configuration in InsteonLegacyNetworkHandler which are restored during the InsteonLegacyDeviceHandler initialization, part of the migration process.

Newly created channels will use the new config description uri along with the stored channel configuration since the channel uid isn't changing. I was able to test it with my setup.

openhab-addons/bundles/org.openhab.binding.insteon/src/main/java/org/openhab/binding/insteon/internal/handler/InsteonDeviceHandler.java

Lines 116 to 125 in d5e65da

	private void changeThingType(ThingTypeUID thingTypeUID, @Nullable BridgeHandler bridgeHandler) {
	if (bridgeHandler instanceof InsteonLegacyNetworkHandler legacyNetworkHandler) {
	Map<ChannelUID, Configuration> channelConfigs = getThing().getChannels().stream()
	.collect(Collectors.toMap(Channel::getUID, Channel::getConfiguration));
	legacyNetworkHandler.addChannelConfigs(channelConfigs);
	}
	changeThingType(thingTypeUID, getConfig());
	}

openhab-addons/bundles/org.openhab.binding.insteon/src/main/java/org/openhab/binding/insteon/internal/handler/InsteonLegacyDeviceHandler.java

Lines 487 to 498 in d5e65da

	private Channel createChannel(String channelId, String channelTypeId, ThingHandlerCallback callback) {
	ChannelUID channelUID = new ChannelUID(getThing().getUID(), channelId);
	ChannelTypeUID channelTypeUID = new ChannelTypeUID(InsteonBindingConstants.BINDING_ID,
	CHANNEL_TYPE_ID_PREFIX + StringUtils.capitalize(channelTypeId));
	Configuration channelConfig = getChannelConfig(channelUID);
	Channel channel = getThing().getChannel(channelUID);
	if (channel == null) {
	channel = callback.createChannelBuilder(channelUID, channelTypeUID).withConfiguration(channelConfig)
	.build();
	}
	return channel;
	}

[jsetton]

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

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