Skip to content
Open
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/jumpstarter-all/pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ dependencies = [
"jumpstarter-cli-admin",
"jumpstarter-cli-common",
"jumpstarter-cli-driver",
"jumpstarter-driver-android",
"jumpstarter-driver-can",
"jumpstarter-driver-composite",
"jumpstarter-driver-corellium",
Expand Down
25 changes: 25 additions & 0 deletions packages/jumpstarter-driver-android/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
# Android Driver

`jumpstarter-driver-android` provides ADB and Android emulator functionality for Jumpstarter.

## Installation

```bash
pip install jumpstarter-driver-android
```

## Configuration

Example configuration:

```yaml
export:
composite:
type: jumpstarter_driver_android.driver.Adb
config:
# Add required config parameters here
```

## API Reference

Add API documentation here.
Original file line number Diff line number Diff line change
@@ -0,0 +1,229 @@
import errno
import os
import socket
import subprocess
import sys
from contextlib import contextmanager
from threading import Event
from typing import Generator

import adbutils
import asyncclick as click
from jumpstarter_driver_composite.client import CompositeClient
from jumpstarter_driver_network.adapters import TcpPortforwardAdapter

from jumpstarter.client import DriverClient


class AdbClientBase(DriverClient):
"""
Base class for ADB clients. This class provides a context manager to
create an ADB client and forward the ADB server address and port.
"""

def _check_port_in_use(self, host: str, port: int) -> bool:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function is unused.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should also be removed after some experimentation, not reliable across platforms.

# Check if port is already bound
sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
try:
sock.bind((host, port))
except socket.error as e:
if e.errno == errno.EADDRINUSE:
return True
finally:
sock.close()
return False

@contextmanager
def forward_adb(self, host: str, port: int) -> Generator[str, None, None]:
"""
Port-forward remote ADB server to local host and port.
If the port is already bound, yields the existing address instead.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not yet implemented?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, this was an attempt to fix port issues, should be removed.


Args:
host (str): The local host to forward to.
port (int): The local port to forward to.

Yields:
str: The address of the forwarded ADB server.
"""
with TcpPortforwardAdapter(
client=self,
local_host=host,
local_port=port,
) as addr:
yield addr

@contextmanager
def adb_client(self, host: str = "127.0.0.1", port: int = 5038) -> Generator[adbutils.AdbClient, None, None]:
"""
Context manager to get an `adbutils.AdbClient`.

Args:
host (str): The local host to forward to.
port (int): The local port to forward to.

Yields:
adbutils.AdbClient: The `adbutils.AdbClient` instance.
"""
with self.forward_adb(host, port) as addr:
client = adbutils.AdbClient(host=addr[0], port=int(addr[1]))
yield client


class AdbClient(AdbClientBase):
"""Power client for controlling power devices."""

def cli(self):
@click.command(context_settings={"ignore_unknown_options": True})
@click.option("host", "-H", default="127.0.0.1", show_default=True, help="Local adb host to forward to.")
@click.option("port", "-P", type=int, default=5038, show_default=True, help="Local adb port to forward to.")
@click.option("-a", is_flag=True, hidden=True)
@click.option("-d", is_flag=True, hidden=True)
@click.option("-e", is_flag=True, hidden=True)
@click.option("-L", hidden=True)
@click.option("--one-device", hidden=True)
@click.option(
"--adb",
default="adb",
show_default=True,
help="Path to the ADB executable",
)
@click.argument("args", nargs=-1)
def adb(
host: str,
port: int,
adb: str,
a: bool,
d: bool,
e: bool,
l: str, # noqa: E741
one_device: str,
args: tuple[str, ...],
):
"""
Run commands using a local adb executable against the remote adb server. This command is a wrapper around
the adb command-line tool. It allows you to run adb commands against a remote ADB server tunneled through
Jumpstarter.

When executing this command, the adb server address and port are forwarded to the local ADB executable. The
adb server address and port are set in the environment variables ANDROID_ADB_SERVER_ADDRESS and
ANDROID_ADB_SERVER_PORT, respectively. This allows the local ADB executable to communicate with the remote
adb server.

Most command line arguments and commands are passed directly to the adb executable. However, some
arguments and commands are not supported by the Jumpstarter adb client. These options include:
-a, -d, -e, -L, --one-device.

The following adb commands are also not supported: connect, disconnect,
reconnect, nodaemon, pair
"""
# Throw exception for all unsupported arguments
if any([a, d, e, l, one_device]):
raise click.UsageError(
"ADB options -a, -d, -e, -L, and --one-device are not supported by the Jumpstarter ADB client"
)
# Check for unsupported server management commands
unsupported_commands = [
"connect",
"disconnect",
"reconnect",
"nodaemon",
"pair",
]
for arg in args:
if arg in unsupported_commands:
raise click.UsageError(f"The adb command '{arg}' is not supported by the Jumpstarter ADB client")

if "start-server" in args:
remote_port = int(self.call("start_server"))
click.echo(f"Remote adb server started on remote port exporter:{remote_port}")
return 0
elif "kill-server" in args:
remote_port = int(self.call("kill_server"))
click.echo(f"Remote adb server killed on remote port exporter:{remote_port}")
return 0
elif "forward-adb" in args:
# Port is available, proceed with forwarding
with self.forward_adb(host, port) as addr:
click.echo(f"Remote adb server forwarded to {addr[0]}:{addr[1]}")
Event().wait()

# Forward the ADB server address and port and call ADB executable with args
with self.forward_adb(host, port) as addr:
env = os.environ | {
"ANDROID_ADB_SERVER_ADDRESS": addr[0],
"ANDROID_ADB_SERVER_PORT": str(addr[1]),
}
cmd = [adb, *args]
process = subprocess.Popen(cmd, stdin=sys.stdin, stdout=sys.stdout, stderr=sys.stderr, env=env)
return process.wait()

Comment on lines +162 to +171
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Use proper subprocess management for resource cleanup.

The subprocess should be managed properly to ensure cleanup even if an exception occurs.

 # Forward the ADB server address and port and call ADB executable with args
 with self.forward_adb(host, port) as addr:
     env = os.environ | {
         "ANDROID_ADB_SERVER_ADDRESS": addr[0],
         "ANDROID_ADB_SERVER_PORT": str(addr[1]),
     }
     cmd = [adb, *args]
-    process = subprocess.Popen(cmd, stdin=sys.stdin, stdout=sys.stdout, stderr=sys.stderr, env=env)
-    return process.wait()
+    with subprocess.Popen(cmd, stdin=sys.stdin, stdout=sys.stdout, stderr=sys.stderr, env=env) as process:
+        return process.wait()
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
# Forward the ADB server address and port and call ADB executable with args
with self.forward_adb(host, port) as addr:
env = os.environ | {
"ANDROID_ADB_SERVER_ADDRESS": addr[0],
"ANDROID_ADB_SERVER_PORT": str(addr[1]),
}
cmd = [adb, *args]
process = subprocess.Popen(cmd, stdin=sys.stdin, stdout=sys.stdout, stderr=sys.stderr, env=env)
return process.wait()
# Forward the ADB server address and port and call ADB executable with args
with self.forward_adb(host, port) as addr:
env = os.environ | {
"ANDROID_ADB_SERVER_ADDRESS": addr[0],
"ANDROID_ADB_SERVER_PORT": str(addr[1]),
}
cmd = [adb, *args]
with subprocess.Popen(cmd, stdin=sys.stdin, stdout=sys.stdout, stderr=sys.stderr, env=env) as process:
return process.wait()
🧰 Tools
🪛 Pylint (3.3.7)

[convention] 169-169: Line too long (111/100)

(C0301)


[refactor] 169-169: Consider using 'with' for resource-allocating operations

(R1732)

🤖 Prompt for AI Agents
In packages/jumpstarter-driver-android/jumpstarter_driver_android/client.py
around lines 162 to 171, the subprocess.Popen call is not managed with a context
that ensures proper cleanup on exceptions. Replace the current subprocess.Popen
usage with a context manager (using subprocess.Popen as a context) or ensure the
process is properly terminated and waited on in a finally block to guarantee
resource cleanup even if an error occurs.

return adb


class ScrcpyClient(AdbClientBase):
"""Scrcpy client for controlling Android devices/emulators."""

def cli(self):
@click.command(context_settings={"ignore_unknown_options": True})
@click.option("host", "-H", default="127.0.0.1", show_default=True, help="Local adb host to forward to.")
@click.option("port", "-P", type=int, default=5038, show_default=True, help="Local adb port to forward to.")
@click.option(
"--scrcpy",
default="scrcpy",
show_default=True,
help="Path to the scrcpy executable",
)
@click.argument("args", nargs=-1)
def scrcpy(
host: str,
port: int,
scrcpy: str,
args: tuple[str, ...],
):
"""
Run scrcpy using a local executable against the remote adb server. This command is a wrapper around
the scrcpy command-line tool. It allows you to run scrcpy against a remote Android device through
an ADB server tunneled via Jumpstarter.

When executing this command, the adb server address and port are forwarded to the local scrcpy executable.
The adb server socket path is set in the environment variable ADB_SERVER_SOCKET, allowing scrcpy to
communicate with the remote adb server.

Most command line arguments are passed directly to the scrcpy executable.
"""
# Unsupported scrcpy arguments that depend on direct adb server management
unsupported_args = [
"--connect",
"-c",
"--serial",
"-s",
"--select-usb",
"--select-tcpip",
]

for arg in args:
for unsupported in unsupported_args:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or print a warning and ignore the argument? Then the same scrcpy invocation can be used with or without jumpstarter.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These arguments don't really work when using remote scrcpy which itself is also an adb wrapper 🙄. I can switch this to warnings instead.

if arg.startswith(unsupported):
raise click.UsageError(
f"Scrcpy argument '{unsupported}' is not supported by the Jumpstarter scrcpy client"
)

# Forward the ADB server address and port and call scrcpy executable with args
with self.forward_adb(host, port) as addr:
# Scrcpy uses ADB_SERVER_SOCKET environment variable
socket_path = f"tcp:{addr[0]}:{addr[1]}"
env = os.environ | {
"ADB_SERVER_SOCKET": socket_path,
}
cmd = [scrcpy, *args]
process = subprocess.Popen(cmd, stdin=sys.stdin, stdout=sys.stdout, stderr=sys.stderr, env=env)
return process.wait()

Comment on lines +224 to +234
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Use proper subprocess management for resource cleanup.

Similar to the AdbClient, the subprocess should be properly managed.

 # Forward the ADB server address and port and call scrcpy executable with args
 with self.forward_adb(host, port) as addr:
     # Scrcpy uses ADB_SERVER_SOCKET environment variable
     socket_path = f"tcp:{addr[0]}:{addr[1]}"
     env = os.environ | {
         "ADB_SERVER_SOCKET": socket_path,
     }
     cmd = [scrcpy, *args]
-    process = subprocess.Popen(cmd, stdin=sys.stdin, stdout=sys.stdout, stderr=sys.stderr, env=env)
-    return process.wait()
+    with subprocess.Popen(cmd, stdin=sys.stdin, stdout=sys.stdout, stderr=sys.stderr, env=env) as process:
+        return process.wait()
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
# Forward the ADB server address and port and call scrcpy executable with args
with self.forward_adb(host, port) as addr:
# Scrcpy uses ADB_SERVER_SOCKET environment variable
socket_path = f"tcp:{addr[0]}:{addr[1]}"
env = os.environ | {
"ADB_SERVER_SOCKET": socket_path,
}
cmd = [scrcpy, *args]
process = subprocess.Popen(cmd, stdin=sys.stdin, stdout=sys.stdout, stderr=sys.stderr, env=env)
return process.wait()
# Forward the ADB server address and port and call scrcpy executable with args
with self.forward_adb(host, port) as addr:
# Scrcpy uses ADB_SERVER_SOCKET environment variable
socket_path = f"tcp:{addr[0]}:{addr[1]}"
env = os.environ | {
"ADB_SERVER_SOCKET": socket_path,
}
cmd = [scrcpy, *args]
with subprocess.Popen(cmd, stdin=sys.stdin, stdout=sys.stdout, stderr=sys.stderr, env=env) as process:
return process.wait()
🧰 Tools
🪛 Pylint (3.3.7)

[convention] 232-232: Line too long (111/100)

(C0301)


[refactor] 232-232: Consider using 'with' for resource-allocating operations

(R1732)

🤖 Prompt for AI Agents
In packages/jumpstarter-driver-android/jumpstarter_driver_android/client.py
around lines 224 to 234, the subprocess.Popen call is not properly managed,
which can lead to resource leaks. Replace the subprocess.Popen and
process.wait() with a subprocess.run call using the same command, stdin, stdout,
stderr, and env parameters to ensure the subprocess is properly waited on and
resources are cleaned up automatically.

return scrcpy


class AndroidClient(CompositeClient):
"""Generic Android client for controlling Android devices/emulators."""

pass
Empty file.
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
from .adb import AdbServer
from .emulator import AndroidEmulator, AndroidEmulatorPower
from .options import AdbOptions, EmulatorOptions
from .scrcpy import Scrcpy

__all__ = [
"AdbServer",
"AndroidEmulator",
"AndroidEmulatorPower",
"AdbOptions",
"EmulatorOptions",
"Scrcpy",
]
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
import os
import shutil
import subprocess
from dataclasses import dataclass
from typing import override

from jumpstarter_driver_network.driver import TcpNetwork

from jumpstarter.common.exceptions import ConfigurationError
from jumpstarter.driver.decorators import export


@dataclass(kw_only=True)
class AdbServer(TcpNetwork):
adb_path: str = "adb"
host: str = "127.0.0.1"
port: int = 5037

@classmethod
@override
def client(cls) -> str:
return "jumpstarter_driver_android.client.AdbClient"

def _print_output(self, output: str, error=False, debug=False):
if output:
for line in output.strip().split("\n"):
if error:
self.logger.error(line)
elif debug:
self.logger.debug(line)
else:
self.logger.info(line)

@export
def start_server(self):
"""
Start the ADB server.
"""
self.logger.debug(f"Starting ADB server on port {self.port}")
try:
result = subprocess.run(
[self.adb_path, "start-server"],
check=True,
stdout=subprocess.PIPE,
stderr=subprocess.PIPE,
text=True,
env={"ANDROID_ADB_SERVER_PORT": str(self.port), **dict(os.environ)},
)
self._print_output(result.stdout)
self._print_output(result.stderr, debug=True)
self.logger.info(f"ADB server started on port {self.port}")
except subprocess.CalledProcessError as e:
self._print_output(e.stdout)
self._print_output(e.stderr, debug=True)
self.logger.error(f"Failed to start ADB server: {e}")
return self.port

@export
def kill_server(self):
"""
Kill the ADB server.
"""
self.logger.debug(f"Killing ADB server on port {self.port}")
try:
result = subprocess.run(
[self.adb_path, "kill-server"],
check=True,
stdout=subprocess.PIPE,
stderr=subprocess.PIPE,
text=True,
env={"ANDROID_ADB_SERVER_PORT": str(self.port), **dict(os.environ)},
)
self._print_output(result.stdout)
self._print_output(result.stderr, error=True)
self.logger.info(f"ADB server stopped on port {self.port}")
except subprocess.CalledProcessError as e:
self._print_output(e.stdout)
self._print_output(e.stderr, error=True)
self.logger.error(f"Failed to stop ADB server: {e}")
except Exception as e:
self.logger.error(f"Unexpected error while stopping ADB server: {e}")
Comment on lines +78 to +79
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Avoid catching broad Exception.

Catching the generic Exception class can hide unexpected errors and make debugging difficult. Consider catching more specific exceptions or re-raising after logging.

-        except Exception as e:
-            self.logger.error(f"Unexpected error while stopping ADB server: {e}")
+        except (OSError, ValueError) as e:
+            self.logger.error(f"Unexpected error while stopping ADB server: {e}")
+            raise
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
except Exception as e:
self.logger.error(f"Unexpected error while stopping ADB server: {e}")
except (OSError, ValueError) as e:
self.logger.error(f"Unexpected error while stopping ADB server: {e}")
raise
🧰 Tools
🪛 Pylint (3.3.7)

[warning] 78-78: Catching too general exception Exception

(W0718)

🤖 Prompt for AI Agents
In packages/jumpstarter-driver-android/jumpstarter_driver_android/driver/adb.py
around lines 78 to 79, avoid catching the broad Exception class as it can
obscure unexpected errors. Identify and catch more specific exceptions related
to stopping the ADB server, such as subprocess or OS-related exceptions. After
logging the error, re-raise the exception to ensure it is not silently
swallowed, aiding in debugging and error handling.

return self.port

def __post_init__(self):
if hasattr(super(), "__post_init__"):
super().__post_init__()

if self.port < 0 or self.port > 65535:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These validation logic can be handled by pydantic, by replacing the builtin dataclass implementation with the pydantic one: https://docs.pydantic.dev/latest/concepts/dataclasses/

raise ConfigurationError(f"Invalid port number: {self.port}")
if not isinstance(self.port, int):
raise ConfigurationError(f"Port must be an integer: {self.port}")

self.logger.info(f"ADB server will run on port {self.port}")

if self.adb_path == "adb":
self.adb_path = shutil.which("adb")
if not self.adb_path:
raise ConfigurationError(f"ADB executable '{self.adb_executable}' not found in PATH.")

try:
result = subprocess.run(
[self.adb_path, "version"], check=True, stdout=subprocess.PIPE, stderr=subprocess.PIPE, text=True
)
self._print_output(result.stdout, debug=True)
except subprocess.CalledProcessError as e:
self.logger.error(f"Failed to execute adb: {e}")

# def close(self):
# self.kill_server()
Loading
Loading