[dwdunwetter] Rework channel creation (#9229)

Signed-off-by: Christoph Weitkamp <github@christophweitkamp.de>

bundles/org.openhab.binding.dwdunwetter/src/main/java/org/openhab/binding/dwdunwetter/internal/config/DwdUnwetterConfiguration.java

@@ -22,6 +22,6 @@ import org.eclipse.jdt.annotation.NonNullByDefault;
 @NonNullByDefault
 public class DwdUnwetterConfiguration {
     public int refresh;
-    public int warningCount;
+    public int warningCount = 1;
     public String cellId = "";
 }

bundles/org.openhab.binding.dwdunwetter/src/main/java/org/openhab/binding/dwdunwetter/internal/handler/DwdUnwetterHandler.java

@@ -18,6 +18,7 @@ import java.util.ArrayList;
 import java.util.List;
 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;
@@ -30,9 +31,10 @@ import org.openhab.core.thing.ChannelUID;
 import org.openhab.core.thing.Thing;
 import org.openhab.core.thing.ThingStatus;
 import org.openhab.core.thing.binding.BaseThingHandler;
-import org.openhab.core.thing.binding.builder.ChannelBuilder;
+import org.openhab.core.thing.binding.ThingHandlerCallback;
-import org.openhab.core.thing.type.ChannelKind;
+import org.openhab.core.thing.binding.builder.ThingBuilder;
 import org.openhab.core.thing.type.ChannelTypeUID;
+import org.openhab.core.thing.util.ThingHandlerHelper;
 import org.openhab.core.types.Command;
 import org.openhab.core.types.RefreshType;
 import org.openhab.core.types.State;
@@ -54,7 +56,6 @@ public class DwdUnwetterHandler extends BaseThingHandler {
     private @Nullable DwdWarningsData data;
 
     private boolean inRefresh;
-    private boolean initializing;
 
     public DwdUnwetterHandler(Thing thing) {
         super(thing);
@@ -79,14 +80,8 @@ public class DwdUnwetterHandler extends BaseThingHandler {
             return;
         }
 
-        if (initializing) {
+        if (!ThingHandlerHelper.isHandlerInitialized(getThing())) {
-            logger.trace("Still initializing. Ignoring refresh request.");
+            logger.debug("Unable to refresh. Thing status is '{}'", getThing().getStatus());
-            return;
-        }
-
-        ThingStatus status = getThing().getStatus();
-        if (status != ThingStatus.ONLINE && status != ThingStatus.UNKNOWN) {
-            logger.debug("Unable to refresh. Thing status is {}", status);
             return;
         }
 
@@ -105,9 +100,7 @@ public class DwdUnwetterHandler extends BaseThingHandler {
             return;
         }
 
-        if (status == ThingStatus.UNKNOWN) {
+        updateStatus(ThingStatus.ONLINE);
-            updateStatus(ThingStatus.ONLINE);
-        }
 
         updateState(getChannelUuid(CHANNEL_LAST_UPDATED), new DateTimeType());
 
@@ -142,18 +135,36 @@ public class DwdUnwetterHandler extends BaseThingHandler {
     @Override
     public void initialize() {
         logger.debug("Start initializing!");
-        initializing = true;
         updateStatus(ThingStatus.UNKNOWN);
 
         DwdUnwetterConfiguration config = getConfigAs(DwdUnwetterConfiguration.class);
-        warningCount = config.warningCount;
+        int newWarningCount = config.warningCount;
+
+        if (warningCount != newWarningCount) {
+            List<Channel> toBeAddedChannels = new ArrayList<>();
+            List<Channel> toBeRemovedChannels = new ArrayList<>();
+            if (warningCount > newWarningCount) {
+                for (int i = newWarningCount + 1; i <= warningCount; ++i) {
+                    toBeRemovedChannels.addAll(removeChannels(i));
+                }
+            } else {
+                for (int i = warningCount + 1; i <= newWarningCount; ++i) {
+                    toBeAddedChannels.addAll(createChannels(i));
+                }
+            }
+            warningCount = newWarningCount;
+
+            ThingBuilder builder = editThing().withoutChannels(toBeRemovedChannels);
+            for (Channel channel : toBeAddedChannels) {
+                builder.withChannel(channel);
+            }
+            updateThing(builder.build());
+        }
 
         data = new DwdWarningsData(config.cellId);
 
-        updateThing(editThing().withChannels(createChannels()).build());
-
         refreshJob = scheduler.scheduleWithFixedDelay(this::refresh, 0, config.refresh, TimeUnit.MINUTES);
-        initializing = false;
+
         logger.debug("Finished initializing!");
     }
 
@@ -165,70 +176,69 @@ public class DwdUnwetterHandler extends BaseThingHandler {
         return new ChannelUID(getThing().getUID(), typeId);
     }
 
-    /**
-     * Creates a trigger Channel.
-     */
-    private Channel createTriggerChannel(String typeId, String label, int warningNumber) {
-        ChannelUID channelUID = getChannelUuid(typeId, warningNumber);
-        return ChannelBuilder.create(channelUID, "String") //
-                .withType(new ChannelTypeUID(BINDING_ID, typeId)) //
-                .withLabel(label + " (" + (warningNumber + 1) + ")")//
-                .withKind(ChannelKind.TRIGGER) //
-                .build();
-    }
-
     /**
      * Creates a normal, state based, channel associated with a warning.
      */
-    private Channel createChannel(String typeId, String itemType, String label, int warningNumber) {
+    private void createChannelIfNotExist(ThingHandlerCallback cb, List<Channel> channels, String typeId, String label,
+            int warningNumber) {
         ChannelUID channelUID = getChannelUuid(typeId, warningNumber);
-        return ChannelBuilder.create(channelUID, itemType) //
+        Channel existingChannel = getThing().getChannel(channelUID);
-                .withType(new ChannelTypeUID(BINDING_ID, typeId)) //
+        if (existingChannel != null) {
-                .withLabel(label + " (" + (warningNumber + 1) + ")")//
+            logger.trace("Thing '{}' already has an existing channel '{}'. Omit adding new channel '{}'.",
-                .build();
+                    getThing().getUID(), existingChannel.getUID(), channelUID);
+        } else {
+            channels.add(cb.createChannelBuilder(channelUID, new ChannelTypeUID(BINDING_ID, typeId))
+                    .withLabel(label + " " + getChannelLabelSuffix(warningNumber)).build());
+        }
+    }
+
+    private String getChannelLabelSuffix(int warningNumber) {
+        return "(" + (warningNumber + 1) + ")";
     }
 
     /**
-     * Creates a normal, state based, channel not associated with a warning.
+     * Creates the Channels for each warning.
-     */
-    private Channel createChannel(String typeId, String itemType, String label) {
-        ChannelUID channelUID = getChannelUuid(typeId);
-        return ChannelBuilder.create(channelUID, itemType) //
-                .withType(new ChannelTypeUID(BINDING_ID, typeId)) //
-                .withLabel(label)//
-                .build();
-    }
-
-    /**
-     * Creates the ChannelsT for each warning.
      *
-     * @return The List of Channels
+     * @return The List of Channels to be added
      */
-    private List<Channel> createChannels() {
+    private List<Channel> createChannels(int warningNumber) {
-        List<Channel> channels = new ArrayList<>(warningCount * 11 + 1);
+        logger.debug("Building channels for thing '{}'.", getThing().getUID());
-        channels.add(createChannel(CHANNEL_LAST_UPDATED, "DateTime", "Last Updated"));
+        List<Channel> channels = new ArrayList<>();
-        for (int i = 0; i < warningCount; i++) {
+        ThingHandlerCallback callback = getCallback();
-            channels.add(createChannel(CHANNEL_WARNING, "Switch", "Warning", i));
+        if (callback != null) {
-            channels.add(createTriggerChannel(CHANNEL_UPDATED, "Updated", i));
+            createChannelIfNotExist(callback, channels, CHANNEL_UPDATED, "Updated", warningNumber);
-            channels.add(createChannel(CHANNEL_SEVERITY, "String", "Severity", i));
+            createChannelIfNotExist(callback, channels, CHANNEL_WARNING, "Warning", warningNumber);
-            channels.add(createChannel(CHANNEL_DESCRIPTION, "String", "Description", i));
+            createChannelIfNotExist(callback, channels, CHANNEL_SEVERITY, "Severity", warningNumber);
-            channels.add(createChannel(CHANNEL_EFFECTIVE, "DateTime", "Issued", i));
+            createChannelIfNotExist(callback, channels, CHANNEL_DESCRIPTION, "Description", warningNumber);
-            channels.add(createChannel(CHANNEL_ONSET, "DateTime", "Valid From", i));
+            createChannelIfNotExist(callback, channels, CHANNEL_EFFECTIVE, "Issued", warningNumber);
-            channels.add(createChannel(CHANNEL_EXPIRES, "DateTime", "Valid To", i));
+            createChannelIfNotExist(callback, channels, CHANNEL_ONSET, "Valid From", warningNumber);
-            channels.add(createChannel(CHANNEL_EVENT, "String", "Type", i));
+            createChannelIfNotExist(callback, channels, CHANNEL_EXPIRES, "Valid To", warningNumber);
-            channels.add(createChannel(CHANNEL_HEADLINE, "String", "Headline", i));
+            createChannelIfNotExist(callback, channels, CHANNEL_EVENT, "Type", warningNumber);
-            channels.add(createChannel(CHANNEL_ALTITUDE, "Number:Length", "Height (from)", i));
+            createChannelIfNotExist(callback, channels, CHANNEL_HEADLINE, "Headline", warningNumber);
-            channels.add(createChannel(CHANNEL_CEILING, "Number:Length", "Height (to)", i));
+            createChannelIfNotExist(callback, channels, CHANNEL_ALTITUDE, "Height (from)", warningNumber);
-            channels.add(createChannel(CHANNEL_INSTRUCTION, "String", "Instruction", i));
+            createChannelIfNotExist(callback, channels, CHANNEL_CEILING, "Height (to)", warningNumber);
-            channels.add(createChannel(CHANNEL_URGENCY, "String", "Urgency", i));
+            createChannelIfNotExist(callback, channels, CHANNEL_INSTRUCTION, "Instruction", warningNumber);
+            createChannelIfNotExist(callback, channels, CHANNEL_URGENCY, "Urgency", warningNumber);
         }
         return channels;
     }
 
+    /**
+     * Filters the Channels for each warning
+     *
+     * @return The List of Channels to be removed
+     */
+    @SuppressWarnings("null")
+    private List<Channel> removeChannels(int warningNumber) {
+        return getThing().getChannels().stream()
+                .filter(channel -> channel.getLabel() != null
+                        && channel.getLabel().endsWith(getChannelLabelSuffix(warningNumber)))
+                .collect(Collectors.toList());
+    }
+
     @Override
     public void dispose() {
         final ScheduledFuture<?> job = refreshJob;
-
         if (job != null) {
             job.cancel(true);
         }

bundles/org.openhab.binding.dwdunwetter/src/main/resources/OH-INF/thing/thing-types.xml

@@ -7,6 +7,9 @@
 	<thing-type id="dwdwarnings">
 		<label>Weather Warnings</label>
 		<description>Weather Warnings for an area</description>
+		<channels>
+			<channel typeId="lastUpdated" id="lastUpdated"></channel>
+		</channels>
 		<config-description>
 			<parameter name="cellId" type="text" required="true">
 				<label>Cell-ID</label>

bundles/org.openhab.binding.dwdunwetter/src/test/java/org/openhab/binding/dwdunwetter/internal/handler/DwdUnwetterHandlerTest.java

@@ -10,11 +10,11 @@
  *
  * SPDX-License-Identifier: EPL-2.0
  */
-package org.openhab.binding.dwdunwetter;
+package org.openhab.binding.dwdunwetter.internal.handler;
 
 import static org.hamcrest.CoreMatchers.is;
 import static org.hamcrest.MatcherAssert.assertThat;
-import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.ArgumentMatchers.*;
 import static org.mockito.Mockito.*;
 
 import java.io.InputStream;
@@ -34,16 +34,17 @@ import org.mockito.junit.jupiter.MockitoExtension;
 import org.mockito.junit.jupiter.MockitoSettings;
 import org.mockito.quality.Strictness;
 import org.openhab.binding.dwdunwetter.internal.DwdUnwetterBindingConstants;
-import org.openhab.binding.dwdunwetter.internal.handler.DwdUnwetterHandler;
 import org.openhab.core.config.core.Configuration;
 import org.openhab.core.test.java.JavaTest;
 import org.openhab.core.thing.Channel;
+import org.openhab.core.thing.ChannelUID;
 import org.openhab.core.thing.Thing;
 import org.openhab.core.thing.ThingStatus;
 import org.openhab.core.thing.ThingStatusInfo;
 import org.openhab.core.thing.ThingUID;
 import org.openhab.core.thing.binding.ThingHandler;
 import org.openhab.core.thing.binding.ThingHandlerCallback;
+import org.openhab.core.thing.binding.builder.ChannelBuilder;
 import org.openhab.core.thing.type.ChannelTypeUID;
 import org.w3c.dom.Document;
 import org.w3c.dom.Node;
@@ -65,6 +66,10 @@ public class DwdUnwetterHandlerTest extends JavaTest {
 
     @BeforeEach
     public void setUp() {
+        when(callback.createChannelBuilder(any(ChannelUID.class), any(ChannelTypeUID.class)))
+                .thenAnswer(invocation -> ChannelBuilder.create(invocation.getArgument(0, ChannelUID.class))
+                        .withType(invocation.getArgument(1, ChannelTypeUID.class)));
+
         handler = new DwdUnwetterHandler(thing);
         handler.setCallback(callback);
         // mock getConfiguration to prevent NPEs