[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

@@ -712,7 +712,23 @@ public boolean isConnected() {
         return currentSocket != null && currentSocket.isConnected();
     }
 
+    /**
+     * Establish a connection, replacing any current one. Serialized with shell commands because it
+     * starts by disconnecting. {@link #disconnect()} stays outside the lock, since aborting a
+     * running command is what some of its callers need.
+     */
     public void connect() throws AndroidDebugBridgeDeviceException, InterruptedException {
+        // Interruptible: dispose() interrupts the connection checker, and a plain lock() would let
+        // it acquire the lock afterwards and build a connection on a disposed handler.
+        commandLock.lockInterruptibly();
+        try {
+            connectInternal();
+        } finally {
+            commandLock.unlock();
+        }
+    }
+
+    private void connectInternal() throws AndroidDebugBridgeDeviceException, InterruptedException {
         this.disconnect();
         AdbConnection adbConnection;
         Socket sock;
@@ -749,38 +765,55 @@ 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<>();
+        // Tracked separately: only a failed open can mean the command never started.
+        AtomicReference<@Nullable Exception> openError = new AtomicReference<>();
+        AtomicReference<@Nullable Exception> readError = new AtomicReference<>();
+        // Interruptible for the same reason as connect().
+        commandLock.lockInterruptibly();
         try {
-            commandLock.lock();
+            // Read under the lock: a reconnect could otherwise close it while we waited.
+            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)) {
+                AdbStream stream;
+                try {
+                    stream = adb.open("shell:" + cmd);
+                } catch (IllegalStateException | IOException e) {
+                    if (!"Stream closed".equals(e.getMessage())) {
+                        // Captured rather than thrown: escaping the scheduled task would log a noisy
+                        // stacktrace for an expected condition. Re-thrown on the caller below.
+                        openError.set(e);
+                    }
+                    return "";
+                }
+                try (stream) {
                     do {
                         byteArrayOutputStream.writeBytes(stream.read());
                     } while (!stream.isClosed());
                 } 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);
+                        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 +862,14 @@ private static AdbCrypto loadKeyPair(Path pubKey, Path privKey)
         return c;
     }
 
+    /**
+     * Reconnect for a retry. Named separately from {@link #connect()} because calling
+     * {@link #disconnect()} here directly would cancel another operation's in-flight command.
+     */
+    public void reconnectForRetry() throws AndroidDebugBridgeDeviceException, InterruptedException {
+        connect();
+    }
+
     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,33 @@
+/*
+ * 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, as a device in standby commonly causes.
+ *
+ * Distinct from a failure while reading an already-open stream, where the command certainly
+ * reached the device. It is not proof the command never ran though: adblib writes the OPEN packet
+ * inside {@code AdbConnection.open()}, so delivery is ambiguous if that write fails.
+ *
+ * @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,19 @@ public void handleCommand(ChannelUID channelUID, Command command) {
                 // try reconnect
                 adbConnection.connect();
             }
-            handleCommandInternal(channelUID, command);
+            try {
+                handleCommandInternal(channelUID, command);
+            } catch (AndroidDebugBridgeDeviceStreamRejectedException e) {
+                // A failed stream open does not prove the device never got the request, 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 +125,30 @@ public void handleCommand(ChannelUID channelUID, Command command) {
         }
     }
 
+    /**
+     * Channels that treat {@link RefreshType} as a read. Elsewhere REFRESH is not special cased and
+     * ends up as a value, so {@code text} would type "REFRESH".
+     */
+    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);
+
+    /**
+     * Whether running {@code command} twice cannot change what the device ends up doing: a read, or
+     * the wake-up key event, which does nothing to an already awake device.
+     */
+    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 {