From 8e34fa4ffd855a0e1fffb22198be0b2f11d20cf7 Mon Sep 17 00:00:00 2001 From: Stefan Roellin Date: Mon, 8 Jan 2024 17:09:18 +0100 Subject: [PATCH] [pilight] Various minor code improvements (#16227) - Fix warnings from static code analysis - Remove superfluous if - Remove redundant member - Introduce constant - Use correct thing type uids in PilightDeviceDiscoveryService - Fix log messages Signed-off-by: Stefan Roellin Signed-off-by: Ciprian Pascu --- .../pilight/internal/PilightConnector.java | 7 ++-- .../PilightBridgeDiscoveryService.java | 11 ++--- .../PilightDeviceDiscoveryService.java | 42 +++++++------------ .../handler/PilightBridgeHandler.java | 4 +- .../handler/PilightGenericHandler.java | 3 -- .../internal/types/PilightContactType.java | 2 + 6 files changed, 26 insertions(+), 43 deletions(-) diff --git a/bundles/org.openhab.binding.pilight/src/main/java/org/openhab/binding/pilight/internal/PilightConnector.java b/bundles/org.openhab.binding.pilight/src/main/java/org/openhab/binding/pilight/internal/PilightConnector.java index aea17050587..5ffc6405665 100644 --- a/bundles/org.openhab.binding.pilight/src/main/java/org/openhab/binding/pilight/internal/PilightConnector.java +++ b/bundles/org.openhab.binding.pilight/src/main/java/org/openhab/binding/pilight/internal/PilightConnector.java @@ -97,8 +97,9 @@ public class PilightConnector implements Runnable, Closeable { try (BufferedReader in = new BufferedReader(new InputStreamReader(socket.getInputStream()))) { String line = in.readLine(); while (!Thread.currentThread().isInterrupted() && line != null) { - if (!line.isEmpty()) { - logger.trace("Received from pilight: {}", line); + logger.trace("Received from pilight: {}", line); + // ignore empty lines and lines starting with "status" + if (!line.isEmpty() && !line.startsWith("{\"status\":")) { final ObjectMapper inputMapper = this.inputMapper; if (line.startsWith("{\"message\":\"config\"")) { final @Nullable Message message = inputMapper.readValue(line, Message.class); @@ -109,8 +110,6 @@ public class PilightConnector implements Runnable, Closeable { } else if (line.startsWith("{\"version\":")) { final @Nullable Version version = inputMapper.readValue(line, Version.class); callback.versionReceived(version); - } else if (line.startsWith("{\"status\":")) { - // currently unused } else if ("1".equals(line)) { throw new IOException("Connection to pilight lost"); } else { diff --git a/bundles/org.openhab.binding.pilight/src/main/java/org/openhab/binding/pilight/internal/discovery/PilightBridgeDiscoveryService.java b/bundles/org.openhab.binding.pilight/src/main/java/org/openhab/binding/pilight/internal/discovery/PilightBridgeDiscoveryService.java index c3e5de04630..0fc141f170c 100644 --- a/bundles/org.openhab.binding.pilight/src/main/java/org/openhab/binding/pilight/internal/discovery/PilightBridgeDiscoveryService.java +++ b/bundles/org.openhab.binding.pilight/src/main/java/org/openhab/binding/pilight/internal/discovery/PilightBridgeDiscoveryService.java @@ -39,7 +39,6 @@ import org.openhab.core.config.discovery.AbstractDiscoveryService; import org.openhab.core.config.discovery.DiscoveryResult; import org.openhab.core.config.discovery.DiscoveryResultBuilder; import org.openhab.core.config.discovery.DiscoveryService; -import org.openhab.core.thing.ThingTypeUID; import org.openhab.core.thing.ThingUID; import org.osgi.service.component.annotations.Component; import org.slf4j.Logger; @@ -76,11 +75,7 @@ public class PilightBridgeDiscoveryService extends AbstractDiscoveryService { private @Nullable ScheduledFuture backgroundDiscoveryJob; public PilightBridgeDiscoveryService() throws IllegalArgumentException { - super(getSupportedThingTypeUIDs(), AUTODISCOVERY_SEARCH_TIME_SEC, true); - } - - public static Set getSupportedThingTypeUIDs() { - return Set.of(PilightBindingConstants.THING_TYPE_BRIDGE); + super(Set.of(PilightBindingConstants.THING_TYPE_BRIDGE), AUTODISCOVERY_SEARCH_TIME_SEC, true); } @Override @@ -152,7 +147,7 @@ public class PilightBridgeDiscoveryService extends AbstractDiscoveryService { @Override protected void startBackgroundDiscovery() { - logger.debug("Start Pilight device background discovery"); + logger.debug("Start Pilight bridge background discovery"); final @Nullable ScheduledFuture backgroundDiscoveryJob = this.backgroundDiscoveryJob; if (backgroundDiscoveryJob == null || backgroundDiscoveryJob.isCancelled()) { this.backgroundDiscoveryJob = scheduler.scheduleWithFixedDelay(this::startScan, 5, @@ -162,7 +157,7 @@ public class PilightBridgeDiscoveryService extends AbstractDiscoveryService { @Override protected void stopBackgroundDiscovery() { - logger.debug("Stop Pilight device background discovery"); + logger.debug("Stop Pilight bridge background discovery"); final @Nullable ScheduledFuture backgroundDiscoveryJob = this.backgroundDiscoveryJob; if (backgroundDiscoveryJob != null) { backgroundDiscoveryJob.cancel(true); diff --git a/bundles/org.openhab.binding.pilight/src/main/java/org/openhab/binding/pilight/internal/discovery/PilightDeviceDiscoveryService.java b/bundles/org.openhab.binding.pilight/src/main/java/org/openhab/binding/pilight/internal/discovery/PilightDeviceDiscoveryService.java index de77d49d9fa..66eda165268 100644 --- a/bundles/org.openhab.binding.pilight/src/main/java/org/openhab/binding/pilight/internal/discovery/PilightDeviceDiscoveryService.java +++ b/bundles/org.openhab.binding.pilight/src/main/java/org/openhab/binding/pilight/internal/discovery/PilightDeviceDiscoveryService.java @@ -14,8 +14,7 @@ package org.openhab.binding.pilight.internal.discovery; import static org.openhab.binding.pilight.internal.PilightBindingConstants.*; -import java.util.Date; -import java.util.HashMap; +import java.time.Instant; import java.util.List; import java.util.Map; import java.util.Optional; @@ -26,7 +25,6 @@ import java.util.concurrent.TimeUnit; import org.eclipse.jdt.annotation.NonNullByDefault; import org.eclipse.jdt.annotation.Nullable; -import org.openhab.binding.pilight.internal.PilightHandlerFactory; import org.openhab.binding.pilight.internal.dto.Config; import org.openhab.binding.pilight.internal.dto.DeviceType; import org.openhab.binding.pilight.internal.dto.Status; @@ -51,15 +49,15 @@ import org.slf4j.LoggerFactory; @Component(scope = ServiceScope.PROTOTYPE, service = PilightDeviceDiscoveryService.class) @NonNullByDefault public class PilightDeviceDiscoveryService extends AbstractThingHandlerDiscoveryService { - private static final Set SUPPORTED_THING_TYPES_UIDS = PilightHandlerFactory.SUPPORTED_THING_TYPES_UIDS; + private static final Set SUPPORTED_THING_TYPES_UIDS = Set.of(THING_TYPE_CONTACT, THING_TYPE_DIMMER, + THING_TYPE_GENERIC, THING_TYPE_SWITCH); private static final int AUTODISCOVERY_SEARCH_TIME_SEC = 10; - private static final int AUTODISCOVERY_BACKGROUND_SEARCH_INTERVAL_SEC = 60 * 10; + private static final int AUTODISCOVERY_BACKGROUND_SEARCH_INITIAL_DELAY_SEC = 20; + private static final int AUTODISCOVERY_BACKGROUND_SEARCH_INTERVAL_SEC = 10 * 60; // 10 minutes private final Logger logger = LoggerFactory.getLogger(PilightDeviceDiscoveryService.class); - private @NonNullByDefault({}) ThingUID bridgeUID; - private @Nullable ScheduledFuture backgroundDiscoveryJob; private CompletableFuture configFuture; private CompletableFuture> statusFuture; @@ -76,7 +74,7 @@ public class PilightDeviceDiscoveryService extends AbstractThingHandlerDiscovery statusFuture = new CompletableFuture<>(); configFuture.thenAcceptBoth(statusFuture, (config, allStatus) -> { - removeOlderResults(getTimestampOfLastScan(), bridgeUID); + removeOlderResults(getTimestampOfLastScan(), thingHandler.getThing().getUID()); config.getDevices().forEach((deviceId, device) -> { final Optional status = allStatus.stream().filter(s -> s.getDevices().contains(deviceId)) .findFirst(); @@ -108,11 +106,9 @@ public class PilightDeviceDiscoveryService extends AbstractThingHandlerDiscovery final ThingUID thingUID = new ThingUID(thingTypeUID, thingHandler.getThing().getUID(), deviceId); - final Map properties = new HashMap<>(); - properties.put(PROPERTY_NAME, deviceId); - DiscoveryResult discoveryResult = DiscoveryResultBuilder.create(thingUID).withThingType(thingTypeUID) - .withProperties(properties).withBridge(bridgeUID).withRepresentationProperty(PROPERTY_NAME) + .withProperties(Map.of(PROPERTY_NAME, deviceId)).withBridge(thingHandler.getThing().getUID()) + .withRepresentationProperty(PROPERTY_NAME) .withLabel("Pilight " + typeString + " Device '" + deviceId + "'").build(); thingDiscovered(discoveryResult); @@ -127,7 +123,7 @@ public class PilightDeviceDiscoveryService extends AbstractThingHandlerDiscovery super.stopScan(); configFuture.cancel(true); statusFuture.cancel(true); - removeOlderResults(getTimestampOfLastScan(), bridgeUID); + removeOlderResults(getTimestampOfLastScan(), thingHandler.getThing().getUID()); } @Override @@ -135,8 +131,9 @@ public class PilightDeviceDiscoveryService extends AbstractThingHandlerDiscovery logger.debug("Start Pilight device background discovery"); final @Nullable ScheduledFuture backgroundDiscoveryJob = this.backgroundDiscoveryJob; if (backgroundDiscoveryJob == null || backgroundDiscoveryJob.isCancelled()) { - this.backgroundDiscoveryJob = scheduler.scheduleWithFixedDelay(this::startScan, 20, - AUTODISCOVERY_BACKGROUND_SEARCH_INTERVAL_SEC, TimeUnit.SECONDS); + this.backgroundDiscoveryJob = scheduler.scheduleWithFixedDelay(this::startScan, + AUTODISCOVERY_BACKGROUND_SEARCH_INITIAL_DELAY_SEC, AUTODISCOVERY_BACKGROUND_SEARCH_INTERVAL_SEC, + TimeUnit.SECONDS); } } @@ -152,20 +149,18 @@ public class PilightDeviceDiscoveryService extends AbstractThingHandlerDiscovery @Override public void initialize() { - bridgeUID = thingHandler.getThing().getUID(); - boolean discoveryEnabled = false; - removeOlderResults(new Date().getTime(), thingHandler.getThing().getUID()); - discoveryEnabled = thingHandler.isBackgroundDiscoveryEnabled(); + removeOlderResults(Instant.now().toEpochMilli(), thingHandler.getThing().getUID()); thingHandler.registerDiscoveryListener(this); super.initialize(); - super.modified(Map.of(DiscoveryService.CONFIG_PROPERTY_BACKGROUND_DISCOVERY, discoveryEnabled)); + super.modified(Map.of(DiscoveryService.CONFIG_PROPERTY_BACKGROUND_DISCOVERY, + thingHandler.isBackgroundDiscoveryEnabled())); } @Override public void dispose() { super.dispose(); - removeOlderResults(getTimestampOfLastScan(), bridgeUID); + removeOlderResults(Instant.now().toEpochMilli(), thingHandler.getThing().getUID()); thingHandler.unregisterDiscoveryListener(); } @@ -186,9 +181,4 @@ public class PilightDeviceDiscoveryService extends AbstractThingHandlerDiscovery public void setStatus(List status) { statusFuture.complete(status); } - - @Override - public Set getSupportedThingTypes() { - return SUPPORTED_THING_TYPES_UIDS; - } } diff --git a/bundles/org.openhab.binding.pilight/src/main/java/org/openhab/binding/pilight/internal/handler/PilightBridgeHandler.java b/bundles/org.openhab.binding.pilight/src/main/java/org/openhab/binding/pilight/internal/handler/PilightBridgeHandler.java index 41aa228d1ba..6ddcaab5ec3 100644 --- a/bundles/org.openhab.binding.pilight/src/main/java/org/openhab/binding/pilight/internal/handler/PilightBridgeHandler.java +++ b/bundles/org.openhab.binding.pilight/src/main/java/org/openhab/binding/pilight/internal/handler/PilightBridgeHandler.java @@ -77,10 +77,10 @@ public class PilightBridgeHandler extends BaseBridgeHandler { @Override public void initialize() { - PilightBridgeConfiguration config = getConfigAs(PilightBridgeConfiguration.class); + PilightBridgeConfiguration pilightConfig = getConfigAs(PilightBridgeConfiguration.class); final @Nullable PilightDeviceDiscoveryService discoveryService = this.discoveryService; - PilightConnector connector = new PilightConnector(config, new IPilightCallback() { + PilightConnector connector = new PilightConnector(pilightConfig, new IPilightCallback() { @Override public void updateThingStatus(ThingStatus status, ThingStatusDetail statusDetail, @Nullable String description) { diff --git a/bundles/org.openhab.binding.pilight/src/main/java/org/openhab/binding/pilight/internal/handler/PilightGenericHandler.java b/bundles/org.openhab.binding.pilight/src/main/java/org/openhab/binding/pilight/internal/handler/PilightGenericHandler.java index 5d758b7699e..c3f4bbbc65c 100644 --- a/bundles/org.openhab.binding.pilight/src/main/java/org/openhab/binding/pilight/internal/handler/PilightGenericHandler.java +++ b/bundles/org.openhab.binding.pilight/src/main/java/org/openhab/binding/pilight/internal/handler/PilightGenericHandler.java @@ -119,9 +119,6 @@ public class PilightGenericHandler extends PilightBaseHandler { if (channelType != null) { logger.debug("initializeReadOnly {} {}", channelType, channelType.getState()); - } - - if (channelType != null) { final @Nullable StateDescription state = channelType.getState(); if (state != null) { channelReadOnlyMap.putIfAbsent(channel.getUID(), state.isReadOnly()); diff --git a/bundles/org.openhab.binding.pilight/src/main/java/org/openhab/binding/pilight/internal/types/PilightContactType.java b/bundles/org.openhab.binding.pilight/src/main/java/org/openhab/binding/pilight/internal/types/PilightContactType.java index 892182d70ad..a2f826df731 100644 --- a/bundles/org.openhab.binding.pilight/src/main/java/org/openhab/binding/pilight/internal/types/PilightContactType.java +++ b/bundles/org.openhab.binding.pilight/src/main/java/org/openhab/binding/pilight/internal/types/PilightContactType.java @@ -12,6 +12,7 @@ */ package org.openhab.binding.pilight.internal.types; +import org.eclipse.jdt.annotation.NonNullByDefault; import org.openhab.core.library.types.OpenClosedType; /** @@ -21,6 +22,7 @@ import org.openhab.core.library.types.OpenClosedType; * @author Stefan Röllin - Port to openHAB 2 pilight binding * @author Niklas Dörfler - Port pilight binding to openHAB 3 + add device discovery */ +@NonNullByDefault public enum PilightContactType { OPENED, CLOSED;