Refactor and fix (#17394)

Signed-off-by: Leo Siepel <[email protected]>
openhab · Sep 18, 2024 · 8d552bf · 8d552bf
1 parent 5a3a0c0
commit 8d552bf
Show file tree

Hide file tree

Showing 6 changed files with 36 additions and 22 deletions.
diff --git a/...c/main/java/org/openhab/binding/smaenergymeter/internal/SMAEnergyMeterHandlerFactory.java b/...c/main/java/org/openhab/binding/smaenergymeter/internal/SMAEnergyMeterHandlerFactory.java
@@ -14,6 +14,8 @@
 
 import static org.openhab.binding.smaenergymeter.internal.SMAEnergyMeterBindingConstants.*;
 
+import org.eclipse.jdt.annotation.NonNullByDefault;
+import org.eclipse.jdt.annotation.Nullable;
 import org.openhab.binding.smaenergymeter.internal.handler.SMAEnergyMeterHandler;
 import org.openhab.binding.smaenergymeter.internal.packet.PacketListenerRegistry;
 import org.openhab.core.thing.Thing;
@@ -31,6 +33,7 @@
  *
  * @author Osman Basha - Initial contribution
  */
+@NonNullByDefault
 @Component(service = ThingHandlerFactory.class, configurationPid = "binding.smaenergymeter")
 public class SMAEnergyMeterHandlerFactory extends BaseThingHandlerFactory {
 
@@ -47,7 +50,7 @@ public boolean supportsThingType(ThingTypeUID thingTypeUID) {
     }
 
     @Override
-    protected ThingHandler createHandler(Thing thing) {
+    protected @Nullable ThingHandler createHandler(Thing thing) {
         ThingTypeUID thingTypeUID = thing.getThingTypeUID();
 
         if (thingTypeUID.equals(THING_TYPE_ENERGY_METER)) {

diff --git a/...org/openhab/binding/smaenergymeter/internal/discovery/SMAEnergyMeterDiscoveryService.java b/...org/openhab/binding/smaenergymeter/internal/discovery/SMAEnergyMeterDiscoveryService.java
@@ -79,7 +79,12 @@ protected void startBackgroundDiscovery() {
             return;
         }
 
-        packetListener.addPayloadHandler(this);
+        try {
+            packetListener.addPayloadHandler(this);
+        } catch (IOException e) {
+            logger.warn("Could not start background discovery, unable to add PayloadHandler", e);
+            return;
+        }
         this.packetListener = packetListener;
     }
 

diff --git a/...ymeter/src/main/java/org/openhab/binding/smaenergymeter/internal/handler/EnergyMeter.java b/...ymeter/src/main/java/org/openhab/binding/smaenergymeter/internal/handler/EnergyMeter.java
@@ -16,6 +16,7 @@
 import java.nio.ByteBuffer;
 import java.util.Arrays;
 
+import org.eclipse.jdt.annotation.NonNullByDefault;
 import org.openhab.core.library.types.DecimalType;
 
 /**
@@ -26,11 +27,12 @@
  * @author Łukasz Dywicki - Extracted multicast group handling to
  *         {@link org.openhab.binding.smaenergymeter.internal.packet.PacketListener}.
  */
+@NonNullByDefault
 public class EnergyMeter {
 
     private static final byte[] E_METER_PROTOCOL_ID = new byte[] { 0x60, 0x69 };
 
-    private String serialNumber;
+    private String serialNumber = "";
     private final FieldDTO powerIn;
     private final FieldDTO energyIn;
     private final FieldDTO powerOut;

diff --git a/.../main/java/org/openhab/binding/smaenergymeter/internal/handler/SMAEnergyMeterHandler.java b/.../main/java/org/openhab/binding/smaenergymeter/internal/handler/SMAEnergyMeterHandler.java
@@ -15,8 +15,10 @@
 import static org.openhab.binding.smaenergymeter.internal.SMAEnergyMeterBindingConstants.*;
 
 import java.io.IOException;
+import java.util.Objects;
 import java.util.concurrent.TimeUnit;
 
+import org.eclipse.jdt.annotation.NonNullByDefault;
 import org.eclipse.jdt.annotation.Nullable;
 import org.openhab.binding.smaenergymeter.internal.configuration.EnergyMeterConfig;
 import org.openhab.binding.smaenergymeter.internal.packet.FilteringPayloadHandler;
@@ -40,13 +42,14 @@
  *
  * @author Osman Basha - Initial contribution
  */
+@NonNullByDefault
 public class SMAEnergyMeterHandler extends BaseThingHandler implements PayloadHandler {
 
     private final Logger logger = LoggerFactory.getLogger(SMAEnergyMeterHandler.class);
     private final PacketListenerRegistry listenerRegistry;
     private @Nullable PacketListener listener;
     private @Nullable PayloadHandler handler;
-    private String serialNumber;
+    private String serialNumber = "";
 
     public SMAEnergyMeterHandler(Thing thing, PacketListenerRegistry listenerRegistry) {
         super(thing);
@@ -73,8 +76,8 @@ public void initialize() {
         EnergyMeterConfig config = getConfigAs(EnergyMeterConfig.class);
 
         try {
-            serialNumber = config.getSerialNumber();
-            if (serialNumber == null) {
+            serialNumber = Objects.requireNonNullElse(config.getSerialNumber(), "");
+            if (serialNumber.isBlank()) {
                 updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.CONFIGURATION_PENDING,
                         "Meter serial number missing");
                 return;

diff --git a/...ava/org/openhab/binding/smaenergymeter/internal/packet/DefaultPacketListenerRegistry.java b/...ava/org/openhab/binding/smaenergymeter/internal/packet/DefaultPacketListenerRegistry.java
@@ -64,7 +64,9 @@ protected void shutdown() throws IOException {
                 logger.warn("Multicast socket {} failed to terminate", entry.getKey(), e);
             }
         }
-        scheduler.shutdownNow();
+        if (!scheduler.isShutdown()) {
+            scheduler.shutdownNow();
+        }
     }
 
     public ScheduledFuture<?> addTask(ReceivingTask runnable) {

diff --git a/...eter/src/main/java/org/openhab/binding/smaenergymeter/internal/packet/PacketListener.java b/...eter/src/main/java/org/openhab/binding/smaenergymeter/internal/packet/PacketListener.java
@@ -16,6 +16,7 @@
 import java.net.DatagramPacket;
 import java.net.DatagramSocket;
 import java.net.InetAddress;
+import java.net.InetSocketAddress;
 import java.net.MulticastSocket;
 import java.util.List;
 import java.util.concurrent.CopyOnWriteArrayList;
@@ -55,7 +56,7 @@ public PacketListener(DefaultPacketListenerRegistry registry, String multicastGr
         this.port = port;
     }
 
-    public void addPayloadHandler(PayloadHandler handler) {
+    public void addPayloadHandler(PayloadHandler handler) throws IOException {
         if (handlers.isEmpty()) {
             open();
         }
@@ -75,22 +76,19 @@ public boolean isOpen() {
         return socket != null && socket.isConnected();
     }
 
-    private void open() {
+    private void open() throws IOException {
         if (isOpen()) {
             // no need to bind socket second time
             return;
         }
-        try {
-            MulticastSocket socket = new MulticastSocket(port);
-            socket.setSoTimeout(5000);
-            InetAddress address = InetAddress.getByName(multicastGroup);
-            socket.joinGroup(address);
-
-            future = registry.addTask(new ReceivingTask(socket, multicastGroup + ":" + port, handlers));
-            this.socket = socket;
-        } catch (IOException e) {
-            throw new RuntimeException("Could not open socket", e);
-        }
+        MulticastSocket socket = new MulticastSocket(port);
+        socket.setSoTimeout(5000);
+        InetAddress mcastGroupAddress = InetAddress.getByName(multicastGroup);
+        InetSocketAddress socketAddress = new InetSocketAddress(mcastGroupAddress, port);
+        socket.joinGroup(socketAddress, null);
+
+        future = registry.addTask(new ReceivingTask(socket, multicastGroup + ":" + port, handlers));
+        this.socket = socket;
     }
 
     void close() throws IOException {
@@ -100,10 +98,11 @@ void close() throws IOException {
             this.future = null;
         }
 
-        InetAddress address = InetAddress.getByName(multicastGroup);
+        InetAddress mcastGroupAddress = InetAddress.getByName(multicastGroup);
+        InetSocketAddress socketAddress = new InetSocketAddress(mcastGroupAddress, port);
         MulticastSocket socket = this.socket;
         if (socket != null) {
-            socket.leaveGroup(address);
+            socket.leaveGroup(socketAddress, null);
             socket.close();
             this.socket = null;
         }