[insteon] improved the handling of duplicate broadcast group messages (…

…#8001) Signed-off-by: Rob Nielsen <[email protected]>
openhab · Jun 24, 2020 · 9c4470f · 9c4470f
1 parent f83ed50
commit 9c4470f
Show file tree

Hide file tree

Showing 3 changed files with 19 additions and 6 deletions.
diff --git a/...n/src/main/java/org/openhab/binding/insteon/internal/device/GroupMessageStateMachine.java b/...n/src/main/java/org/openhab/binding/insteon/internal/device/GroupMessageStateMachine.java
@@ -13,6 +13,7 @@
 package org.openhab.binding.insteon.internal.device;
 
 import org.eclipse.jdt.annotation.NonNullByDefault;
+import org.openhab.binding.insteon.internal.utils.Utils;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -104,16 +105,18 @@ enum State {
     private State state = State.EXPECT_BCAST;
     private long lastUpdated = 0;
     private boolean publish = false;
+    private byte lastCmd1 = 0;
 
     /**
      * Advance the state machine and determine if update is genuine (no duplicate)
      *
      * @param a the group message (action) that was received
      * @param address the address of the device that this state machine belongs to
      * @param group the group that this state machine belongs to
+     * @param cmd1 cmd1 from the message received
      * @return true if the group message is not a duplicate
      */
-    public boolean action(GroupMessage a, InsteonAddress address, int group) {
+    public boolean action(GroupMessage a, InsteonAddress address, int group, byte cmd1) {
         publish = false;
         switch (state) {
             case EXPECT_BCAST:
@@ -132,7 +135,15 @@ public boolean action(GroupMessage a, InsteonAddress address, int group) {
             case EXPECT_CLEAN:
                 switch (a) {
                     case BCAST:
-                        publish = false;
+                        if (lastCmd1 == cmd1) {
+                            publish = false;
+                        } else {
+                            if (logger.isDebugEnabled()) {
+                                logger.debug("{} group {} cmd1 {} is not a dup BCAST, last cmd1 {}", address, group,
+                                        Utils.getHexByte(cmd1), Utils.getHexByte(lastCmd1));
+                            }
+                            publish = true;
+                        }
                         break; // missed(CLEAN, SUCCESS) or dup BCAST
                     case CLEAN:
                         publish = false;
@@ -169,6 +180,7 @@ public boolean action(GroupMessage a, InsteonAddress address, int group) {
                 break;
         }
 
+        lastCmd1 = cmd1;
         lastUpdated = System.currentTimeMillis();
         logger.debug("{} group {} state: {} --{}--> {}, publish: {}", address, group, oldState, a, state, publish);
         return (publish);

diff --git a/...ding.insteon/src/main/java/org/openhab/binding/insteon/internal/device/InsteonDevice.java b/...ding.insteon/src/main/java/org/openhab/binding/insteon/internal/device/InsteonDevice.java
@@ -552,9 +552,10 @@ private void addFeature(String name, DeviceFeature f) {
      *
      * @param group the insteon group of the broadcast message
      * @param a the type of group message came in (action etc)
+     * @param cmd1 cmd1 from the message received
      * @return true if this is message is NOT a duplicate
      */
-    public boolean getGroupState(int group, GroupMessage a) {
+    public boolean getGroupState(int group, GroupMessage a, byte cmd1) {
         GroupMessageStateMachine m = groupState.get(group);
         if (m == null) {
             m = new GroupMessageStateMachine();
@@ -568,7 +569,7 @@ public boolean getGroupState(int group, GroupMessage a) {
         }
 
         logger.trace("{} updating group {} state to {}", address, group, a);
-        return (m.action(a, address, group));
+        return (m.action(a, address, group, cmd1));
     }
 
     @Override

diff --git a/...ing.insteon/src/main/java/org/openhab/binding/insteon/internal/device/MessageHandler.java b/...ing.insteon/src/main/java/org/openhab/binding/insteon/internal/device/MessageHandler.java
@@ -259,12 +259,12 @@ protected boolean isDuplicate(Msg msg) {
                 // from the original broadcaster, with which the device
                 // confirms that it got all cleanup replies successfully.
                 GroupMessage gm = (cmd1 == 0x06) ? GroupMessage.SUCCESS : GroupMessage.BCAST;
-                isDuplicate = !feature.getDevice().getGroupState(group, gm);
+                isDuplicate = !feature.getDevice().getGroupState(group, gm, cmd1);
             } else if (t == MsgType.ALL_LINK_CLEANUP) {
                 // the cleanup messages are direct messages, so the
                 // group # is not in the toAddress, but in cmd2
                 int group = msg.getByte("command2") & 0xff;
-                isDuplicate = !feature.getDevice().getGroupState(group, GroupMessage.CLEAN);
+                isDuplicate = !feature.getDevice().getGroupState(group, GroupMessage.CLEAN, (byte) 0);
             }
         } catch (IllegalArgumentException e) {
             logger.warn("cannot parse msg: {}", msg, e);