[mqtt.homeassistant] fix newStyleChannels

This fixes a couple of problems. The biggest is that if multiple components
were single channel OR no-name, they would all get mapped to a "groupId" of "",
and the next time the Thing started, all but one of them would disappear unless
new discovery information was received. While fixing that, I fixed two other
issues:
 * Multi-channel components with a blank name were getting all their channels
   dumped into the top level of no group. In the newStyleChannels world, we
   don't care about the component's name anymore - multi-channel components
   get a group, single-channel components do not.
 * Instead of statically saying that certain components are only ever single
   or multi-channel components, determine at runtime if a component only added
   a single channel, and if so re-classify it as a single-channel component,
   moving its channel out of the group (and just naming it as the group was
   named)

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

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,13 @@ public Channel getChannel() {
         return channel;
     }
 
+    public void replaceChannelUID(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();
+    }
+
     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,11 @@ 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 || (name != null && !name.isEmpty())) {
+            groupId = componentId = this.haID.getGroupId(channelConfiguration.getUniqueId(), newStyleChannels);
         } else {
-            this.groupId = this.singleChannelComponent ? haID.component : "";
-            this.channelGroupUID = null;
+            groupId = null;
+            componentId = "";
         }
         uniqueId = this.haID.getGroupId(channelConfiguration.getUniqueId(), false);
 
@@ -155,10 +147,18 @@ public AbstractComponent(ComponentFactory.ComponentConfiguration componentConfig
         }
     }
 
+    protected void finalizeChannels() {
+        if (newStyleChannels && channels.size() == 1) {
+            groupId = null;
+            channels.forEach(
+                    (id, componentChannel) -> componentChannel.replaceChannelUID(buildChannelUID(componentId)));
+        }
+    }
+
     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 +216,15 @@ 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;
     }
 
     /**
@@ -273,15 +273,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 +295,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 +311,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();
     }
 }

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

@@ -46,7 +46,7 @@ static class ChannelConfiguration extends AbstractChannelConfiguration {
     }
 
     public DeviceTrigger(ComponentFactory.ComponentConfiguration componentConfiguration, boolean newStyleChannels) {
-        super(componentConfiguration, ChannelConfiguration.class, newStyleChannels, true);
+        super(componentConfiguration, ChannelConfiguration.class, newStyleChannels);
 
         if (!"trigger".equals(channelConfiguration.automationType)) {
             throw new ConfigurationException("Component:DeviceTrigger must have automation_type 'trigger'");
@@ -69,5 +69,6 @@ public DeviceTrigger(ComponentFactory.ComponentConfiguration componentConfigurat
         buildChannel(channelConfiguration.type, ComponentChannelType.TRIGGER, value, getName(),
                 componentConfiguration.getUpdateListener())
                 .stateTopic(channelConfiguration.topic, channelConfiguration.getValueTemplate()).trigger(true).build();
+        finalizeChannels();
     }
 }

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

@@ -184,6 +184,7 @@ public Fan(ComponentFactory.ComponentConfiguration componentConfiguration, boole
                             channelConfiguration.getQos(), channelConfiguration.directionCommandTemplate)
                     .build();
         }
+        finalizeChannels();
     }
 
     private boolean handlePercentageCommand(Command command) {

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

@@ -292,6 +292,7 @@ protected Light(ComponentFactory.ComponentConfiguration builder, boolean newStyl
         colorTempValue = new NumberValue(min, max, BigDecimal.ONE, Units.MIRED);
 
         buildChannels();
+        finalizeChannels();
     }
 
     protected abstract void buildChannels();

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

@@ -121,6 +121,7 @@ public Lock(ComponentFactory.ComponentConfiguration componentConfiguration, bool
                     }
                     return true;
                 }).build();
+        finalizeChannels();
     }
 
     private void autoUpdate(boolean locking) {

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

@@ -71,7 +71,7 @@ static class ChannelConfiguration extends AbstractChannelConfiguration {
     }
 
     public Number(ComponentFactory.ComponentConfiguration componentConfiguration, boolean newStyleChannels) {
-        super(componentConfiguration, ChannelConfiguration.class, newStyleChannels, true);
+        super(componentConfiguration, ChannelConfiguration.class, newStyleChannels);
 
         boolean optimistic = channelConfiguration.optimistic != null ? channelConfiguration.optimistic
                 : channelConfiguration.stateTopic.isBlank();
@@ -89,5 +89,6 @@ public Number(ComponentFactory.ComponentConfiguration componentConfiguration, bo
                 .commandTopic(channelConfiguration.commandTopic, channelConfiguration.isRetain(),
                         channelConfiguration.getQos(), channelConfiguration.commandTemplate)
                 .build();
+        finalizeChannels();
     }
 }

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

@@ -46,7 +46,7 @@ static class ChannelConfiguration extends AbstractChannelConfiguration {
     }
 
     public Scene(ComponentFactory.ComponentConfiguration componentConfiguration, boolean newStyleChannels) {
-        super(componentConfiguration, ChannelConfiguration.class, newStyleChannels, true);
+        super(componentConfiguration, ChannelConfiguration.class, newStyleChannels);
 
         TextValue value = new TextValue(new String[] { channelConfiguration.payloadOn });
 
@@ -55,5 +55,6 @@ public Scene(ComponentFactory.ComponentConfiguration componentConfiguration, boo
                 .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/Select.java

@@ -56,7 +56,7 @@ static class ChannelConfiguration extends AbstractChannelConfiguration {
     }
 
     public Select(ComponentFactory.ComponentConfiguration componentConfiguration, boolean newStyleChannels) {
-        super(componentConfiguration, ChannelConfiguration.class, newStyleChannels, true);
+        super(componentConfiguration, ChannelConfiguration.class, newStyleChannels);
 
         boolean optimistic = channelConfiguration.optimistic != null ? channelConfiguration.optimistic
                 : channelConfiguration.stateTopic.isBlank();
@@ -73,5 +73,6 @@ public Select(ComponentFactory.ComponentConfiguration componentConfiguration, bo
                 .commandTopic(channelConfiguration.commandTopic, channelConfiguration.isRetain(),
                         channelConfiguration.getQos(), channelConfiguration.commandTemplate)
                 .build();
+        finalizeChannels();
     }
 }

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

@@ -69,7 +69,7 @@ static class ChannelConfiguration extends AbstractChannelConfiguration {
     }
 
     public Sensor(ComponentFactory.ComponentConfiguration componentConfiguration, boolean newStyleChannels) {
-        super(componentConfiguration, ChannelConfiguration.class, newStyleChannels, true);
+        super(componentConfiguration, ChannelConfiguration.class, newStyleChannels);
 
         Value value;
         String uom = channelConfiguration.unitOfMeasurement;
@@ -96,6 +96,7 @@ public Sensor(ComponentFactory.ComponentConfiguration componentConfiguration, bo
         buildChannel(SENSOR_CHANNEL_ID, type, value, getName(), getListener(componentConfiguration, value))
                 .stateTopic(channelConfiguration.stateTopic, channelConfiguration.getValueTemplate())//
                 .trigger(trigger).build();
+        finalizeChannels();
     }
 
     private ChannelStateUpdateListener getListener(ComponentFactory.ComponentConfiguration componentConfiguration,

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

@@ -61,7 +61,7 @@ static class ChannelConfiguration extends AbstractChannelConfiguration {
     }
 
     public Switch(ComponentFactory.ComponentConfiguration componentConfiguration, boolean newStyleChannels) {
-        super(componentConfiguration, ChannelConfiguration.class, newStyleChannels, true);
+        super(componentConfiguration, ChannelConfiguration.class, newStyleChannels);
 
         boolean optimistic = channelConfiguration.optimistic != null ? channelConfiguration.optimistic
                 : channelConfiguration.stateTopic.isBlank();
@@ -79,5 +79,6 @@ public Switch(ComponentFactory.ComponentConfiguration componentConfiguration, bo
                 .commandTopic(channelConfiguration.commandTopic, channelConfiguration.isRetain(),
                         channelConfiguration.getQos())
                 .build();
+        finalizeChannels();
     }
 }

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

@@ -290,6 +290,7 @@ public Vacuum(ComponentFactory.ComponentConfiguration componentConfiguration, bo
 
         buildOptionalChannel(JSON_ATTRIBUTES_CH_ID, ComponentChannelType.STRING, new TextValue(), updateListener, null,
                 null, channelConfiguration.jsonAttributesTemplate, channelConfiguration.jsonAttributesTopic);
+        finalizeChannels();
     }
 
     @Nullable