From 415a920811a6e36506bad4f5abec5fafbd416f52 Mon Sep 17 00:00:00 2001 From: Rob Nielsen Date: Mon, 15 Jun 2020 07:40:26 -0500 Subject: [PATCH] [insteon] keep track of group state at the device level Signed-off-by: Rob Nielsen --- .../device/GroupMessageStateMachine.java | 24 ++++++++--- .../internal/device/InsteonDevice.java | 43 +++++++++++++++---- .../internal/device/MessageHandler.java | 28 ++---------- 3 files changed, 56 insertions(+), 39 deletions(-) diff --git a/bundles/org.openhab.binding.insteon/src/main/java/org/openhab/binding/insteon/internal/device/GroupMessageStateMachine.java b/bundles/org.openhab.binding.insteon/src/main/java/org/openhab/binding/insteon/internal/device/GroupMessageStateMachine.java index 1f6cd84d6074f..21ba6d0b28fd8 100644 --- a/bundles/org.openhab.binding.insteon/src/main/java/org/openhab/binding/insteon/internal/device/GroupMessageStateMachine.java +++ b/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; + } } 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 97b9e00c9b0ae..dcdb26963d923 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 @@ -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 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 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(); diff --git a/bundles/org.openhab.binding.insteon/src/main/java/org/openhab/binding/insteon/internal/device/MessageHandler.java b/bundles/org.openhab.binding.insteon/src/main/java/org/openhab/binding/insteon/internal/device/MessageHandler.java index 6a725a8ced971..5fb0578d74ba8 100644 --- a/bundles/org.openhab.binding.insteon/src/main/java/org/openhab/binding/insteon/internal/device/MessageHandler.java +++ b/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 parameters = new HashMap<>(); - Map groupState = new HashMap<>(); + protected DeviceFeature feature; + protected Map parameters = new HashMap<>(); /** * Constructor @@ -253,7 +252,6 @@ 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"); @@ -261,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 = !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 *