From 0dce37ae1eca74f0797f7c08b67ed9e2ece6f441 Mon Sep 17 00:00:00 2001
From: Jeremy <jsetton@users.noreply.github.com>
Date: Sat, 4 Jan 2025 18:44:31 -0500
Subject: [PATCH] [insteon] Fix x10 device message handling (#18031)

Signed-off-by: Jeremy Setton <jeremy.setton@gmail.com>
---
 .../insteon/internal/device/BaseDevice.java   | 14 ++++----------
 .../internal/device/InsteonDevice.java        | 19 +++++++++++++++++++
 .../insteon/internal/device/InsteonModem.java | 14 +++++---------
 .../insteon/internal/device/X10Address.java   |  4 ++--
 .../device/feature/MessageDispatcher.java     | 12 +++++++-----
 5 files changed, 37 insertions(+), 26 deletions(-)

diff --git a/bundles/org.openhab.binding.insteon/src/main/java/org/openhab/binding/insteon/internal/device/BaseDevice.java b/bundles/org.openhab.binding.insteon/src/main/java/org/openhab/binding/insteon/internal/device/BaseDevice.java
index 2f7af2bed32..6f68c0ffab7 100644
--- a/bundles/org.openhab.binding.insteon/src/main/java/org/openhab/binding/insteon/internal/device/BaseDevice.java
+++ b/bundles/org.openhab.binding.insteon/src/main/java/org/openhab/binding/insteon/internal/device/BaseDevice.java
@@ -564,16 +564,10 @@ public abstract class BaseDevice<@NonNull T extends DeviceAddress, @NonNull S ex
     public void requestReplied(Msg msg) {
         DeviceFeature feature = getFeatureQueried();
         if (feature != null && feature.isMyReply(msg)) {
-            if (msg.isReplyAck()) {
-                // mark feature queried as acked
-                feature.setQueryStatus(QueryStatus.QUERY_ACKED);
-            } else {
-                logger.debug("got a reply nack msg: {}", msg);
-                // mark feature queried as processed and answered
-                setFeatureQueried(null);
-                feature.setQueryMessage(null);
-                feature.setQueryStatus(QueryStatus.QUERY_ANSWERED);
-            }
+            // mark feature queried as processed and answered
+            setFeatureQueried(null);
+            feature.setQueryMessage(null);
+            feature.setQueryStatus(QueryStatus.QUERY_ANSWERED);
         }
     }
 
diff --git a/bundles/org.openhab.binding.insteon/src/main/java/org/openhab/binding/insteon/internal/device/InsteonDevice.java b/bundles/org.openhab.binding.insteon/src/main/java/org/openhab/binding/insteon/internal/device/InsteonDevice.java
index 0364ad979b5..5c2185f8e31 100644
--- a/bundles/org.openhab.binding.insteon/src/main/java/org/openhab/binding/insteon/internal/device/InsteonDevice.java
+++ b/bundles/org.openhab.binding.insteon/src/main/java/org/openhab/binding/insteon/internal/device/InsteonDevice.java
@@ -906,6 +906,25 @@ public class InsteonDevice extends BaseDevice<InsteonAddress, InsteonDeviceHandl
         }
     }
 
+    /**
+     * Notifies that a message request was replied for this device
+     *
+     * @param msg the message received
+     */
+    @Override
+    public void requestReplied(Msg msg) {
+        DeviceFeature feature = getFeatureQueried();
+        if (feature != null && feature.isMyReply(msg)) {
+            if (msg.isReplyAck()) {
+                // mark feature queried as acked
+                feature.setQueryStatus(QueryStatus.QUERY_ACKED);
+            } else {
+                logger.debug("got a reply nack msg: {}", msg);
+                super.requestReplied(msg);
+            }
+        }
+    }
+
     /**
      * Notifies that the link db has been updated for this device
      */
diff --git a/bundles/org.openhab.binding.insteon/src/main/java/org/openhab/binding/insteon/internal/device/InsteonModem.java b/bundles/org.openhab.binding.insteon/src/main/java/org/openhab/binding/insteon/internal/device/InsteonModem.java
index d3328d8cd8e..805ba2fb3e7 100644
--- a/bundles/org.openhab.binding.insteon/src/main/java/org/openhab/binding/insteon/internal/device/InsteonModem.java
+++ b/bundles/org.openhab.binding.insteon/src/main/java/org/openhab/binding/insteon/internal/device/InsteonModem.java
@@ -447,9 +447,6 @@ public class InsteonModem extends BaseDevice<InsteonAddress, InsteonBridgeHandle
             if (address == null) {
                 return;
             }
-            if (msg.isX10()) {
-                lastX10Address = msg.isX10Address() ? (X10Address) address : null;
-            }
             long time = System.currentTimeMillis();
             Device device = getAddress().equals(address) ? this : getDevice(address);
             if (device != null) {
@@ -482,13 +479,12 @@ public class InsteonModem extends BaseDevice<InsteonAddress, InsteonBridgeHandle
     }
 
     private void handleX10Message(Msg msg) throws FieldException {
-        X10Address address = lastX10Address;
-        if (msg.isX10Address()) {
-            // store the x10 address to use with the next cmd
-            lastX10Address = msg.getX10Address();
-        } else if (address != null) {
+        X10Address address = msg.isX10Address() ? msg.getX10Address() : lastX10Address;
+        if (address != null) {
             handleMessage(address, msg);
-            lastX10Address = null;
+            // store the x10 address to use with the next cmd
+            lastX10Address = msg.isX10Address() ? address : null;
+
         }
     }
 
diff --git a/bundles/org.openhab.binding.insteon/src/main/java/org/openhab/binding/insteon/internal/device/X10Address.java b/bundles/org.openhab.binding.insteon/src/main/java/org/openhab/binding/insteon/internal/device/X10Address.java
index b2f46630869..6c95baa387c 100644
--- a/bundles/org.openhab.binding.insteon/src/main/java/org/openhab/binding/insteon/internal/device/X10Address.java
+++ b/bundles/org.openhab.binding.insteon/src/main/java/org/openhab/binding/insteon/internal/device/X10Address.java
@@ -39,7 +39,7 @@ public class X10Address implements DeviceAddress {
     private final byte unitCode;
 
     public X10Address(byte address) {
-        this.houseCode = (byte) (address >> 4);
+        this.houseCode = (byte) (address >> 4 & 0x0F);
         this.unitCode = (byte) (address & 0x0F);
     }
 
@@ -49,7 +49,7 @@ public class X10Address implements DeviceAddress {
     }
 
     public X10Address(String address) throws IllegalArgumentException {
-        String[] parts = address.replace(".", "").split("");
+        String[] parts = address.replace(".", "").split("", 2);
         if (parts.length != 2) {
             throw new IllegalArgumentException("Invalid X10 address format");
         }
diff --git a/bundles/org.openhab.binding.insteon/src/main/java/org/openhab/binding/insteon/internal/device/feature/MessageDispatcher.java b/bundles/org.openhab.binding.insteon/src/main/java/org/openhab/binding/insteon/internal/device/feature/MessageDispatcher.java
index 38fe5e4fb8c..be760a45a4e 100644
--- a/bundles/org.openhab.binding.insteon/src/main/java/org/openhab/binding/insteon/internal/device/feature/MessageDispatcher.java
+++ b/bundles/org.openhab.binding.insteon/src/main/java/org/openhab/binding/insteon/internal/device/feature/MessageDispatcher.java
@@ -242,11 +242,13 @@ public abstract class MessageDispatcher extends BaseFeatureHandler {
         @Override
         public boolean dispatch(Msg msg) {
             try {
-                byte cmd = msg.getByte("rawX10");
-                MessageHandler handler = feature.getOrDefaultMsgHandler(cmd);
-                logger.debug("{}:{}->{} X10", getX10Device().getAddress(), feature.getName(),
-                        handler.getClass().getSimpleName());
-                handler.handleMessage(cmd, msg);
+                if (msg.isX10Command()) {
+                    byte cmd = (byte) (msg.getByte("rawX10") & 0x0F);
+                    MessageHandler handler = feature.getOrDefaultMsgHandler(cmd);
+                    logger.debug("{}:{}->{} X10", getX10Device().getAddress(), feature.getName(),
+                            handler.getClass().getSimpleName());
+                    handler.handleMessage(cmd, msg);
+                }
             } catch (FieldException e) {
                 logger.warn("error parsing, dropping msg {}", msg);
             }