[androiddebugbridge] Retry a command once when standby rejects the shell stream

bundles/org.openhab.binding.androiddebugbridge/src/main/java/org/openhab/binding/androiddebugbridge/internal/AndroidDebugBridgeBindingConstants.java

@@ -50,6 +50,9 @@ public class AndroidDebugBridgeBindingConstants {
     public static final String SHUTDOWN_CHANNEL = "shutdown";
     public static final String RECORD_INPUT_CHANNEL = "record-input";
     public static final String RECORDED_INPUT_CHANNEL = "recorded-input";
+    // Wake-up key event, accepted by "input keyevent" either by name or by numeric code
+    public static final String KEY_EVENT_WAKEUP_NAME = "KEYCODE_WAKEUP";
+    public static final String KEY_EVENT_WAKEUP_CODE = "224";
     // List of all Parameters
     public static final String PARAMETER_IP = "ip";
     public static final String PARAMETER_PORT = "port";

bundles/org.openhab.binding.androiddebugbridge/src/main/java/org/openhab/binding/androiddebugbridge/internal/AndroidDebugBridgeDevice.java

@@ -749,38 +749,61 @@ private String runAdbShell(String... args)
 
     private String runAdbShell(int commandTimeout, String... args)
             throws InterruptedException, AndroidDebugBridgeDeviceException, TimeoutException, ExecutionException {
-        var adb = connection;
-        if (adb == null) {
-            throw new AndroidDebugBridgeDeviceException("Device not connected");
-        }
-        AtomicReference<@Nullable Exception> streamError = new AtomicReference<>();
+        // Failures while opening the stream and failures while reading its output are tracked
+        // separately: only the former can mean the shell command never started. Folding both into
+        // one reference would let a failure that happened *after* the command already ran be
+        // reported as a stream rejection, and the caller may retry on that.
+        AtomicReference<@Nullable Exception> openError = new AtomicReference<>();
+        AtomicReference<@Nullable Exception> readError = new AtomicReference<>();
+        commandLock.lock();
         try {
-            commandLock.lock();
+            // Read the connection only once the lock is held. Capturing it earlier would let a
+            // reconnect replace and close it while this call was still waiting for the lock, so the
+            // command would then run against a connection that is already closed.
+            var adb = connection;
+            if (adb == null) {
+                throw new AndroidDebugBridgeDeviceException("Device not connected");
+            }
             var commandFuture = scheduler.submit(() -> {
                 var byteArrayOutputStream = new ByteArrayOutputStream();
                 String cmd = String.join(" ", args);
                 logger.debug("{} - shell:{}", ip, cmd);
-                try (AdbStream stream = adb.open("shell:" + cmd)) {
-                    do {
-                        byteArrayOutputStream.writeBytes(stream.read());
-                    } while (!stream.isClosed());
+                AdbStream stream;
+                try {
+                    stream = adb.open("shell:" + cmd);
                 } catch (IllegalStateException | IOException e) {
                     if (!"Stream closed".equals(e.getMessage())) {
                         // Capture rather than throw: letting it escape the scheduled task makes openHAB's
                         // WrappedScheduledExecutorService log a noisy "Scheduled runnable ended with an
                         // exception" stacktrace for an expected condition (a standby adbd rejecting the
                         // shell stream). It is re-surfaced as a typed exception on the calling thread below.
-                        streamError.set(e);
+                        openError.set(e);
+                    }
+                    return "";
+                }
+                try (stream) {
+                    do {
+                        byteArrayOutputStream.writeBytes(stream.read());
+                    } while (!stream.isClosed());
+                } catch (IllegalStateException | IOException e) {
+                    if (!"Stream closed".equals(e.getMessage())) {
+                        readError.set(e);
                     }
                 }
                 return byteArrayOutputStream.toString(StandardCharsets.US_ASCII);
             });
             this.commandFuture = commandFuture;
             String result = commandFuture.get(commandTimeout, TimeUnit.SECONDS);
-            Exception error = streamError.get();
-            if (error != null) {
+            Exception failedOpen = openError.get();
+            if (failedOpen != null) {
+                throw new AndroidDebugBridgeDeviceStreamRejectedException(
+                        "Error opening adb shell stream " + ip + ":" + port + ": " + failedOpen.getMessage());
+            }
+            Exception failedRead = readError.get();
+            if (failedRead != null) {
+                // The stream was open, so the command reached the device: never retryable.
                 throw new AndroidDebugBridgeDeviceException(
-                        "Error opening adb shell stream " + ip + ":" + port + ": " + error.getMessage());
+                        "Error reading adb shell stream " + ip + ":" + port + ": " + failedRead.getMessage());
             }
             return result;
         } finally {
@@ -829,6 +852,25 @@ private static AdbCrypto loadKeyPair(Path pubKey, Path privKey)
         return c;
     }
 
+    /**
+     * Reconnect for a retry, without disturbing anything else that is running.
+     *
+     * Taken under the command lock so no shell command can be in flight: a plain
+     * {@link #disconnect()} here would cancel whatever future another operation had just installed
+     * in {@code commandFuture}, making its {@code get()} throw {@link java.util.concurrent.CancellationException}
+     * in a caller that does not expect it. {@code disconnect()} keeps aborting running commands,
+     * which is what its other callers want.
+     */
+    public void reconnectForRetry() throws AndroidDebugBridgeDeviceException, InterruptedException {
+        commandLock.lock();
+        try {
+            disconnect();
+            connect();
+        } finally {
+            commandLock.unlock();
+        }
+    }
+
     public void disconnect() {
         var commandFuture = this.commandFuture;
         if (commandFuture != null && !commandFuture.isDone()) {

bundles/org.openhab.binding.androiddebugbridge/src/main/java/org/openhab/binding/androiddebugbridge/internal/AndroidDebugBridgeDeviceStreamRejectedException.java

@@ -0,0 +1,36 @@
+/*
+ * Copyright (c) 2010-2026 Contributors to the openHAB project
+ *
+ * See the NOTICE file(s) distributed with this work for additional
+ * information.
+ *
+ * This program and the accompanying materials are made available under the
+ * terms of the Eclipse Public License 2.0 which is available at
+ * http://www.eclipse.org/legal/epl-2.0
+ *
+ * SPDX-License-Identifier: EPL-2.0
+ */
+package org.openhab.binding.androiddebugbridge.internal;
+
+import org.eclipse.jdt.annotation.NonNullByDefault;
+
+/**
+ * Thrown when opening the adb shell stream fails, which a device in standby commonly causes.
+ *
+ * It is a distinct type so the caller can tell this apart from a failure that happened while
+ * reading an already-open stream, where the command certainly reached the device. It is however
+ * <em>not</em> proof that the command never ran: adblib writes the OPEN packet inside
+ * {@code AdbConnection.open()}, so an {@link java.io.IOException} raised while sending that packet
+ * leaves delivery ambiguous. Callers must therefore only repeat commands that stay correct when
+ * executed twice.
+ *
+ * @author Stamate Viorel - Initial contribution
+ */
+@NonNullByDefault
+public class AndroidDebugBridgeDeviceStreamRejectedException extends AndroidDebugBridgeDeviceException {
+    private static final long serialVersionUID = 5471982041566281957L;
+
+    public AndroidDebugBridgeDeviceStreamRejectedException(String message) {
+        super(message);
+    }
+}

bundles/org.openhab.binding.androiddebugbridge/src/main/java/org/openhab/binding/androiddebugbridge/internal/AndroidDebugBridgeHandler.java

@@ -17,6 +17,7 @@
 import java.util.Arrays;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.ScheduledFuture;
 import java.util.concurrent.TimeUnit;
@@ -97,7 +98,24 @@ public void handleCommand(ChannelUID channelUID, Command command) {
                 // try reconnect
                 adbConnection.connect();
             }
-            handleCommandInternal(channelUID, command);
+            try {
+                handleCommandInternal(channelUID, command);
+            } catch (AndroidDebugBridgeDeviceStreamRejectedException e) {
+                // Socket.isConnected() only records that a connection was once established; it stays
+                // true after the device stops serving the socket, so the check above cannot notice a
+                // device that went into standby and the shell stream open is refused instead. That
+                // drops the first command after standby, typically the very wake-up meant to end it.
+                //
+                // Opening the stream failing does not prove the device never got the request though,
+                // so only repeat commands that stay correct when they run twice.
+                if (!isSafeToRepeat(channelUID.getId(), command)) {
+                    throw e;
+                }
+                logger.debug("{} - shell stream rejected, reconnecting and retrying command: {}", currentConfig.ip,
+                        e.getMessage());
+                adbConnection.reconnectForRetry();
+                handleCommandInternal(channelUID, command);
+            }
         } catch (InterruptedException ignored) {
         } catch (AndroidDebugBridgeDeviceException | ExecutionException e) {
             if (!(e.getCause() instanceof InterruptedException)) {
@@ -112,6 +130,37 @@ public void handleCommand(ChannelUID channelUID, Command command) {
         }
     }
 
+    /**
+     * Channels whose handling of {@link RefreshType} is a read. Elsewhere REFRESH is not special
+     * cased and ends up as a value, so {@code text} would type "REFRESH" and {@code record-input}
+     * would act -- neither may be repeated.
+     */
+    private static final Set<String> REFRESH_READS_STATE = Set.of(CURRENT_PACKAGE_CHANNEL, WAKE_LOCK_CHANNEL,
+            AWAKE_STATE_CHANNEL, SCREEN_STATE_CHANNEL, MEDIA_VOLUME_CHANNEL, MEDIA_CONTROL_CHANNEL,
+            START_INTENT_CHANNEL);
+
+    /**
+     * Tells whether running {@code command} a second time cannot change what the device ends up
+     * doing, which is what makes it safe to repeat after a rejected shell stream.
+     *
+     * Two cases qualify. A {@link RefreshType} on a channel that actually treats it as a read, and
+     * the wake-up key event, which is naturally idempotent since waking an already awake device
+     * does nothing -- that is also the case this recovery exists for. Everything else may act
+     * twice: {@code text} would type the input again, {@code tap} would tap again,
+     * {@code media-control} would toggle back, {@code shutdown} would reboot, and so on.
+     */
+    private static boolean isSafeToRepeat(String channelId, Command command) {
+        if (command instanceof RefreshType) {
+            return REFRESH_READS_STATE.contains(channelId);
+        }
+        if (!KEY_EVENT_CHANNEL.equals(channelId)) {
+            return false;
+        }
+        // "input keyevent" takes either the symbolic name or the numeric code.
+        String keyEvent = command.toFullString().trim();
+        return KEY_EVENT_WAKEUP_NAME.equalsIgnoreCase(keyEvent) || KEY_EVENT_WAKEUP_CODE.equals(keyEvent);
+    }
+
     private void handleCommandInternal(ChannelUID channelUID, Command command)
             throws InterruptedException, AndroidDebugBridgeDeviceException, AndroidDebugBridgeDeviceReadException,
             TimeoutException, ExecutionException {