From 6dbe2eabd0a007917cbbc235ed96e55dbf90c331 Mon Sep 17 00:00:00 2001 From: jsetton Date: Wed, 25 Dec 2024 02:07:13 +0000 Subject: [PATCH] [insteon] Fix legacy backward compatibility Signed-off-by: jsetton --- bundles/org.openhab.binding.insteon/README.md | 37 +++++++++--------- .../InsteonLegacyDiscoveryService.java | 3 ++ .../handler/InsteonDeviceHandler.java | 8 +--- .../handler/InsteonLegacyDeviceHandler.java | 38 +++++++++---------- .../handler/InsteonLegacyNetworkHandler.java | 17 ++++++--- 5 files changed, 52 insertions(+), 51 deletions(-) diff --git a/bundles/org.openhab.binding.insteon/README.md b/bundles/org.openhab.binding.insteon/README.md index 4bee1a400ea92..dd811e3ab787c 100644 --- a/bundles/org.openhab.binding.insteon/README.md +++ b/bundles/org.openhab.binding.insteon/README.md @@ -26,6 +26,7 @@ You can follow the [migration guide](#migration-guide). However, the new version is fully backward compatible by supporting the legacy things. On the first start, existing `device` things connected to a `network` bridge will be migrated to the `legacy-device` thing type while still keeping the same ids to prevent any breakage. +For textual configuration with defined thing channels, the channel types must be manually updated to the new ones by adding the `legacy` prefix and capitalizing the first letter, as shown in [these examples](#full-example). It is important to note that once the migration has occurred, downgrading to an older version will not be possible. ## Supported Things @@ -474,27 +475,27 @@ Bridge insteon:plm:home [serialPort="/dev/ttyUSB0"] { Bridge insteon:network:home [port="/dev/ttyUSB0"] { Thing device 22F8A8 [address="22.F8.A8", productKey="F00.00.15"] { Channels: - Type keypadButtonA : keypadButtonA [ group=3 ] - Type keypadButtonB : keypadButtonB [ group=4 ] - Type keypadButtonC : keypadButtonC [ group=5 ] - Type keypadButtonD : keypadButtonD [ group=6 ] + Type legacyKeypadButtonA : keypadButtonA [ group=3 ] + Type legacyKeypadButtonB : keypadButtonB [ group=4 ] + Type legacyKeypadButtonC : keypadButtonC [ group=5 ] + Type legacyKeypadButtonD : keypadButtonD [ group=6 ] } Thing device 238D93 [address="23.8D.93", productKey="F00.00.12"] Thing device 238F55 [address="23.8F.55", productKey="F00.00.11"] { Channels: - Type dimmer : dimmer [related="23.B0.D9+23.8F.C9"] + Type legacyDimmer : dimmer [related="23.B0.D9+23.8F.C9"] } Thing device 238FC9 [address="23.8F.C9", productKey="F00.00.11"] { Channels: - Type dimmer : dimmer [related="23.8F.55+23.B0.D9"] + Type legacyDimmer : dimmer [related="23.8F.55+23.B0.D9"] } Thing device 23B0D9 [address="23.B0.D9", productKey="F00.00.11"] { Channels: - Type dimmer : dimmer [related="23.8F.55+23.8F.C9"] + Type legacyDimmer : dimmer [related="23.8F.55+23.8F.C9"] } Thing device 243141 [address="24.31.41", productKey="F00.00.11"] { Channels: - Type dimmer : dimmer [dimmermax=60] + Type legacyDimmer : dimmer [dimmermax=60] } } ``` @@ -644,11 +645,11 @@ Thing device 23b0d9 [address="23.B0.D9"] { Bridge insteon:network:home [port="/dev/ttyUSB0"] { Thing device AABBCC [address="AA.BB.CC", productKey="F00.00.11"] { Channels: - Type dimmer : dimmer [dimmermax=70] + Type legacyDimmer : dimmer [dimmermax=70] } Thing device AABBCD [address="AA.BB.CD", productKey="F00.00.15"] { Channels: - Type loadDimmer : loadDimmer [dimmermax=60] + Type legacyLoadDimmer : loadDimmer [dimmermax=60] } } ``` @@ -782,10 +783,10 @@ end Bridge insteon:network:home [port="/dev/ttyUSB0"] { Thing device AABBCC [address="AA.BB.CC", productKey="F00.00.15"] { Channels: - Type keypadButtonA : keypadButtonA [ group="0xf3" ] - Type keypadButtonB : keypadButtonB [ group="0xf4" ] - Type keypadButtonC : keypadButtonC [ group="0xf5" ] - Type keypadButtonD : keypadButtonD [ group="0xf6" ] + Type legacyKeypadButtonA : keypadButtonA [ group="0xf3" ] + Type legacyKeypadButtonB : keypadButtonB [ group="0xf4" ] + Type legacyKeypadButtonC : keypadButtonC [ group="0xf5" ] + Type legacyKeypadButtonD : keypadButtonD [ group="0xf6" ] } } ``` @@ -1324,7 +1325,7 @@ An `ON` state indicates that all the device states associated to a scene are mat Bridge insteon:network:home [port="/dev/ttyUSB0"] { Thing device AABBCC [address="AA.BB.CC", productKey="0x000045"] { Channels: - Type broadcastOnOff : broadcastOnOff#2 + Type legacyBroadcastOnOff : broadcastOnOff#2 } } ``` @@ -1426,11 +1427,11 @@ For scenes, these will be polled based on the modem database, after sending a gr Bridge insteon:network:home [port="/dev/ttyUSB0"] { Thing device AABBCC [address="AA.BB.CC", productKey="F00.00.11"] { Channels: - Type dimmer : dimmer [related="AA.BB.DD"] + Type legacyDimmer : dimmer [related="AA.BB.DD"] } Thing device AABBDD [address="AA.BB.DD", productKey="F00.00.11"] { Channels: - Type dimmer : dimmer [related="AA.BB.CC"] + Type legacyDimmer : dimmer [related="AA.BB.CC"] } } ``` @@ -1444,7 +1445,7 @@ For scenes, these will be polled based on the modem database, after sending a gr Bridge insteon:network:home [port="/dev/ttyUSB0"] { Thing device AABBCC [address="AA.BB.CC", productKey="0x000045"] { Channels: - Type broadcastOnOff : broadcastOnOff#3 [related="AA.BB.DD+AA.BB.EE"] + Type legacyBroadcastOnOff : broadcastOnOff#3 [related="AA.BB.DD+AA.BB.EE"] } Thing device AABBDD [address="AA.BB.DD", productKey="F00.00.11"] Thing device AABBEE [address="AA.BB.EE", productKey="F00.00.11"] diff --git a/bundles/org.openhab.binding.insteon/src/main/java/org/openhab/binding/insteon/internal/discovery/InsteonLegacyDiscoveryService.java b/bundles/org.openhab.binding.insteon/src/main/java/org/openhab/binding/insteon/internal/discovery/InsteonLegacyDiscoveryService.java index 521913b17b3ed..1771b5c9301c7 100644 --- a/bundles/org.openhab.binding.insteon/src/main/java/org/openhab/binding/insteon/internal/discovery/InsteonLegacyDiscoveryService.java +++ b/bundles/org.openhab.binding.insteon/src/main/java/org/openhab/binding/insteon/internal/discovery/InsteonLegacyDiscoveryService.java @@ -59,6 +59,7 @@ public void addInsteonDevices(List addresses) { ThingUID bridgeUID = handler.getThing().getUID(); String id = address.toString().replace(".", ""); ThingUID thingUID = new ThingUID(THING_TYPE_LEGACY_DEVICE, bridgeUID, id); + ThingUID oldThingUID = new ThingUID(THING_TYPE_DEVICE, bridgeUID, id); String label = "Insteon Device (Legacy) " + address; Map properties = new HashMap<>(); properties.put(PROPERTY_DEVICE_ADDRESS, address.toString()); @@ -66,6 +67,8 @@ public void addInsteonDevices(List addresses) { thingDiscovered(DiscoveryResultBuilder.create(thingUID).withBridge(bridgeUID).withLabel(label) .withProperties(properties).withRepresentationProperty(PROPERTY_DEVICE_ADDRESS).build()); + thingRemoved(oldThingUID); + logger.debug("added Insteon device {} to inbox", address); } } diff --git a/bundles/org.openhab.binding.insteon/src/main/java/org/openhab/binding/insteon/internal/handler/InsteonDeviceHandler.java b/bundles/org.openhab.binding.insteon/src/main/java/org/openhab/binding/insteon/internal/handler/InsteonDeviceHandler.java index 429c79baf664f..ad781645dcf35 100644 --- a/bundles/org.openhab.binding.insteon/src/main/java/org/openhab/binding/insteon/internal/handler/InsteonDeviceHandler.java +++ b/bundles/org.openhab.binding.insteon/src/main/java/org/openhab/binding/insteon/internal/handler/InsteonDeviceHandler.java @@ -15,10 +15,8 @@ import static org.openhab.binding.insteon.internal.InsteonBindingConstants.*; import java.util.List; -import java.util.Map; import java.util.concurrent.ScheduledFuture; import java.util.concurrent.TimeUnit; -import java.util.stream.Collectors; import org.eclipse.jdt.annotation.NonNullByDefault; import org.eclipse.jdt.annotation.Nullable; @@ -32,7 +30,6 @@ import org.openhab.binding.insteon.internal.device.InsteonDevice; import org.openhab.binding.insteon.internal.device.InsteonEngine; import org.openhab.binding.insteon.internal.device.InsteonModem; -import org.openhab.core.config.core.Configuration; import org.openhab.core.thing.Bridge; import org.openhab.core.thing.Channel; import org.openhab.core.thing.ChannelUID; @@ -114,10 +111,7 @@ public void initialize() { private void changeThingType(ThingTypeUID thingTypeUID, @Nullable BridgeHandler bridgeHandler) { if (bridgeHandler instanceof InsteonLegacyNetworkHandler legacyNetworkHandler) { - Map channelConfigs = getThing().getChannels().stream() - .collect(Collectors.toMap(Channel::getUID, Channel::getConfiguration)); - - legacyNetworkHandler.addChannelConfigs(channelConfigs); + getThing().getChannels().forEach(legacyNetworkHandler::cacheChannel); } changeThingType(thingTypeUID, getConfig()); diff --git a/bundles/org.openhab.binding.insteon/src/main/java/org/openhab/binding/insteon/internal/handler/InsteonLegacyDeviceHandler.java b/bundles/org.openhab.binding.insteon/src/main/java/org/openhab/binding/insteon/internal/handler/InsteonLegacyDeviceHandler.java index d32264ab88f1a..7485ab4a90477 100644 --- a/bundles/org.openhab.binding.insteon/src/main/java/org/openhab/binding/insteon/internal/handler/InsteonLegacyDeviceHandler.java +++ b/bundles/org.openhab.binding.insteon/src/main/java/org/openhab/binding/insteon/internal/handler/InsteonLegacyDeviceHandler.java @@ -19,11 +19,9 @@ import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; -import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Objects; -import java.util.Set; import org.eclipse.jdt.annotation.NonNullByDefault; import org.eclipse.jdt.annotation.Nullable; @@ -37,7 +35,6 @@ import org.openhab.binding.insteon.internal.device.LegacyDeviceFeature; import org.openhab.binding.insteon.internal.device.LegacyDeviceTypeLoader; import org.openhab.binding.insteon.internal.device.X10Address; -import org.openhab.core.config.core.Configuration; import org.openhab.core.thing.Bridge; import org.openhab.core.thing.Channel; import org.openhab.core.thing.ChannelUID; @@ -46,6 +43,7 @@ import org.openhab.core.thing.ThingStatusDetail; import org.openhab.core.thing.binding.BaseThingHandler; import org.openhab.core.thing.binding.ThingHandlerCallback; +import org.openhab.core.thing.binding.builder.ChannelBuilder; import org.openhab.core.thing.type.ChannelTypeUID; import org.openhab.core.types.Command; import org.openhab.core.util.StringUtils; @@ -203,12 +201,17 @@ public void initialize() { if (feature != null) { if (!feature.isFeatureGroup()) { if (channelId.equalsIgnoreCase(BROADCAST_ON_OFF)) { - Set broadcastChannels = new HashSet<>(); for (Channel channel : thing.getChannels()) { String id = channel.getUID().getId(); if (id.startsWith(BROADCAST_ON_OFF)) { channelMap.put(id, channel); - broadcastChannels.add(id); + } + } + + for (Channel channel : getInsteonNetworkHandler().getCachedChannels(thing.getUID())) { + String id = channel.getUID().getId(); + if (id.startsWith(BROADCAST_ON_OFF)) { + channelMap.putIfAbsent(id, createChannel(id, BROADCAST_ON_OFF, callback)); } } @@ -220,10 +223,7 @@ public void initialize() { for (Object value : list) { if (value instanceof Double doubleValue && doubleValue % 1 == 0) { String id = BROADCAST_ON_OFF + "#" + doubleValue.intValue(); - if (!broadcastChannels.contains(id)) { - channelMap.put(id, createChannel(id, BROADCAST_ON_OFF, callback)); - broadcastChannels.add(id); - } + channelMap.putIfAbsent(id, createChannel(id, BROADCAST_ON_OFF, callback)); } else { valid = false; break; @@ -315,6 +315,9 @@ public void dispose() { if (getBridge() != null && address != null) { getInsteonBinding().removeDevice(address); + getThing().getChannels().stream().map(Channel::getUID).filter(this::isLinked) + .forEach(this::channelUnlinked); + logger.debug("removed {} address = {}", getThing().getUID().getAsString(), address); } @@ -481,23 +484,18 @@ private Channel createChannel(String channelId, String channelTypeId, ThingHandl 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 oldChannel = getInsteonNetworkHandler().pollCachedChannel(channelUID); Channel channel = getThing().getChannel(channelUID); if (channel == null) { - channel = callback.createChannelBuilder(channelUID, channelTypeUID).withConfiguration(channelConfig) - .build(); + if (oldChannel == null) { + channel = callback.createChannelBuilder(channelUID, channelTypeUID).build(); + } else { + channel = ChannelBuilder.create(oldChannel).withType(channelTypeUID).build(); + } } return channel; } - private Configuration getChannelConfig(ChannelUID channelUID) { - try { - return getInsteonNetworkHandler().getChannelConfig(channelUID); - } catch (IllegalArgumentException e) { - return new Configuration(); - } - } - private InsteonLegacyNetworkHandler getInsteonNetworkHandler() { Bridge bridge = getBridge(); if (bridge == null) { diff --git a/bundles/org.openhab.binding.insteon/src/main/java/org/openhab/binding/insteon/internal/handler/InsteonLegacyNetworkHandler.java b/bundles/org.openhab.binding.insteon/src/main/java/org/openhab/binding/insteon/internal/handler/InsteonLegacyNetworkHandler.java index 53471200dd378..264a9dca06076 100644 --- a/bundles/org.openhab.binding.insteon/src/main/java/org/openhab/binding/insteon/internal/handler/InsteonLegacyNetworkHandler.java +++ b/bundles/org.openhab.binding.insteon/src/main/java/org/openhab/binding/insteon/internal/handler/InsteonLegacyNetworkHandler.java @@ -28,10 +28,10 @@ import org.openhab.binding.insteon.internal.device.DeviceAddress; import org.openhab.binding.insteon.internal.device.InsteonAddress; import org.openhab.binding.insteon.internal.discovery.InsteonLegacyDiscoveryService; -import org.openhab.core.config.core.Configuration; import org.openhab.core.io.console.Console; import org.openhab.core.io.transport.serial.SerialPortManager; import org.openhab.core.thing.Bridge; +import org.openhab.core.thing.Channel; import org.openhab.core.thing.ChannelUID; import org.openhab.core.thing.Thing; import org.openhab.core.thing.ThingManager; @@ -74,7 +74,7 @@ public class InsteonLegacyNetworkHandler extends BaseBridgeHandler { private ThingRegistry thingRegistry; private Map deviceInfo = new ConcurrentHashMap<>(); private Map channelInfo = new ConcurrentHashMap<>(); - private Map channelConfigs = new ConcurrentHashMap<>(); + private Map channelCache = new ConcurrentHashMap<>(); public InsteonLegacyNetworkHandler(Bridge bridge, HttpClient httpClient, SerialPortManager serialPortManager, ThingManager thingManager, ThingRegistry thingRegistry) { @@ -318,12 +318,17 @@ public void unlinked(ChannelUID uid) { channelInfo.remove(uid.getAsString()); } - public Configuration getChannelConfig(ChannelUID channelUID) { - return channelConfigs.getOrDefault(channelUID, new Configuration()); + public List getCachedChannels(ThingUID thingUID) { + return channelCache.values().stream().filter(channel -> channel.getUID().getThingUID().equals(thingUID)) + .toList(); } - public void addChannelConfigs(Map channelConfigs) { - this.channelConfigs.putAll(channelConfigs); + public @Nullable Channel pollCachedChannel(ChannelUID channelUID) { + return channelCache.remove(channelUID); + } + + public void cacheChannel(Channel channel) { + channelCache.put(channel.getUID(), channel); } private void display(Console console, Map info) {