Skip to content

Commit

Permalink
[insteon] Fix legacy backward compatibility
Browse files Browse the repository at this point in the history 
Signed-off-by: jsetton <[email protected]>
  • Loading branch information
@jsetton
jsetton committed Dec 25, 2024
1 parent 0e5e48f commit 6dbe2ea
Show file tree
Hide file tree
Showing 5 changed files with 52 additions and 51 deletions.
37 changes: 19 additions & 18 deletions bundles/org.openhab.binding.insteon/README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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]
}
}
```
Expand Down Expand Up @@ -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]
}
}
```
Expand Down Expand Up @@ -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" ]
}
}
```
Expand Down Expand Up @@ -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
}
}
```
Expand Down Expand Up @@ -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"]
}
}
```
Expand All @@ -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"]
Expand Down
3 changes: 3 additions & 0 deletions ...in/java/org/openhab/binding/insteon/internal/discovery/InsteonLegacyDiscoveryService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,13 +59,16 @@ public void addInsteonDevices(List<InsteonAddress> 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<String, Object> properties = new HashMap<>();
properties.put(PROPERTY_DEVICE_ADDRESS, address.toString());

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);
}
}
Expand Down
8 changes: 1 addition & 7 deletions ...teon/src/main/java/org/openhab/binding/insteon/internal/handler/InsteonDeviceHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -114,10 +111,7 @@ public void initialize() {

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);
getThing().getChannels().forEach(legacyNetworkHandler::cacheChannel);
}

changeThingType(thingTypeUID, getConfig());
Expand Down
38 changes: 18 additions & 20 deletions ...rc/main/java/org/openhab/binding/insteon/internal/handler/InsteonLegacyDeviceHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -203,12 +201,17 @@ public void initialize() {
if (feature != null) {
if (!feature.isFeatureGroup()) {
if (channelId.equalsIgnoreCase(BROADCAST_ON_OFF)) {
Set<String> 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));
}
}

Expand All @@ -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;
Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -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) {
Expand Down
17 changes: 11 additions & 6 deletions ...c/main/java/org/openhab/binding/insteon/internal/handler/InsteonLegacyNetworkHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -74,7 +74,7 @@ public class InsteonLegacyNetworkHandler extends BaseBridgeHandler {
private ThingRegistry thingRegistry;
private Map<String, String> deviceInfo = new ConcurrentHashMap<>();
private Map<String, String> channelInfo = new ConcurrentHashMap<>();
private Map<ChannelUID, Configuration> channelConfigs = new ConcurrentHashMap<>();
private Map<ChannelUID, Channel> channelCache = new ConcurrentHashMap<>();

public InsteonLegacyNetworkHandler(Bridge bridge, HttpClient httpClient, SerialPortManager serialPortManager,
ThingManager thingManager, ThingRegistry thingRegistry) {
Expand Down Expand Up @@ -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<Channel> getCachedChannels(ThingUID thingUID) {
return channelCache.values().stream().filter(channel -> channel.getUID().getThingUID().equals(thingUID))
.toList();
}

public void addChannelConfigs(Map<ChannelUID, Configuration> 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<String, String> info) {
Expand Down

0 comments on commit 6dbe2ea

Please sign in to comment.