[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 <stefan@roellin-baumann.ch>

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 {

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<ThingTypeUID> 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);

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<PilightBridgeHandler> {
-    private static final Set<ThingTypeUID> SUPPORTED_THING_TYPES_UIDS = PilightHandlerFactory.SUPPORTED_THING_TYPES_UIDS;
+    private static final Set<ThingTypeUID> 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<Config> configFuture;
     private CompletableFuture<List<Status>> 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> 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<String, Object> 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> status) {
         statusFuture.complete(status);
     }
-
-    @Override
-    public Set<ThingTypeUID> getSupportedThingTypes() {
-        return SUPPORTED_THING_TYPES_UIDS;
-    }
 }

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) {

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());

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;