[insteon] keep track of group state at the device level (#7929)

Signed-off-by: Rob Nielsen <[email protected]>

bundles/org.openhab.binding.insteon/src/main/java/org/openhab/binding/insteon/internal/device/GroupMessageStateMachine.java

@@ -101,18 +101,20 @@ enum State {
         EXPECT_SUCCESS
     };
 
-    State state = State.EXPECT_BCAST;
-    int lastHops = 0;
+    private State state = State.EXPECT_BCAST;
+    private long lastUpdated = 0;
+    private boolean publish = false;
 
     /**
      * Advance the state machine and determine if update is genuine (no duplicate)
      *
      * @param a the group message (action) that was received
-     * @param hops number of hops that was given on the message. Currently not used.
+     * @param address the address of the device that this state machine belongs to
+     * @param group the group that this state machine belongs to
      * @return true if the group message is not a duplicate
      */
-    public boolean action(GroupMessage a, int hops) {
-        boolean publish = false;
+    public boolean action(GroupMessage a, InsteonAddress address, int group) {
+        publish = false;
         switch (state) {
             case EXPECT_BCAST:
                 switch (a) {
@@ -166,7 +168,17 @@ public boolean action(GroupMessage a, int hops) {
                 state = State.EXPECT_BCAST;
                 break;
         }
-        logger.trace("group state: {} --{}--> {}, publish: {}", oldState, a, state, publish);
+
+        lastUpdated = System.currentTimeMillis();
+        logger.debug("{} group {} state: {} --{}--> {}, publish: {}", address, group, oldState, a, state, publish);
         return (publish);
     }
+
+    public long getLastUpdated() {
+        return lastUpdated;
+    }
+
+    public boolean getPublish() {
+        return publish;
+    }
 }

bundles/org.openhab.binding.insteon/src/main/java/org/openhab/binding/insteon/internal/device/InsteonDevice.java

@@ -17,6 +17,7 @@
 import java.util.HashMap;
 import java.util.Iterator;
 import java.util.List;
+import java.util.Map;
 import java.util.Map.Entry;
 import java.util.PriorityQueue;
 
@@ -25,6 +26,7 @@
 import org.eclipse.smarthome.core.types.Command;
 import org.openhab.binding.insteon.internal.config.InsteonChannelConfiguration;
 import org.openhab.binding.insteon.internal.device.DeviceType.FeatureGroup;
+import org.openhab.binding.insteon.internal.device.GroupMessageStateMachine.GroupMessage;
 import org.openhab.binding.insteon.internal.driver.Driver;
 import org.openhab.binding.insteon.internal.message.FieldException;
 import org.openhab.binding.insteon.internal.message.InvalidMessageTypeException;
@@ -63,14 +65,15 @@ public static enum DeviceStatus {
     private @Nullable Driver driver = null;
     private HashMap<String, @Nullable DeviceFeature> features = new HashMap<>();
     private @Nullable String productKey = null;
-    private Long lastTimePolled = 0L;
-    private Long lastMsgReceived = 0L;
+    private volatile long lastTimePolled = 0L;
+    private volatile long lastMsgReceived = 0L;
     private boolean isModem = false;
     private PriorityQueue<@Nullable QEntry> mrequestQueue = new PriorityQueue<>();
     private @Nullable DeviceFeature featureQueried = null;
     private long lastQueryTime = 0L;
     private boolean hasModemDBEntry = false;
     private DeviceStatus status = DeviceStatus.INITIALIZED;
+    private Map<Integer, @Nullable GroupMessageStateMachine> groupState = new HashMap<>();
 
     /**
      * Constructor
@@ -266,22 +269,18 @@ public void doPoll(long delay) {
         RequestQueueManager.instance().addQueue(this, now + delay);
 
         if (!l.isEmpty()) {
-            synchronized (lastTimePolled) {
-                lastTimePolled = now;
-            }
+            lastTimePolled = now;
         }
     }
 
     /**
      * Handle incoming message for this device by forwarding
      * it to all features that this device supports
-     * 
+     *
      * @param msg the incoming message
      */
     public void handleMessage(Msg msg) {
-        synchronized (lastMsgReceived) {
-            lastMsgReceived = System.currentTimeMillis();
-        }
+        lastMsgReceived = System.currentTimeMillis();
         synchronized (features) {
             // first update all features that are
             // not status features
@@ -546,6 +545,32 @@ private void addFeature(String name, DeviceFeature f) {
         }
     }
 
+    /**
+     * Get the state of the state machine that suppresses duplicates for group messages.
+     * The state machine is advance the first time it is called for a message,
+     * otherwise return the current state.
+     *
+     * @param group the insteon group of the broadcast message
+     * @param a the type of group message came in (action etc)
+     * @return true if this is message is NOT a duplicate
+     */
+    public boolean getGroupState(int group, GroupMessage a) {
+        GroupMessageStateMachine m = groupState.get(group);
+        if (m == null) {
+            m = new GroupMessageStateMachine();
+            groupState.put(group, m);
+            logger.trace("{} created group {} state", address, group);
+        } else {
+            if (lastMsgReceived <= m.getLastUpdated()) {
+                logger.trace("{} using previous group {} state for {}", address, group, a);
+                return m.getPublish();
+            }
+        }
+
+        logger.trace("{} updating group {} state to {}", address, group, a);
+        return (m.action(a, address, group));
+    }
+
     @Override
     public String toString() {
         String s = address.toString();

bundles/org.openhab.binding.insteon/src/main/java/org/openhab/binding/insteon/internal/device/MessageHandler.java

@@ -53,9 +53,8 @@
 public abstract class MessageHandler {
     private static final Logger logger = LoggerFactory.getLogger(MessageHandler.class);
 
-    DeviceFeature feature;
-    Map<String, @Nullable String> parameters = new HashMap<>();
-    Map<Integer, @Nullable GroupMessageStateMachine> groupState = new HashMap<>();
+    protected DeviceFeature feature;
+    protected Map<String, @Nullable String> parameters = new HashMap<>();
 
     /**
      * Constructor
@@ -253,20 +252,19 @@ protected boolean isDuplicate(Msg msg) {
         boolean isDuplicate = false;
         try {
             MsgType t = MsgType.fromValue(msg.getByte("messageFlags"));
-            int hops = msg.getHopsLeft();
             if (t == MsgType.ALL_LINK_BROADCAST) {
                 int group = msg.getAddress("toAddress").getLowByte() & 0xff;
                 byte cmd1 = msg.getByte("command1");
                 // if the command is 0x06, then it's success message
                 // 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 = !updateGroupState(group, hops, gm);
+                isDuplicate = !feature.getDevice().getGroupState(group, gm);
             } 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 = !updateGroupState(group, hops, GroupMessage.CLEAN);
+                isDuplicate = !feature.getDevice().getGroupState(group, GroupMessage.CLEAN);
             }
         } catch (IllegalArgumentException e) {
             logger.warn("cannot parse msg: {}", msg, e);
@@ -276,24 +274,6 @@ protected boolean isDuplicate(Msg msg) {
         return (isDuplicate);
     }
 
-    /**
-     * Advance the state of the state machine that suppresses duplicates
-     *
-     * @param group the insteon group of the broadcast message
-     * @param hops number of hops left
-     * @param a what type of group message came in (action etc)
-     * @return true if this is message is NOT a duplicate
-     */
-    private boolean updateGroupState(int group, int hops, GroupMessage a) {
-        GroupMessageStateMachine m = groupState.get(new Integer(group));
-        if (m == null) {
-            m = new GroupMessageStateMachine();
-            groupState.put(new Integer(group), m);
-        }
-        logger.trace("updating group state for {} to {}", group, a);
-        return (m.action(a, hops));
-    }
-
     /**
      * Extract button information from message
      *