diff --git a/python/packages/jumpstarter-driver-adb/README.md b/python/packages/jumpstarter-driver-adb/README.md index 2ea7cddf0..3cb3c4c1d 100644 --- a/python/packages/jumpstarter-driver-adb/README.md +++ b/python/packages/jumpstarter-driver-adb/README.md @@ -31,9 +31,10 @@ export: | Parameter | Description | Type | Required | Default | | --------- | ---------------------------------------------- | ---- | -------- | -------------------------- | -| adb_path | Path to the ADB executable on the exporter | str | no | "adb" (resolved from PATH) | -| host | Host address of the ADB server on the exporter | str | no | "127.0.0.1" | -| port | Port of the ADB server on the exporter | int | no | 15037 | +| adb_path | Path to the ADB executable on the exporter | str | no | "adb" (resolved from PATH) | +| host | Host address of the ADB server on the exporter | str | no | "127.0.0.1" | +| port | Port of the ADB server on the exporter | int | no | 15037 | +| connect_timeout | Timeout (seconds) for `connect`/`disconnect` commands | float | no | 30.0 | ### Port Assignment @@ -102,6 +103,81 @@ For native `adb` or external tools, export the env vars printed by the The `nodaemon` command is not supported as it would start a local ADB server process, ignoring the tunnel entirely. +### Connecting to a remote device + +When the Android device is **not** attached to the exporter over USB but is +reachable over the network (for example a virtual device such as +[Cuttlefish](https://source.android.com/docs/devices/cuttlefish), or a device +exposing `adb` over TCP/IP), the exporter's ADB server must `connect` to it +before any `adb` command will see it. + +The `connect_device` / `disconnect_device` driver methods run +`adb connect ` / `adb disconnect ` on the exporter. The +address is supplied by the caller — this driver does **not** discover or scan +for devices. Timeouts and command failures raise, so callers can react instead +of receiving a silent error string. + +#### From the CLI + +`connect` and `disconnect` are also plain adb commands, so they pass through the +tunnel like any other: + +```bash +# Connect the exporter's ADB server to a networked device, then use it +j adb connect 10.0.0.5:6520 +j adb devices +j adb shell getprop ro.product.model +j adb disconnect 10.0.0.5:6520 +``` + +#### From a parent (composite) driver + +The intended use case is a higher-level driver that owns the device lifecycle +and knows the address deterministically — no IP discovery needed. For example, +the Cuttlefish driver embeds an `AdbServer` child and connects to a pinned +address derived from its own config (`host` + an ADB port computed from the +instance number) after the virtual device is created: + +```python +class CuttlefishServer(CompositeInterface, Driver): + def __post_init__(self): + super().__post_init__() + # AdbServer runs on the exporter; the parent drives connect/disconnect + self.children["adb"] = AdbServer(host="127.0.0.1", port=self.adb_server_port) + + def _adb_device(self) -> str: + # Address is known from config, never scanned + return f"{self.host}:{6520 + self.instance_num - 1}" + + def _connect(self): + adb = self.children["adb"] + device = self._adb_device() + try: + adb.connect_device(device) + except (subprocess.CalledProcessError, TimeoutError) as e: + # Device may not be up yet; the boot-wait loop below reconnects. + self.logger.warning("ADB connect to %s failed (%s); retrying while waiting for boot", device, e) + # unexpected exceptions (config/programming errors) propagate + + def _wait_for_boot(self): + adb = self.children["adb"] + device = self._adb_device() + deadline = time.monotonic() + self.boot_timeout + while time.monotonic() < deadline: + try: + adb.connect_device(device) + if self._is_booted(device): + return + except (subprocess.CalledProcessError, TimeoutError): + pass + time.sleep(3) + raise TimeoutError(f"{device} did not come online within {self.boot_timeout}s") +``` + +Because `connect_device` raises on failure or timeout, the parent catches only +the *expected* connection failures (letting configuration or programming errors +propagate) and drives its own retry loop rather than parsing return strings. + ### Integration with Android Ecosystem Tools #### Forward ADB for external tools @@ -207,12 +283,12 @@ with client.adb.forward_adb(port=0) as (host, port): ```{eval-rst} .. autoclass:: jumpstarter_driver_adb.driver.AdbServer() - :members: start_server, kill_server, list_devices + :members: start_server, kill_server, connect_device, disconnect_device, list_devices ``` ### Client ```{eval-rst} .. autoclass:: jumpstarter_driver_adb.client.AdbClient() - :members: forward_adb, start_server, kill_server, list_devices + :members: forward_adb, start_server, kill_server, connect_device, disconnect_device, list_devices ``` diff --git a/python/packages/jumpstarter-driver-adb/jumpstarter_driver_adb/client.py b/python/packages/jumpstarter-driver-adb/jumpstarter_driver_adb/client.py index 0b7a02da9..19d5a917c 100644 --- a/python/packages/jumpstarter-driver-adb/jumpstarter_driver_adb/client.py +++ b/python/packages/jumpstarter-driver-adb/jumpstarter_driver_adb/client.py @@ -79,6 +79,14 @@ def kill_server(self) -> int: """Kill ADB server on the exporter.""" return self.call("kill_server") + def connect_device(self, device: str) -> str: + """Connect to an ADB device by address (host:port).""" + return self.call("connect_device", device) + + def disconnect_device(self, device: str) -> str: + """Disconnect an ADB device by address (host:port).""" + return self.call("disconnect_device", device) + def list_devices(self) -> str: """List devices visible to the exporter's ADB server.""" return self.call("list_devices") diff --git a/python/packages/jumpstarter-driver-adb/jumpstarter_driver_adb/driver.py b/python/packages/jumpstarter-driver-adb/jumpstarter_driver_adb/driver.py index 45725e5e7..f09f39503 100644 --- a/python/packages/jumpstarter-driver-adb/jumpstarter_driver_adb/driver.py +++ b/python/packages/jumpstarter-driver-adb/jumpstarter_driver_adb/driver.py @@ -1,3 +1,4 @@ +import math import os import shutil import subprocess @@ -21,7 +22,7 @@ class AdbServer(TcpNetwork): adb_path: str = "adb" host: str = "127.0.0.1" port: int = 15037 - + connect_timeout: float = 30.0 @classmethod def client(cls) -> str: return "jumpstarter_driver_adb.client.AdbClient" @@ -35,6 +36,14 @@ def __post_init__(self): if self.port < 1 or self.port > 65535: raise ConfigurationError(f"Invalid port number: {self.port}") + if ( + isinstance(self.connect_timeout, bool) + or not isinstance(self.connect_timeout, (int, float)) + or not math.isfinite(self.connect_timeout) + or self.connect_timeout <= 0 + ): + raise ConfigurationError(f"connect_timeout must be a positive number: {self.connect_timeout}") + # Resolve adb binary if self.adb_path == "adb": resolved = shutil.which("adb") @@ -59,6 +68,7 @@ def __post_init__(self): self.start_server() self.logger.info(f"ADB server running on {self.host}:{self.port}") + def close(self): self.kill_server() @@ -106,6 +116,67 @@ def kill_server(self) -> int: self.logger.error(f"Failed to kill ADB server: {e}") return self.port + def _connect_device(self, device: str) -> str: + self.logger.info(f"Connecting to device {device}") + try: + result = subprocess.run( + [self.adb_path, "connect", device], + check=True, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + text=True, + env=self.adb_env(), + timeout=self.connect_timeout, + ) + output = result.stdout.strip() + self.logger.info(output) + return output + except subprocess.TimeoutExpired as e: + self.logger.error(f"Timed out connecting to device {device} after {self.connect_timeout}s") + raise TimeoutError(f"adb connect {device} timed out after {self.connect_timeout}s") from e + except subprocess.CalledProcessError as e: + stderr = (e.stderr or "").strip() + self.logger.error(f"Failed to connect to device {device}: {stderr or e}") + raise + + @export + def connect_device(self, device: str) -> str: + """Connect to an ADB device by address (host:port). + + Raises on failure or timeout so callers can react instead of + silently receiving an error string. + """ + return self._connect_device(device) + + @export + def disconnect_device(self, device: str) -> str: + """Disconnect an ADB device by address (host:port). + + Raises on failure or timeout so callers can react instead of + silently receiving an error string. + """ + self.logger.info(f"Disconnecting device {device}") + try: + result = subprocess.run( + [self.adb_path, "disconnect", device], + check=True, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + text=True, + env=self.adb_env(), + timeout=self.connect_timeout, + ) + output = result.stdout.strip() + self.logger.info(output) + return output + except subprocess.TimeoutExpired as e: + self.logger.error(f"Timed out disconnecting device {device} after {self.connect_timeout}s") + raise TimeoutError(f"adb disconnect {device} timed out after {self.connect_timeout}s") from e + except subprocess.CalledProcessError as e: + stderr = (e.stderr or "").strip() + self.logger.error(f"Failed to disconnect device {device}: {stderr or e}") + raise + @export def list_devices(self) -> str: """List devices visible to the exporter's ADB server.""" diff --git a/python/packages/jumpstarter-driver-adb/jumpstarter_driver_adb/driver_test.py b/python/packages/jumpstarter-driver-adb/jumpstarter_driver_adb/driver_test.py index e4473f78c..1112524a3 100644 --- a/python/packages/jumpstarter-driver-adb/jumpstarter_driver_adb/driver_test.py +++ b/python/packages/jumpstarter-driver-adb/jumpstarter_driver_adb/driver_test.py @@ -1,3 +1,4 @@ +import subprocess from unittest.mock import MagicMock, patch import pytest @@ -39,6 +40,13 @@ def test_invalid_port_too_high(): AdbServer(port=70000) +@pytest.mark.parametrize("bad", [0, -1, float("inf"), float("nan"), True, "30", None]) +@patch("shutil.which", return_value="/usr/bin/adb") +def test_invalid_connect_timeout(_, bad): + with pytest.raises(ConfigurationError, match="connect_timeout"): + AdbServer(connect_timeout=bad) + + @patch("shutil.which", return_value="/usr/bin/adb") @patch("subprocess.run", return_value=_mock_adb_ok()) def test_start_server(mock_run, _): @@ -80,3 +88,99 @@ def test_list_devices(mock_run, _): def test_custom_port(mock_run, _): server = AdbServer(port=5038) assert server.port == 5038 + + +@patch("shutil.which", return_value="/usr/bin/adb") +@patch("subprocess.run", return_value=_mock_adb_ok()) +def test_init_no_auto_connect(mock_run, _): + AdbServer() + assert mock_run.call_count == 2 # version + start-server only + + +@patch("shutil.which", return_value="/usr/bin/adb") +@patch("subprocess.run") +def test_connect_device(mock_run, _): + mock_run.side_effect = [ + _mock_adb_ok(), # version check + _mock_adb_ok(), # start-server + MagicMock(stdout="connected to 10.0.0.1:6520\n", stderr="", returncode=0), + ] + server = AdbServer() + mock_run.reset_mock() + # reset_mock() does not clear side_effect; clear it so return_value is used. + mock_run.side_effect = None + mock_run.return_value = MagicMock(stdout="connected to 10.0.0.2:6520\n", stderr="", returncode=0) + result = server.connect_device("10.0.0.2:6520") + assert result == "connected to 10.0.0.2:6520" + assert mock_run.call_args[0][0] == ["/usr/bin/adb", "connect", "10.0.0.2:6520"] + + +@patch("shutil.which", return_value="/usr/bin/adb") +@patch("subprocess.run") +def test_connect_device_error(mock_run, _): + mock_run.side_effect = [ + _mock_adb_ok(), # version check + _mock_adb_ok(), # start-server + ] + server = AdbServer() + mock_run.side_effect = subprocess.CalledProcessError(1, "adb connect") + with pytest.raises(subprocess.CalledProcessError): + server.connect_device("bad:99") + + +@patch("shutil.which", return_value="/usr/bin/adb") +@patch("subprocess.run") +def test_connect_device_timeout(mock_run, _): + mock_run.side_effect = [ + _mock_adb_ok(), # version check + _mock_adb_ok(), # start-server + ] + server = AdbServer() + mock_run.side_effect = subprocess.TimeoutExpired("adb connect", 30.0) + with pytest.raises(TimeoutError): + server.connect_device("bad:99") + assert mock_run.call_args[0][0] == ["/usr/bin/adb", "connect", "bad:99"] + assert mock_run.call_args[1]["timeout"] == server.connect_timeout + + +@patch("shutil.which", return_value="/usr/bin/adb") +@patch("subprocess.run") +def test_disconnect_device(mock_run, _): + mock_run.side_effect = [ + _mock_adb_ok(), # version check + _mock_adb_ok(), # start-server + ] + server = AdbServer() + mock_run.side_effect = None + mock_run.return_value = MagicMock(stdout="disconnected 10.0.0.1:6520\n", stderr="", returncode=0) + result = server.disconnect_device("10.0.0.1:6520") + assert "disconnected" in result + assert mock_run.call_args[0][0] == ["/usr/bin/adb", "disconnect", "10.0.0.1:6520"] + + +@patch("shutil.which", return_value="/usr/bin/adb") +@patch("subprocess.run") +def test_disconnect_device_error(mock_run, _): + mock_run.side_effect = [ + _mock_adb_ok(), # version check + _mock_adb_ok(), # start-server + ] + server = AdbServer() + mock_run.side_effect = subprocess.CalledProcessError(1, "adb disconnect") + with pytest.raises(subprocess.CalledProcessError): + server.disconnect_device("bad:99") + + +@patch("shutil.which", return_value="/usr/bin/adb") +@patch("subprocess.run") +def test_disconnect_device_timeout(mock_run, _): + mock_run.side_effect = [ + _mock_adb_ok(), # version check + _mock_adb_ok(), # start-server + ] + server = AdbServer() + mock_run.side_effect = subprocess.TimeoutExpired("adb disconnect", 30.0) + with pytest.raises(TimeoutError): + server.disconnect_device("bad:99") + assert mock_run.call_args[0][0] == ["/usr/bin/adb", "disconnect", "bad:99"] + assert mock_run.call_args[1]["timeout"] == server.connect_timeout