[mqtt.homeassistant] fix newStyleChannels (openhab#17491)

* [mqtt.homeassistant] fix newStyleChannels
* further simplify channel IDs

Signed-off-by: Cody Cutrer <cody@cutrer.us>

bundles/org.openhab.binding.mqtt.generic/src/main/java/org/openhab/binding/mqtt/generic/ChannelState.java

@@ -51,13 +51,13 @@ public class ChannelState implements MqttMessageSubscriber {
 
     // Immutable channel configuration
     protected final boolean readOnly;
-    protected final ChannelUID channelUID;
     protected final ChannelConfig config;
 
     /** Channel value **/
     protected final Value cachedValue;
 
     // Runtime variables
+    protected ChannelUID channelUID;
     private @Nullable MqttBrokerConnection connection;
     protected final ChannelTransformation incomingTransformation;
     protected final ChannelTransformation outgoingTransformation;
@@ -132,6 +132,11 @@ public ChannelUID channelUID() {
         return channelUID;
     }
 
+    // If the UID of the channel changed after it was initially created
+    public void setChannelUID(ChannelUID channelUID) {
+        this.channelUID = channelUID;
+    }
+
     /**
      * Incoming message from the MqttBrokerConnection
      *

bundles/org.openhab.binding.mqtt.homeassistant/src/main/java/org/openhab/binding/mqtt/homeassistant/internal/ComponentChannel.java

@@ -58,7 +58,7 @@
 @NonNullByDefault
 public class ComponentChannel {
     private final ChannelState channelState;
-    private final Channel channel;
+    private Channel channel;
     private final @Nullable StateDescription stateDescription;
     private final @Nullable CommandDescription commandDescription;
     private final ChannelStateUpdateListener channelStateUpdateListener;
@@ -77,6 +77,18 @@ public Channel getChannel() {
         return channel;
     }
 
+    public void resetUID(ChannelUID channelUID) {
+        channel = ChannelBuilder.create(channelUID, channel.getAcceptedItemType()).withType(channel.getChannelTypeUID())
+                .withKind(channel.getKind()).withLabel(Objects.requireNonNull(channel.getLabel()))
+                .withConfiguration(channel.getConfiguration()).withAutoUpdatePolicy(channel.getAutoUpdatePolicy())
+                .build();
+        channelState.setChannelUID(channelUID);
+    }
+
+    public void clearConfiguration() {
+        channel = ChannelBuilder.create(channel).withConfiguration(new Configuration()).build();
+    }
+
     public ChannelState getState() {
         return channelState;
     }

bundles/org.openhab.binding.mqtt.homeassistant/src/main/java/org/openhab/binding/mqtt/homeassistant/internal/component/AbstractComponent.java

@@ -24,6 +24,7 @@
 import org.eclipse.jdt.annotation.NonNullByDefault;
 import org.eclipse.jdt.annotation.Nullable;
 import org.openhab.binding.mqtt.generic.AvailabilityTracker;
+import org.openhab.binding.mqtt.generic.ChannelState;
 import org.openhab.binding.mqtt.generic.ChannelStateUpdateListener;
 import org.openhab.binding.mqtt.generic.MqttChannelStateDescriptionProvider;
 import org.openhab.binding.mqtt.generic.values.Value;
@@ -39,7 +40,6 @@
 import org.openhab.binding.mqtt.homeassistant.internal.config.dto.Device;
 import org.openhab.core.io.transport.mqtt.MqttBrokerConnection;
 import org.openhab.core.thing.Channel;
-import org.openhab.core.thing.ChannelGroupUID;
 import org.openhab.core.thing.ChannelUID;
 import org.openhab.core.thing.binding.generic.ChannelTransformation;
 import org.openhab.core.thing.type.ChannelDefinition;
@@ -65,7 +65,6 @@ public abstract class AbstractComponent<C extends AbstractChannelConfiguration>
 
     // Component location fields
     protected final ComponentConfiguration componentConfiguration;
-    protected final @Nullable ChannelGroupUID channelGroupUID;
     protected final HaID haID;
 
     // Channels and configuration
@@ -79,14 +78,10 @@ public abstract class AbstractComponent<C extends AbstractChannelConfiguration>
     protected final C channelConfiguration;
 
     protected boolean configSeen;
-    protected final boolean singleChannelComponent;
-    protected final String groupId;
+    protected final boolean newStyleChannels;
     protected final String uniqueId;
-
-    public AbstractComponent(ComponentFactory.ComponentConfiguration componentConfiguration, Class<C> clazz,
-            boolean newStyleChannels) {
-        this(componentConfiguration, clazz, newStyleChannels, false);
-    }
+    protected @Nullable String groupId;
+    protected String componentId;
 
     /**
      * Creates component based on generic configuration and component configuration type.
@@ -98,9 +93,9 @@ public AbstractComponent(ComponentFactory.ComponentConfiguration componentConfig
      *            (only if newStyleChannels is true)
      */
     public AbstractComponent(ComponentFactory.ComponentConfiguration componentConfiguration, Class<C> clazz,
-            boolean newStyleChannels, boolean singleChannelComponent) {
+            boolean newStyleChannels) {
         this.componentConfiguration = componentConfiguration;
-        this.singleChannelComponent = newStyleChannels && singleChannelComponent;
+        this.newStyleChannels = newStyleChannels;
 
         this.channelConfigurationJson = componentConfiguration.getConfigJSON();
         this.channelConfiguration = componentConfiguration.getConfig(clazz);
@@ -109,14 +104,16 @@ public AbstractComponent(ComponentFactory.ComponentConfiguration componentConfig
         this.haID = componentConfiguration.getHaID();
 
         String name = channelConfiguration.getName();
-        if (name != null && !name.isEmpty()) {
-            groupId = this.haID.getGroupId(channelConfiguration.getUniqueId(), newStyleChannels);
-
-            this.channelGroupUID = this.singleChannelComponent ? null
-                    : new ChannelGroupUID(componentConfiguration.getThingUID(), groupId);
+        if (newStyleChannels) {
+            // try for a simple component/group ID first; if there are conflicts
+            // (components of different types, but the same object id)
+            // we'll resolve them later
+            groupId = componentId = haID.objectID.replace('-', '_');
+        } else if (name != null && !name.isEmpty()) {
+            groupId = componentId = this.haID.getGroupId(channelConfiguration.getUniqueId(), false);
         } else {
-            this.groupId = this.singleChannelComponent ? haID.component : "";
-            this.channelGroupUID = null;
+            groupId = null;
+            componentId = "";
         }
         uniqueId = this.haID.getGroupId(channelConfiguration.getUniqueId(), false);
 
@@ -155,10 +152,30 @@ public AbstractComponent(ComponentFactory.ComponentConfiguration componentConfig
         }
     }
 
+    protected void finalizeChannels() {
+        if (!newStyleChannels) {
+            return;
+        }
+        if (channels.size() == 1) {
+            groupId = null;
+            channels.values().forEach(c -> c.resetUID(buildChannelUID(componentId)));
+        } else {
+            // only the first channel needs to persist the configuration
+            channels.values().stream().skip(1).forEach(c -> {
+                c.clearConfiguration();
+            });
+        }
+    }
+
+    public void resolveConflict() {
+        componentId = this.haID.getGroupId(channelConfiguration.getUniqueId(), newStyleChannels);
+        channels.values().forEach(c -> c.resetUID(buildChannelUID(c.getChannel().getUID().getIdWithoutGroup())));
+    }
+
     protected ComponentChannel.Builder buildChannel(String channelID, ComponentChannelType channelType,
             Value valueState, String label, ChannelStateUpdateListener channelStateUpdateListener) {
-        if (singleChannelComponent) {
-            channelID = groupId;
+        if (groupId == null) {
+            channelID = componentId;
         }
         return new ComponentChannel.Builder(this, channelID, channelType.getChannelTypeUID(), valueState, label,
                 channelStateUpdateListener);
@@ -216,15 +233,19 @@ public void addStateDescriptions(MqttChannelStateDescriptionProvider stateDescri
     }
 
     public ChannelUID buildChannelUID(String channelID) {
-        final ChannelGroupUID groupUID = channelGroupUID;
-        if (groupUID != null) {
-            return new ChannelUID(groupUID, channelID);
+        final String localGroupID = groupId;
+        if (localGroupID != null) {
+            return new ChannelUID(componentConfiguration.getThingUID(), localGroupID, channelID);
         }
         return new ChannelUID(componentConfiguration.getThingUID(), channelID);
     }
 
-    public String getGroupId() {
-        return groupId;
+    public String getComponentId() {
+        return componentId;
+    }
+
+    public String getUniqueId() {
+        return uniqueId;
     }
 
     /**
@@ -273,15 +294,15 @@ public int getConfigHash() {
      * Return the channel group type.
      */
     public @Nullable ChannelGroupType getChannelGroupType(String prefix) {
-        if (channelGroupUID == null) {
+        if (groupId == null) {
             return null;
         }
         return ChannelGroupTypeBuilder.instance(getChannelGroupTypeUID(prefix), getName())
                 .withChannelDefinitions(getAllChannelDefinitions()).build();
     }
 
     public List<ChannelDefinition> getChannelDefinitions() {
-        if (channelGroupUID != null) {
+        if (groupId != null) {
             return List.of();
         }
         return getAllChannelDefinitions();
@@ -295,6 +316,10 @@ public List<Channel> getChannels() {
         return channels.values().stream().map(ComponentChannel::getChannel).toList();
     }
 
+    public void getChannelStates(Map<ChannelUID, ChannelState> states) {
+        channels.values().forEach(c -> states.put(c.getChannel().getUID(), c.getState()));
+    }
+
     /**
      * Resets all channel states to state UNDEF. Call this method after the connection
      * to the MQTT broker got lost.
@@ -307,14 +332,15 @@ public void resetState() {
      * Return the channel group definition for this component.
      */
     public @Nullable ChannelGroupDefinition getGroupDefinition(String prefix) {
-        if (channelGroupUID == null) {
+        String localGroupId = groupId;
+        if (localGroupId == null) {
             return null;
         }
-        return new ChannelGroupDefinition(channelGroupUID.getId(), getChannelGroupTypeUID(prefix), getName(), null);
+        return new ChannelGroupDefinition(localGroupId, getChannelGroupTypeUID(prefix), getName(), null);
     }
 
     public boolean hasGroup() {
-        return channelGroupUID != null;
+        return groupId != null;
     }
 
     public HaID getHaID() {

bundles/org.openhab.binding.mqtt.homeassistant/src/main/java/org/openhab/binding/mqtt/homeassistant/internal/component/AlarmControlPanel.java

@@ -97,5 +97,6 @@ public AlarmControlPanel(ComponentFactory.ComponentConfiguration componentConfig
                     componentConfiguration.getUpdateListener())
                     .commandTopic(commandTopic, channelConfiguration.isRetain(), channelConfiguration.getQos()).build();
         }
+        finalizeChannels();
     }
 }

bundles/org.openhab.binding.mqtt.homeassistant/src/main/java/org/openhab/binding/mqtt/homeassistant/internal/component/BinarySensor.java

@@ -69,14 +69,15 @@ static class ChannelConfiguration extends AbstractChannelConfiguration {
     }
 
     public BinarySensor(ComponentFactory.ComponentConfiguration componentConfiguration, boolean newStyleChannels) {
-        super(componentConfiguration, ChannelConfiguration.class, newStyleChannels, true);
+        super(componentConfiguration, ChannelConfiguration.class, newStyleChannels);
 
         OnOffValue value = new OnOffValue(channelConfiguration.payloadOn, channelConfiguration.payloadOff);
 
         buildChannel(SENSOR_CHANNEL_ID, ComponentChannelType.SWITCH, value, getName(),
                 getListener(componentConfiguration, value))
                 .stateTopic(channelConfiguration.stateTopic, channelConfiguration.getValueTemplate())
                 .withAutoUpdatePolicy(AutoUpdatePolicy.VETO).build();
+        finalizeChannels();
     }
 
     private ChannelStateUpdateListener getListener(ComponentFactory.ComponentConfiguration componentConfiguration,

bundles/org.openhab.binding.mqtt.homeassistant/src/main/java/org/openhab/binding/mqtt/homeassistant/internal/component/Button.java

@@ -48,7 +48,7 @@ static class ChannelConfiguration extends AbstractChannelConfiguration {
     }
 
     public Button(ComponentFactory.ComponentConfiguration componentConfiguration, boolean newStyleChannels) {
-        super(componentConfiguration, ChannelConfiguration.class, newStyleChannels, true);
+        super(componentConfiguration, ChannelConfiguration.class, newStyleChannels);
 
         TextValue value = new TextValue(new String[] { channelConfiguration.payloadPress });
 
@@ -57,5 +57,6 @@ public Button(ComponentFactory.ComponentConfiguration componentConfiguration, bo
                 .commandTopic(channelConfiguration.commandTopic, channelConfiguration.isRetain(),
                         channelConfiguration.getQos())
                 .withAutoUpdatePolicy(AutoUpdatePolicy.VETO).build();
+        finalizeChannels();
     }
 }

bundles/org.openhab.binding.mqtt.homeassistant/src/main/java/org/openhab/binding/mqtt/homeassistant/internal/component/Camera.java

@@ -46,5 +46,6 @@ public Camera(ComponentFactory.ComponentConfiguration componentConfiguration, bo
 
         buildChannel(CAMERA_CHANNEL_ID, ComponentChannelType.IMAGE, value, getName(),
                 componentConfiguration.getUpdateListener()).stateTopic(channelConfiguration.topic).build();
+        finalizeChannels();
     }
 }

bundles/org.openhab.binding.mqtt.homeassistant/src/main/java/org/openhab/binding/mqtt/homeassistant/internal/component/Climate.java

@@ -284,6 +284,7 @@ public Climate(ComponentFactory.ComponentConfiguration componentConfiguration, b
 
         buildOptionalChannel(POWER_CH_ID, ComponentChannelType.SWITCH, new OnOffValue(), updateListener, null,
                 channelConfiguration.powerCommandTopic, null, null, null);
+        finalizeChannels();
     }
 
     @Nullable

bundles/org.openhab.binding.mqtt.homeassistant/src/main/java/org/openhab/binding/mqtt/homeassistant/internal/component/Cover.java

@@ -150,5 +150,6 @@ public Cover(ComponentFactory.ComponentConfiguration componentConfiguration, boo
                     }
                     return true;
                 }).build();
+        finalizeChannels();
     }
 }