From af4371844dda47f8775fe88b44d3be81454dde51 Mon Sep 17 00:00:00 2001 From: Christoph Weitkamp Date: Tue, 8 Dec 2020 05:53:29 +0100 Subject: [PATCH] [dwdunwetter] Rework channel creation (#9229) Signed-off-by: Christoph Weitkamp --- .../config/DwdUnwetterConfiguration.java | 2 +- .../internal/handler/DwdUnwetterHandler.java | 144 ++++++++++-------- .../resources/OH-INF/thing/thing-types.xml | 3 + .../handler}/DwdUnwetterHandlerTest.java | 11 +- 4 files changed, 89 insertions(+), 71 deletions(-) rename bundles/org.openhab.binding.dwdunwetter/src/test/java/org/openhab/binding/dwdunwetter/{ => internal/handler}/DwdUnwetterHandlerTest.java (91%) diff --git a/bundles/org.openhab.binding.dwdunwetter/src/main/java/org/openhab/binding/dwdunwetter/internal/config/DwdUnwetterConfiguration.java b/bundles/org.openhab.binding.dwdunwetter/src/main/java/org/openhab/binding/dwdunwetter/internal/config/DwdUnwetterConfiguration.java index 83535537c5f..a7bbde15646 100644 --- a/bundles/org.openhab.binding.dwdunwetter/src/main/java/org/openhab/binding/dwdunwetter/internal/config/DwdUnwetterConfiguration.java +++ b/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 = ""; } diff --git a/bundles/org.openhab.binding.dwdunwetter/src/main/java/org/openhab/binding/dwdunwetter/internal/handler/DwdUnwetterHandler.java b/bundles/org.openhab.binding.dwdunwetter/src/main/java/org/openhab/binding/dwdunwetter/internal/handler/DwdUnwetterHandler.java index 7ae12b2f783..d271fc8c38e 100644 --- a/bundles/org.openhab.binding.dwdunwetter/src/main/java/org/openhab/binding/dwdunwetter/internal/handler/DwdUnwetterHandler.java +++ b/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.type.ChannelKind; +import org.openhab.core.thing.binding.ThingHandlerCallback; +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) { - logger.trace("Still initializing. Ignoring refresh request."); - return; - } - - ThingStatus status = getThing().getStatus(); - if (status != ThingStatus.ONLINE && status != ThingStatus.UNKNOWN) { - logger.debug("Unable to refresh. Thing status is {}", status); + if (!ThingHandlerHelper.isHandlerInitialized(getThing())) { + logger.debug("Unable to refresh. Thing status is '{}'", getThing().getStatus()); 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 toBeAddedChannels = new ArrayList<>(); + List 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 channels, String typeId, String label, + int warningNumber) { ChannelUID channelUID = getChannelUuid(typeId, warningNumber); - return ChannelBuilder.create(channelUID, itemType) // - .withType(new ChannelTypeUID(BINDING_ID, typeId)) // - .withLabel(label + " (" + (warningNumber + 1) + ")")// - .build(); + Channel existingChannel = getThing().getChannel(channelUID); + if (existingChannel != null) { + logger.trace("Thing '{}' already has an existing channel '{}'. Omit adding new channel '{}'.", + 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. - */ - 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. + * Creates the Channels for each warning. * - * @return The List of Channels + * @return The List of Channels to be added */ - private List createChannels() { - List channels = new ArrayList<>(warningCount * 11 + 1); - channels.add(createChannel(CHANNEL_LAST_UPDATED, "DateTime", "Last Updated")); - for (int i = 0; i < warningCount; i++) { - channels.add(createChannel(CHANNEL_WARNING, "Switch", "Warning", i)); - channels.add(createTriggerChannel(CHANNEL_UPDATED, "Updated", i)); - channels.add(createChannel(CHANNEL_SEVERITY, "String", "Severity", i)); - channels.add(createChannel(CHANNEL_DESCRIPTION, "String", "Description", i)); - channels.add(createChannel(CHANNEL_EFFECTIVE, "DateTime", "Issued", i)); - channels.add(createChannel(CHANNEL_ONSET, "DateTime", "Valid From", i)); - channels.add(createChannel(CHANNEL_EXPIRES, "DateTime", "Valid To", i)); - channels.add(createChannel(CHANNEL_EVENT, "String", "Type", i)); - channels.add(createChannel(CHANNEL_HEADLINE, "String", "Headline", i)); - channels.add(createChannel(CHANNEL_ALTITUDE, "Number:Length", "Height (from)", i)); - channels.add(createChannel(CHANNEL_CEILING, "Number:Length", "Height (to)", i)); - channels.add(createChannel(CHANNEL_INSTRUCTION, "String", "Instruction", i)); - channels.add(createChannel(CHANNEL_URGENCY, "String", "Urgency", i)); + private List createChannels(int warningNumber) { + logger.debug("Building channels for thing '{}'.", getThing().getUID()); + List channels = new ArrayList<>(); + ThingHandlerCallback callback = getCallback(); + if (callback != null) { + createChannelIfNotExist(callback, channels, CHANNEL_UPDATED, "Updated", warningNumber); + createChannelIfNotExist(callback, channels, CHANNEL_WARNING, "Warning", warningNumber); + createChannelIfNotExist(callback, channels, CHANNEL_SEVERITY, "Severity", warningNumber); + createChannelIfNotExist(callback, channels, CHANNEL_DESCRIPTION, "Description", warningNumber); + createChannelIfNotExist(callback, channels, CHANNEL_EFFECTIVE, "Issued", warningNumber); + createChannelIfNotExist(callback, channels, CHANNEL_ONSET, "Valid From", warningNumber); + createChannelIfNotExist(callback, channels, CHANNEL_EXPIRES, "Valid To", warningNumber); + createChannelIfNotExist(callback, channels, CHANNEL_EVENT, "Type", warningNumber); + createChannelIfNotExist(callback, channels, CHANNEL_HEADLINE, "Headline", warningNumber); + createChannelIfNotExist(callback, channels, CHANNEL_ALTITUDE, "Height (from)", warningNumber); + createChannelIfNotExist(callback, channels, CHANNEL_CEILING, "Height (to)", warningNumber); + 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 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); } diff --git a/bundles/org.openhab.binding.dwdunwetter/src/main/resources/OH-INF/thing/thing-types.xml b/bundles/org.openhab.binding.dwdunwetter/src/main/resources/OH-INF/thing/thing-types.xml index 714e92133f6..5a25f8beef9 100644 --- a/bundles/org.openhab.binding.dwdunwetter/src/main/resources/OH-INF/thing/thing-types.xml +++ b/bundles/org.openhab.binding.dwdunwetter/src/main/resources/OH-INF/thing/thing-types.xml @@ -7,6 +7,9 @@ Weather Warnings for an area + + + diff --git a/bundles/org.openhab.binding.dwdunwetter/src/test/java/org/openhab/binding/dwdunwetter/DwdUnwetterHandlerTest.java b/bundles/org.openhab.binding.dwdunwetter/src/test/java/org/openhab/binding/dwdunwetter/internal/handler/DwdUnwetterHandlerTest.java similarity index 91% rename from bundles/org.openhab.binding.dwdunwetter/src/test/java/org/openhab/binding/dwdunwetter/DwdUnwetterHandlerTest.java rename to bundles/org.openhab.binding.dwdunwetter/src/test/java/org/openhab/binding/dwdunwetter/internal/handler/DwdUnwetterHandlerTest.java index 7673057cc51..76afca30739 100644 --- a/bundles/org.openhab.binding.dwdunwetter/src/test/java/org/openhab/binding/dwdunwetter/DwdUnwetterHandlerTest.java +++ b/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