[govee] Fix NullPointerException in discovery for non-scan and malformed packets (#20984)

* [govee] Fix NullPointerException in discovery for non-scan and malformed packets

Signed-off-by: Edoardo Barbano <edo89b@gmail.com>

bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/GoveeDiscoveryService.java

@@ -25,6 +25,7 @@ import org.eclipse.jdt.annotation.NonNullByDefault;
 import org.eclipse.jdt.annotation.Nullable;
 import org.openhab.binding.govee.internal.CommunicationManager.GoveeDiscoveryListener;
 import org.openhab.binding.govee.internal.model.DiscoveryData;
+import org.openhab.binding.govee.internal.model.DiscoveryMsg;
 import org.openhab.binding.govee.internal.model.DiscoveryResponse;
 import org.openhab.core.config.discovery.AbstractDiscoveryService;
 import org.openhab.core.config.discovery.DiscoveryResult;
@@ -115,22 +116,28 @@ public class GoveeDiscoveryService extends AbstractDiscoveryService implements G
     }
 
     public @Nullable DiscoveryResult responseToResult(DiscoveryResponse response) {
-        final DiscoveryData data = response.msg().data();
+        final DiscoveryMsg msg = response.msg();
+        if (!"scan".equals(msg.cmd())) {
+            logger.trace("Ignoring non-scan message received during discovery - {}", response);
+            return null;
+        }
+
+        final DiscoveryData data = msg.data();
         final String macAddress = data.device();
-        if (macAddress.isEmpty()) {
+        if (macAddress == null || macAddress.isEmpty()) {
-            logger.warn("Empty Mac address received during discovery - ignoring {}", response);
+            logger.warn("Missing Mac address received during discovery - ignoring {}", response);
             return null;
         }
 
         final String ipAddress = data.ip();
-        if (ipAddress.isEmpty()) {
+        if (ipAddress == null || ipAddress.isEmpty()) {
-            logger.warn("Empty IP address received during discovery - ignoring {}", response);
+            logger.warn("Missing IP address received during discovery - ignoring {}", response);
             return null;
         }
 
         final String sku = data.sku();
-        if (sku.isEmpty()) {
+        if (sku == null || sku.isEmpty()) {
-            logger.warn("Empty SKU (product name) received during discovery - ignoring {}", response);
+            logger.warn("Missing SKU (product name) received during discovery - ignoring {}", response);
             return null;
         }
 
@@ -157,11 +164,11 @@ public class GoveeDiscoveryService extends AbstractDiscoveryService implements G
         }
 
         String hwVersion = data.wifiVersionHard();
-        if (!hwVersion.isEmpty()) {
+        if (hwVersion != null && !hwVersion.isEmpty()) {
             builder.withProperty(GoveeBindingConstants.HW_VERSION, hwVersion);
         }
         String swVersion = data.wifiVersionSoft();
-        if (!swVersion.isEmpty()) {
+        if (swVersion != null && !swVersion.isEmpty()) {
             builder.withProperty(GoveeBindingConstants.SW_VERSION, swVersion);
         }
 

bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/model/DiscoveryData.java

@@ -13,6 +13,7 @@
 package org.openhab.binding.govee.internal.model;
 
 import org.eclipse.jdt.annotation.NonNullByDefault;
+import org.eclipse.jdt.annotation.Nullable;
 
 /**
  * Govee Message - Device information
@@ -28,8 +29,9 @@ import org.eclipse.jdt.annotation.NonNullByDefault;
  * @author Stefan Höhn - Initial contribution
  */
 @NonNullByDefault
-public record DiscoveryData(String ip, String device, String sku, String bleVersionHard, String bleVersionSoft,
+public record DiscoveryData(@Nullable String ip, @Nullable String device, @Nullable String sku,
-        String wifiVersionHard, String wifiVersionSoft) {
+        @Nullable String bleVersionHard, @Nullable String bleVersionSoft, @Nullable String wifiVersionHard,
+        @Nullable String wifiVersionSoft) {
     public DiscoveryData() {
         this("", "", "", "", "", "", "");
     }

bundles/org.openhab.binding.govee/src/test/java/org/openhab/binding/govee/internal/GoveeDiscoveryTest.java

@@ -48,6 +48,35 @@ public class GoveeDiscoveryTest {
             }
              """;
 
+    // A status update is received on the same port as discovery responses. Its "data" object does not
+    // contain the device information expected by discovery, so Gson leaves those fields null.
+    String statusResponse = """
+             {
+                "msg":{
+                   "cmd":"devStatus",
+                   "data":{
+                      "onOff":1,
+                      "brightness":100,
+                      "color":{ "r":255, "g":255, "b":255 },
+                      "colorTemInKelvin":7200
+                   }
+                }
+            }
+             """;
+
+    // A malformed scan response that is missing the "ip" field.
+    String scanResponseWithoutIp = """
+             {
+                "msg":{
+                   "cmd":"scan",
+                   "data":{
+                      "device":"7D:31:C3:35:33:33:44:15",
+                      "sku":"H6076"
+                   }
+                }
+            }
+             """;
+
     @Test
     public void testProcessScanMessage() {
         GoveeDiscoveryService service = new GoveeDiscoveryService(new CommunicationManager());
@@ -61,4 +90,22 @@ public class GoveeDiscoveryTest {
         assertEquals(deviceProperties.get(GoveeBindingConstants.IP_ADDRESS), "192.168.178.171");
         assertEquals(deviceProperties.get(GoveeBindingConstants.MAC_ADDRESS), "7D:31:C3:35:33:33:44:15");
     }
+
+    @Test
+    public void testStatusMessageIsIgnored() {
+        GoveeDiscoveryService service = new GoveeDiscoveryService(new CommunicationManager());
+        DiscoveryResponse resp = new Gson().fromJson(statusResponse, DiscoveryResponse.class);
+        Objects.requireNonNull(resp);
+        // Must not throw and must not be discovered as a Thing.
+        assertNull(service.responseToResult(resp));
+    }
+
+    @Test
+    public void testScanMessageWithoutIpIsIgnored() {
+        GoveeDiscoveryService service = new GoveeDiscoveryService(new CommunicationManager());
+        DiscoveryResponse resp = new Gson().fromJson(scanResponseWithoutIp, DiscoveryResponse.class);
+        Objects.requireNonNull(resp);
+        // Must not throw a NullPointerException when the IP field is absent.
+        assertNull(service.responseToResult(resp));
+    }
 }