From a9b6d9244e7b90b5265292d890b9ad8a62fae38f Mon Sep 17 00:00:00 2001 From: Jeremy Date: Sun, 6 Jul 2025 13:25:51 -0400 Subject: [PATCH] [Insteon] Refactor device request scheduled delay (#18892) * [Insteon] Refactor device request scheduled delay Signed-off-by: Jeremy Setton --- .../insteon/internal/device/BaseDevice.java | 74 +++++++++---------- .../insteon/internal/device/Device.java | 4 +- .../internal/device/InsteonDevice.java | 4 +- .../insteon/internal/device/LegacyDevice.java | 2 +- .../internal/device/RequestManager.java | 27 ++++--- .../internal/transport/message/Msg.java | 4 +- 6 files changed, 56 insertions(+), 59 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 7f0ccfab4c..215f8c951e 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 @@ -414,7 +414,7 @@ public abstract class BaseDevice<@NonNull T extends DeviceAddress, @NonNull S ex * * @param msg the message to be sent * @param feature device feature associated to the message - * @param delay time (in milliseconds) to delay before sending message + * @param delay delay (in milliseconds) before sending message */ @Override public void sendMessage(Msg msg, DeviceFeature feature, long delay) { @@ -426,7 +426,7 @@ public abstract class BaseDevice<@NonNull T extends DeviceAddress, @NonNull S ex * * @param msg message to be sent * @param feature device feature that sent this message - * @param delay time (in milliseconds) to delay before sending message + * @param delay delay (in milliseconds) before sending message */ protected void addRequest(Msg msg, DeviceFeature feature, long delay) { logger.trace("enqueuing request with delay {} msec", delay); @@ -451,16 +451,19 @@ public abstract class BaseDevice<@NonNull T extends DeviceAddress, @NonNull S ex /** * Handles next request for this device * - * @return wait time (in milliseconds) before processing the subsequent request + * @return delay (in milliseconds) before processing the subsequent request */ @Override public long handleNextRequest() { - long now = System.currentTimeMillis(); - // wait for feature queried to be processed or next request to be scheduled - long waitTime = Optional.of(checkFeatureQueriedStatus(now)).filter(time -> time > 0) - .orElseGet(() -> checkNextRequestScheduledTime(now)); - if (waitTime > 0) { - return waitTime; + // wait for feature queried to be processed + long queryDelay = checkFeatureQueriedStatus(); + if (queryDelay > 0) { + return queryDelay; + } + // wait for next request to be scheduled + long scheduledDelay = checkNextRequestScheduledDelay(); + if (scheduledDelay > 0) { + return scheduledDelay; } // poll next request from queue DeviceRequest request = pollNextRequest(); @@ -470,8 +473,8 @@ public abstract class BaseDevice<@NonNull T extends DeviceAddress, @NonNull S ex // get request feature and message DeviceFeature feature = request.getFeature(); Msg msg = request.getMessage(); - // update message timestamp - msg.setTimestamp(now); + // reset message timestamp + msg.resetTimestamp(); // set feature queried for non-broadcast request message if (!msg.isAllLinkBroadcast()) { logger.trace("request taken off direct for {}: {}", feature.getName(), msg); @@ -489,15 +492,15 @@ public abstract class BaseDevice<@NonNull T extends DeviceAddress, @NonNull S ex if (modem != null) { modem.writeMessage(msg); } - // determine the wait time for the next request + // determine the delay for the next request DeviceRequest nextRequest = peekNextRequest(); - waitTime = now + msg.getQuietTime(); + long nextRequestDelay = msg.getQuietTime(); if (nextRequest != null) { - waitTime = Math.max(waitTime, nextRequest.getScheduledTime()); - nextRequest.setScheduledTime(waitTime); + nextRequestDelay = Math.max(nextRequestDelay, nextRequest.getScheduledDelay()); + nextRequest.setScheduledDelay(nextRequestDelay); } - logger.trace("next request scheduled in {} msec", waitTime - now); - return waitTime; + logger.trace("next request scheduled in {} msec", nextRequestDelay); + return nextRequestDelay; } /** @@ -529,10 +532,9 @@ public abstract class BaseDevice<@NonNull T extends DeviceAddress, @NonNull S ex /** * Checks feature queried status * - * @param now the current time - * @return wait time if necessary otherwise 0 + * @return delay (in milliseconds) if necessary otherwise 0 */ - private long checkFeatureQueriedStatus(long now) { + private long checkFeatureQueriedStatus() { DeviceFeature feature = getFeatureQueried(); if (feature != null) { QueryStatus queryStatus = feature.getQueryStatus(); @@ -542,7 +544,7 @@ public abstract class BaseDevice<@NonNull T extends DeviceAddress, @NonNull S ex DeviceRequest request = peekNextRequest(); if (request == null || !request.getMessage().hasHigherPriorityThan(feature.getQueryMessage())) { logger.trace("still waiting for {} query to be sent to {}", feature.getName(), address); - return now + 1000L; // retry in 1000 ms + return 1000L; // retry in 1000 ms } logger.debug("gave up waiting for {} query to be sent to {}", feature.getName(), address); // notify feature queried expired @@ -551,11 +553,11 @@ public abstract class BaseDevice<@NonNull T extends DeviceAddress, @NonNull S ex case QUERY_SENT: case QUERY_ACKED: // wait for the feature queried to be answered unless timed out - long maxAckTime = lastRequestSent + DIRECT_ACK_TIMEOUT; - if (maxAckTime > now) { + long delay = lastRequestSent + DIRECT_ACK_TIMEOUT - System.currentTimeMillis(); + if (delay > 0) { logger.trace("still waiting for {} query reply from {} for another {} msec", feature.getName(), - address, maxAckTime - now); - return now + 500L; // retry in 500 ms + address, delay); + return 500L; // retry in 500 ms } logger.debug("gave up waiting for {} query reply from {}", feature.getName(), address); // notify feature queried failed @@ -572,18 +574,12 @@ public abstract class BaseDevice<@NonNull T extends DeviceAddress, @NonNull S ex } /** - * Checks next request scheduled time + * Checks next request scheduled delay * - * @param now the current time - * @return wait time if necessary otherwise 0 + * @return delay (in milliseconds) if necessary otherwise 0 */ - private long checkNextRequestScheduledTime(long now) { - DeviceRequest request = peekNextRequest(); - // wait for next request scheduled time if necessary - if (request != null && request.getScheduledTime() > now) { - return request.getScheduledTime() - now; - } - return 0L; + private long checkNextRequestScheduledDelay() { + return Optional.ofNullable(peekNextRequest()).map(DeviceRequest::getScheduledDelay).orElse(0L); } /** @@ -720,18 +716,14 @@ public abstract class BaseDevice<@NonNull T extends DeviceAddress, @NonNull S ex return msg; } - public long getScheduledTime() { - return scheduledTime; + public long getScheduledDelay() { + return Math.max(0, scheduledTime - System.currentTimeMillis()); } public void setScheduledDelay(long delay) { this.scheduledTime = System.currentTimeMillis() + delay; } - public void setScheduledTime(long scheduledTime) { - this.scheduledTime = scheduledTime; - } - @Override public int compareTo(DeviceRequest other) { int result = msg.getPriority().compareTo(other.msg.getPriority()); diff --git a/bundles/org.openhab.binding.insteon/src/main/java/org/openhab/binding/insteon/internal/device/Device.java b/bundles/org.openhab.binding.insteon/src/main/java/org/openhab/binding/insteon/internal/device/Device.java index 12cd752a2c..715ee19c82 100644 --- a/bundles/org.openhab.binding.insteon/src/main/java/org/openhab/binding/insteon/internal/device/Device.java +++ b/bundles/org.openhab.binding.insteon/src/main/java/org/openhab/binding/insteon/internal/device/Device.java @@ -80,14 +80,14 @@ public interface Device { * * @param msg the message to be sent * @param feature device feature associated to the message - * @param delay time (in milliseconds) to delay before sending message + * @param delay delay (in milliseconds) before sending message */ public void sendMessage(Msg msg, DeviceFeature feature, long delay); /** * Handles next request for this device * - * @return time (in milliseconds) before processing the subsequent request + * @return delay (in milliseconds) before processing the subsequent request */ public long handleNextRequest(); 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 2c469ac33a..bff1b93b91 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 @@ -471,7 +471,7 @@ public class InsteonDevice extends BaseDevice { if (request == null) { logger.trace("scheduling request for {} in {} msec", device.getAddress(), delay); - request = new RequestEntry(device, scheduledTime); + request = new RequestEntry(device, delay); scheduleRequest(request); - } else if (request.scheduledTime > scheduledTime) { + } else if (request.getScheduledDelay() > delay) { logger.trace("rescheduling request for {} from {} to {} msec", device.getAddress(), - request.scheduledTime - now, delay); - request.scheduledTime = scheduledTime; + request.getScheduledDelay(), delay); + request.setScheduledDelay(delay); cancelRequest(request); scheduleRequest(request); } @@ -123,7 +120,7 @@ public class RequestManager { return; } - long delay = Math.max(0, request.scheduledTime - System.currentTimeMillis()); + long delay = request.getScheduledDelay(); request.job = scheduler.schedule(() -> handleRequest(request.device), delay, TimeUnit.MILLISECONDS); logger.trace("request for {} scheduled in {} msec", request.device.getAddress(), delay); @@ -143,7 +140,7 @@ public class RequestManager { logger.trace("handling request for {}", device.getAddress()); - long delay = device.handleNextRequest() - System.currentTimeMillis(); + long delay = device.handleNextRequest(); if (delay > 0) { addRequest(device, delay); } else { @@ -159,9 +156,17 @@ public class RequestManager { private volatile long scheduledTime; private volatile @Nullable ScheduledFuture job; - RequestEntry(Device device, long scheduledTime) { + public RequestEntry(Device device, long delay) { this.device = device; - this.scheduledTime = scheduledTime; + setScheduledDelay(delay); + } + + public long getScheduledDelay() { + return Math.max(0, scheduledTime - System.currentTimeMillis()); + } + + public void setScheduledDelay(long delay) { + this.scheduledTime = System.currentTimeMillis() + delay; } } } diff --git a/bundles/org.openhab.binding.insteon/src/main/java/org/openhab/binding/insteon/internal/transport/message/Msg.java b/bundles/org.openhab.binding.insteon/src/main/java/org/openhab/binding/insteon/internal/transport/message/Msg.java index fa836e3501..8ed9da7ee0 100644 --- a/bundles/org.openhab.binding.insteon/src/main/java/org/openhab/binding/insteon/internal/transport/message/Msg.java +++ b/bundles/org.openhab.binding.insteon/src/main/java/org/openhab/binding/insteon/internal/transport/message/Msg.java @@ -254,8 +254,8 @@ public class Msg { this.replayed = replayed; } - public void setTimestamp(long timestamp) { - this.timestamp = timestamp; + public void resetTimestamp() { + this.timestamp = System.currentTimeMillis(); } public boolean containsField(String key) {