From 88695a4f0c675f98afe24bbeec7ec4d3e31eac1e Mon Sep 17 00:00:00 2001 From: Juan Pablo Pino Bravo Date: Fri, 10 Jul 2026 12:55:55 +0200 Subject: [PATCH 1/7] feat: add `blueye logs` command for listing and downloading dive logs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Surfaces the documented drone.logs workflow as a CLI built-in: - `blueye logs list` — table of the drone's binary logs (name, time, max depth, size, dive flag). - `blueye logs download [NAME ...] [--latest N | --all] [-o DIR]` — downloads .bez files; unknown names error listing what is available. - Bare `blueye logs` on a terminal opens an interactive picker: the table plus a checkbox multi-select and a destination prompt. The Prompter seam gains a checkbox method (questionary.checkbox interactively; a CliError naming the flags non-interactively). - Connects to the drone as an observer (connect_as_observer=True), so no control is taken and no SDK changes were needed; an unreachable drone fails with the friendly message and exit 1. The connection is released on exit. - The --drone-ip/--timeout parent parser and the failure-translation helper are lifted into commands/_common.py, now shared with the models command. Verified against the bench drone (list + `--latest 1` downloaded a real 2.5 MiB .bez) and an unreachable address. +14 tests (432 total); docs gain a "From the command line" section in the logs guide. Co-Authored-By: Claude Fable 5 --- blueye/sdk/cli/commands/__init__.py | 3 +- blueye/sdk/cli/commands/_common.py | 41 +++++ blueye/sdk/cli/commands/logs/__init__.py | 14 ++ blueye/sdk/cli/commands/logs/command.py | 179 ++++++++++++++++++++++ blueye/sdk/cli/commands/models/command.py | 54 +++---- blueye/sdk/cli/prompts.py | 11 +- docs/logs/listing-and-downloading.md | 12 ++ tests/test_cli_logs_command.py | 138 +++++++++++++++++ 8 files changed, 414 insertions(+), 38 deletions(-) create mode 100644 blueye/sdk/cli/commands/_common.py create mode 100644 blueye/sdk/cli/commands/logs/__init__.py create mode 100644 blueye/sdk/cli/commands/logs/command.py create mode 100644 tests/test_cli_logs_command.py diff --git a/blueye/sdk/cli/commands/__init__.py b/blueye/sdk/cli/commands/__init__.py index 48634e3e..246203f0 100644 --- a/blueye/sdk/cli/commands/__init__.py +++ b/blueye/sdk/cli/commands/__init__.py @@ -52,7 +52,8 @@ class CommandSpec: def all_commands() -> tuple[CommandSpec, ...]: """Return every built-in command, in the order shown in ``blueye --help``.""" from .bundle_model import COMMAND as bundle_model_command + from .logs import COMMAND as logs_command from .models import COMMAND as models_command from .tools import COMMAND as tools_command - return (bundle_model_command, models_command, tools_command) + return (bundle_model_command, logs_command, models_command, tools_command) diff --git a/blueye/sdk/cli/commands/_common.py b/blueye/sdk/cli/commands/_common.py new file mode 100644 index 00000000..0c13ce22 --- /dev/null +++ b/blueye/sdk/cli/commands/_common.py @@ -0,0 +1,41 @@ +"""Shared helpers for built-in commands that talk to the drone.""" + +from __future__ import annotations + +import argparse +import logging + +from ..errors import CliError + +logger = logging.getLogger(__name__) + + +def drone_options_parser(timeout_default: float = 5.0) -> argparse.ArgumentParser: + """Build the parent parser carrying the common drone connection options.""" + common = argparse.ArgumentParser(add_help=False) + common.add_argument( + "--drone-ip", default="192.168.1.101", help="Drone address (default: %(default)s)" + ) + common.add_argument( + "--timeout", type=float, default=timeout_default, help="Request timeout in seconds" + ) + return common + + +def friendly_errors(action): + """Run an action, translating transport/API failures into CliErrors.""" + import requests + + try: + return action() + except ( + ConnectionError, # Raised by Drone.connect()/_update_drone_info. + requests.exceptions.ConnectionError, + requests.exceptions.Timeout, + ) as error: + raise CliError( + "Could not reach the drone — is it connected? (Use --drone-ip if it is not " + "at the default address.)" + ) from error + except requests.exceptions.HTTPError as error: + raise CliError(str(error)) from error diff --git a/blueye/sdk/cli/commands/logs/__init__.py b/blueye/sdk/cli/commands/logs/__init__.py new file mode 100644 index 00000000..e033f418 --- /dev/null +++ b/blueye/sdk/cli/commands/logs/__init__.py @@ -0,0 +1,14 @@ +"""The `blueye logs` command: list and download dive logs from the drone.""" + +from __future__ import annotations + +from .. import CommandSpec +from .command import add_parser, run + +COMMAND = CommandSpec( + name="logs", + help="List and download dive logs from the drone", + requires=("rich", "questionary"), + add_parser=add_parser, + run=run, +) diff --git a/blueye/sdk/cli/commands/logs/command.py b/blueye/sdk/cli/commands/logs/command.py new file mode 100644 index 00000000..49efc587 --- /dev/null +++ b/blueye/sdk/cli/commands/logs/command.py @@ -0,0 +1,179 @@ +"""Implementation of the `blueye logs` subcommands. + +Follows the documented log workflow (docs/logs/listing-and-downloading.md): connect to +the drone **as an observer** (taking no control), read the binary log index from +`drone.logs`, and download `.bez` files with `LogFile.download`. Legacy CSV logs are +not covered — use `drone.legacy_logs` from the SDK for those. + +Argument definitions are stdlib-only; rich/questionary/blueye.sdk imports happen +inside `run` (after the dependency gate). +""" + +from __future__ import annotations + +import argparse +import logging +import sys +from pathlib import Path + +from ...errors import CliError +from .._common import drone_options_parser, friendly_errors + +logger = logging.getLogger(__name__) + + +def add_parser(subparsers) -> None: + """Register the ``logs`` subcommand and its sub-subcommands.""" + common = drone_options_parser(timeout_default=10.0) + + parser = subparsers.add_parser( + "logs", + parents=[common], + help="List and download dive logs from the drone", + description=( + "List and download the drone's binary dive logs (.bez). Connects to the " + "drone as an observer, taking no control. Run without an action on a " + "terminal to pick logs interactively." + ), + ) + actions = parser.add_subparsers(dest="logs_command", metavar="ACTION") + + actions.add_parser("list", parents=[common], help="List the logs on the drone") + + download = actions.add_parser("download", parents=[common], help="Download logs from the drone") + download.add_argument("names", nargs="*", help="Log names to download") + download.add_argument( + "-o", "--output", default=".", help="Destination directory (default: current)" + ) + download.add_argument( + "--latest", + type=int, + metavar="N", + help="Download the N most recent logs", + ) + download.add_argument("--all", action="store_true", help="Download every log") + + +def _connect(args): + """Connect to the drone as an observer and return the Drone object.""" + from blueye.sdk import Drone + + return friendly_errors( + lambda: Drone(ip=args.drone_ip, timeout=args.timeout, connect_as_observer=True) + ) + + +def _log_rows(logs) -> list: + """The drone's logs as a list of LogFile objects (index fetched lazily).""" + return friendly_errors(lambda: list(logs)) + + +def _print_logs_table(console, log_files) -> None: + from rich.table import Table + + from blueye.sdk.logs import human_readable_filesize + + table = Table(show_header=True, header_style="bold", box=None, pad_edge=False) + for column in ("NAME", "TIME", "MAX DEPTH", "SIZE", "DIVE"): + table.add_column(column) + for log in log_files: + table.add_row( + log.name, + log.start_time.strftime("%d. %b %Y %H:%M"), + f"{log.max_depth_magnitude} m", + human_readable_filesize(log.filesize), + "yes" if log.is_dive else "[dim]no[/dim]", + ) + console.print(table) + + +def _download_logs(console, log_files, output_dir: Path, timeout: float) -> None: + from blueye.sdk.logs import human_readable_filesize + + output_dir.mkdir(parents=True, exist_ok=True) + for log in log_files: + with console.status(f"[cyan]Downloading {log.name}..."): + friendly_errors(lambda: log.download(output_path=output_dir, timeout=timeout)) + console.print( + f"Downloaded {log.name}.bez ({human_readable_filesize(log.filesize)}) " + f"to {output_dir}" + ) + + +def _select_downloads(args, log_files) -> list: + """Resolve the download selection from names/--latest/--all.""" + by_name = {log.name: log for log in log_files} + if args.all: + return list(log_files) + if args.latest is not None: + newest_first = sorted(log_files, key=lambda log: log.start_time, reverse=True) + return newest_first[: args.latest] + if args.names: + missing = [name for name in args.names if name not in by_name] + if missing: + available = ", ".join(sorted(by_name)) or "none" + raise CliError( + f"No log named {', '.join(missing)} on the drone (available: {available})." + ) + return [by_name[name] for name in args.names] + raise CliError("Nothing selected — pass log names, --latest N, or --all.") + + +def _run_interactive(console, args, prompter, drone) -> int: + """Show the table, pick logs with a checkbox, download to a chosen directory.""" + log_files = _log_rows(drone.logs) + if not log_files: + console.print("No logs on the drone.") + return 0 + _print_logs_table(console, log_files) + + from blueye.sdk.logs import human_readable_filesize + + by_label = { + f"{log.name} {log.start_time.strftime('%d. %b %Y %H:%M')} " + f"{human_readable_filesize(log.filesize)}": log + for log in log_files + } + selected = prompter.checkbox("Select logs to download:", list(by_label), "--latest/--all") + if not selected: + console.print("Nothing selected.") + return 0 + output_dir = Path(prompter.text("Download to directory:", ".", "--output")).expanduser() + _download_logs(console, [by_label[label] for label in selected], output_dir, args.timeout) + return 0 + + +def run(args: argparse.Namespace) -> int: + """Dispatch the logs sub-subcommand.""" + from ... import prompts, ui + + console = ui.make_console() + action = getattr(args, "logs_command", None) + + drone = _connect(args) + try: + if action == "download": + log_files = _log_rows(drone.logs) + selection = _select_downloads(args, log_files) + _download_logs(console, selection, Path(args.output).expanduser(), args.timeout) + return 0 + + if action is None and sys.stdin.isatty() and sys.stdout.isatty(): + try: + return _run_interactive(console, args, prompts.QuestionaryPrompter(), drone) + except prompts.PromptAborted: + console.print("[yellow]Cancelled.[/yellow]") + return 130 + + # `logs list` and non-TTY bare invocation. + log_files = _log_rows(drone.logs) + if not log_files: + console.print("No logs on the drone.") + return 0 + _print_logs_table(console, log_files) + return 0 + finally: + try: + drone.disconnect() + except Exception: # Never let cleanup mask the real outcome. + logger.debug("Failed to disconnect cleanly", exc_info=True) diff --git a/blueye/sdk/cli/commands/models/command.py b/blueye/sdk/cli/commands/models/command.py index 67707629..72798749 100644 --- a/blueye/sdk/cli/commands/models/command.py +++ b/blueye/sdk/cli/commands/models/command.py @@ -17,6 +17,7 @@ from pathlib import Path from ...errors import CliError +from .._common import drone_options_parser, friendly_errors logger = logging.getLogger(__name__) @@ -26,11 +27,7 @@ def add_parser(subparsers) -> None: """Register the ``models`` subcommand and its sub-subcommands.""" - common = argparse.ArgumentParser(add_help=False) - common.add_argument( - "--drone-ip", default="192.168.1.101", help="Drone address (default: %(default)s)" - ) - common.add_argument("--timeout", type=float, default=5.0, help="Request timeout in seconds") + common = drone_options_parser(timeout_default=5.0) parser = subparsers.add_parser( "models", @@ -100,21 +97,6 @@ def _cv_models(args): return Drone(ip=args.drone_ip, auto_connect=False).cv_models -def _friendly_errors(action): - """Run an action, translating transport/API failures into CliErrors.""" - import requests - - try: - return action() - except (requests.exceptions.ConnectionError, requests.exceptions.Timeout) as error: - raise CliError( - "Could not reach the drone — is it connected? (Use --drone-ip if it is not " - "at the default address.)" - ) from error - except requests.exceptions.HTTPError as error: - raise CliError(str(error)) from error - - def _runtime_field(model, key: str, default: str = "-") -> str: value = model.raw.get("runtime", {}).get(key) return str(value) if value is not None else default @@ -143,7 +125,7 @@ def _print_models_table(console, models) -> None: def _run_list(console, args) -> int: - models = _friendly_errors(lambda: _cv_models(args).list(timeout=args.timeout)) + models = friendly_errors(lambda: _cv_models(args).list(timeout=args.timeout)) if not models: console.print("No CV models installed on the drone.") return 0 @@ -155,7 +137,7 @@ def _run_interactive(console, args, prompter) -> int: """Interactive management loop: pick a model, pick an action, repeat.""" cv_models = _cv_models(args) while True: - models = _friendly_errors(lambda: cv_models.list(timeout=args.timeout)) + models = friendly_errors(lambda: cv_models.list(timeout=args.timeout)) if not models: console.print("No CV models installed on the drone.") return 0 @@ -180,7 +162,7 @@ def _run_interactive(console, args, prompter) -> int: "ACTION", ) if action == toggle: - _friendly_errors( + friendly_errors( lambda: cv_models.set_enabled( model.directory, not model.enabled, timeout=args.timeout ) @@ -189,7 +171,7 @@ def _run_interactive(console, args, prompter) -> int: device = prompter.select( "Execution device:", list(_DEVICES), _runtime_field(model, "device"), "ACTION" ) - _friendly_errors( + friendly_errors( lambda: cv_models.set_device(model.directory, device, timeout=args.timeout) ) elif action == "Set rate": @@ -200,15 +182,15 @@ def _run_interactive(console, args, prompter) -> int: "ACTION", ) hz = 0 if rate.startswith("max") else int(rate) - _friendly_errors(lambda: cv_models.set_hz(model.directory, hz, timeout=args.timeout)) + friendly_errors(lambda: cv_models.set_hz(model.directory, hz, timeout=args.timeout)) elif action == "Warm up": with console.status( f"[cyan]Warming up '{model.directory}' (TensorRT builds can take minutes)..." ): - _friendly_errors(lambda: cv_models.warmup(model.directory)) + friendly_errors(lambda: cv_models.warmup(model.directory)) elif action == "Delete": if prompter.confirm(f"Delete '{model.directory}' from the drone?", False, "--force"): - _friendly_errors(lambda: cv_models.delete(model.directory, timeout=args.timeout)) + friendly_errors(lambda: cv_models.delete(model.directory, timeout=args.timeout)) def run(args: argparse.Namespace) -> int: @@ -232,23 +214,23 @@ def run(args: argparse.Namespace) -> int: cv_models = _cv_models(args) if action == "enable": - _friendly_errors(lambda: cv_models.set_enabled(args.name, True, timeout=args.timeout)) + friendly_errors(lambda: cv_models.set_enabled(args.name, True, timeout=args.timeout)) console.print(f"Enabled autolaunch for '{args.name}'.") elif action == "disable": - _friendly_errors(lambda: cv_models.set_enabled(args.name, False, timeout=args.timeout)) + friendly_errors(lambda: cv_models.set_enabled(args.name, False, timeout=args.timeout)) console.print(f"Disabled autolaunch for '{args.name}'.") elif action == "set-device": - _friendly_errors(lambda: cv_models.set_device(args.name, args.device, timeout=args.timeout)) + friendly_errors(lambda: cv_models.set_device(args.name, args.device, timeout=args.timeout)) console.print(f"'{args.name}' now runs on {args.device}.") elif action == "set-hz": - _friendly_errors(lambda: cv_models.set_hz(args.name, args.hz, timeout=args.timeout)) + friendly_errors(lambda: cv_models.set_hz(args.name, args.hz, timeout=args.timeout)) rate = "unlimited" if args.hz == 0 else f"{args.hz} Hz" console.print(f"'{args.name}' rate set to {rate}.") elif action == "warmup": with console.status( f"[cyan]Warming up '{args.name}' (TensorRT builds can take minutes)..." ): - _friendly_errors(lambda: cv_models.warmup(args.name)) + friendly_errors(lambda: cv_models.warmup(args.name)) console.print(f"Warmup of '{args.name}' complete.") elif action == "delete": if not args.force: @@ -261,22 +243,22 @@ def run(args: argparse.Namespace) -> int: f"Delete '{args.name}' from the drone?", False, "--force" ): return 1 - _friendly_errors(lambda: cv_models.delete(args.name, timeout=args.timeout)) + friendly_errors(lambda: cv_models.delete(args.name, timeout=args.timeout)) console.print(f"Deleted '{args.name}' from the drone.") elif action == "upload": package = Path(args.package).expanduser() if not package.is_file(): raise CliError(f"No such file: {package}") with console.status("[cyan]Uploading to the drone..."): - model = _friendly_errors(lambda: cv_models.upload(package)) + model = friendly_errors(lambda: cv_models.upload(package)) state = "enabled" if model.enabled else "disabled" console.print(f"Uploaded '{model.name}' as '{model.directory}' (autolaunch {state}).") elif action == "download": output = Path(args.output).expanduser() if args.output else None with console.status("[cyan]Downloading from the drone..."): - path = _friendly_errors(lambda: cv_models.download(args.name, output_path=output)) + path = friendly_errors(lambda: cv_models.download(args.name, output_path=output)) console.print(f"Downloaded '{args.name}' to {path}.") elif action == "rescan": - _friendly_errors(lambda: cv_models.rescan(timeout=args.timeout)) + friendly_errors(lambda: cv_models.rescan(timeout=args.timeout)) console.print("Rescan triggered.") return 0 diff --git a/blueye/sdk/cli/prompts.py b/blueye/sdk/cli/prompts.py index 7715f36f..aeef060f 100644 --- a/blueye/sdk/cli/prompts.py +++ b/blueye/sdk/cli/prompts.py @@ -22,7 +22,7 @@ class PromptAborted(Exception): class Prompter(Protocol): - """The questions the bundler can ask. Implementations decide how.""" + """The questions the CLI commands can ask. Implementations decide how.""" def select( self, question: str, choices: Sequence[str], default: str | None, flag: str @@ -34,6 +34,8 @@ def confirm(self, question: str, default: bool, flag: str) -> bool: ... def path(self, question: str, default: str | None, flag: str) -> str: ... + def checkbox(self, question: str, choices: Sequence[str], flag: str) -> list[str]: ... + def _require(answer: object) -> object: """Translate questionary's None (Ctrl+C) into PromptAborted.""" @@ -68,6 +70,10 @@ def confirm(self, question: str, default: bool, flag: str) -> bool: def path(self, question: str, default: str | None, flag: str) -> str: return str(_require(questionary.path(question, default=default or "").ask())) + def checkbox(self, question: str, choices: Sequence[str], flag: str) -> list[str]: + answer = _require(questionary.checkbox(question, choices=list(choices)).ask()) + return [str(item) for item in answer] + class NonInteractivePrompter: """Prompt resolution for ``--yes`` runs and non-TTY environments. @@ -96,3 +102,6 @@ def path(self, question: str, default: str | None, flag: str) -> str: if not default: raise CliError(f"Cannot answer '{question}' non-interactively — pass {flag}.") return default + + def checkbox(self, question: str, choices: Sequence[str], flag: str) -> list[str]: + raise CliError(f"Cannot answer '{question}' non-interactively — pass {flag}.") diff --git a/docs/logs/listing-and-downloading.md b/docs/logs/listing-and-downloading.md index eaa86998..37cc0981 100644 --- a/docs/logs/listing-and-downloading.md +++ b/docs/logs/listing-and-downloading.md @@ -6,6 +6,18 @@ When the drone is powered on a new log file is created, where it stores telemetr Every entry in the binary log is a [BinlogRecord][blueye.protocol.types.message_formats.BinlogRecord] Protobuf message, which in turn contains a unix timestamp in UTC, the monotonic timestamp (time since boot), and an Any message wrapping the Blueye telemetry message. The telemetry messages are documented in the [telemetry proto][blueye.protocol.types.telemetry]. +## From the command line + +The binary logs are also available through the `blueye` CLI (installed with the +SDK's `[cli]` extra), which connects to the drone as an observer — taking no control: + +```shell +blueye logs list # table of logs on the drone +blueye logs download --latest 1 # newest log to the current directory +blueye logs download BYEDP000000_ea9ac92e1817a1d4_00002 -o ~/dives +blueye logs # interactive: pick logs to download +``` + ## Listing the log files If your drone has completed 5 dives and you do diff --git a/tests/test_cli_logs_command.py b/tests/test_cli_logs_command.py new file mode 100644 index 00000000..e3934bd7 --- /dev/null +++ b/tests/test_cli_logs_command.py @@ -0,0 +1,138 @@ +import pytest + +from blueye.sdk.cli.main import main +from blueye.sdk.logs import LogFile + + +def make_log(name: str, start_time: int, filesize: int = 2048, is_dive: bool = True) -> LogFile: + return LogFile( + name=name, + is_dive=is_dive, + filesize=filesize, + start_time=start_time, + max_depth_magnitude=20, + ip="192.168.1.101", + ) + + +@pytest.fixture +def drone(mocker, monkeypatch): + """Mocked Drone with two real LogFile objects; download patched out.""" + monkeypatch.setenv("COLUMNS", "200") + logs = [ + make_log("BYEDP000000_aaaa_00000", start_time=1700000000), + make_log("BYEDP000000_aaaa_00001", start_time=1700100000, is_dive=False), + ] + mocker.patch.object(LogFile, "download", autospec=True, return_value=b"") + drone_cls = mocker.patch("blueye.sdk.Drone", autospec=True) + instance = drone_cls.return_value + instance.logs = logs # `list(drone.logs)` works on a plain list. + instance._logs = {log.name: log for log in logs} + instance._drone_cls = drone_cls + return instance + + +class TestList: + def test_list_renders_table(self, drone, capsys): + assert main(["logs", "list"]) == 0 + out = capsys.readouterr().out + assert "BYEDP000000_aaaa_00000" in out + assert "20 m" in out + assert "2.0 KiB" in out + assert "yes" in out and "no" in out + + def test_connects_as_observer_and_disconnects(self, drone): + assert main(["logs", "list"]) == 0 + kwargs = drone._drone_cls.call_args.kwargs + assert kwargs["connect_as_observer"] is True + assert kwargs["ip"] == "192.168.1.101" + drone.disconnect.assert_called_once() + + def test_drone_ip_flag(self, drone): + assert main(["logs", "list", "--drone-ip", "192.168.1.42"]) == 0 + assert drone._drone_cls.call_args.kwargs["ip"] == "192.168.1.42" + + def test_empty_logs(self, drone, capsys): + drone.logs = [] + assert main(["logs", "list"]) == 0 + assert "No logs" in capsys.readouterr().out + + def test_bare_invocation_without_tty_lists(self, drone, mocker, capsys): + mocker.patch("sys.stdin.isatty", return_value=False) + assert main(["logs"]) == 0 + assert "BYEDP000000_aaaa_00000" in capsys.readouterr().out + + +class TestDownload: + def test_download_by_name(self, drone, tmp_path): + assert main(["logs", "download", "BYEDP000000_aaaa_00000", "-o", str(tmp_path)]) == 0 + LogFile.download.assert_called_once() + call = LogFile.download.call_args + assert call.args[0].name == "BYEDP000000_aaaa_00000" + assert call.kwargs["output_path"] == tmp_path + + def test_unknown_name_lists_available(self, drone, capsys): + assert main(["logs", "download", "nope"]) == 1 + err = capsys.readouterr().err + assert "No log named nope" in err + assert "BYEDP000000_aaaa_00000" in err + + def test_latest_picks_newest(self, drone, tmp_path): + assert main(["logs", "download", "--latest", "1", "-o", str(tmp_path)]) == 0 + call = LogFile.download.call_args + assert call.args[0].name == "BYEDP000000_aaaa_00001" # newer start_time + + def test_all_downloads_everything(self, drone, tmp_path): + assert main(["logs", "download", "--all", "-o", str(tmp_path)]) == 0 + assert LogFile.download.call_count == 2 + + def test_no_selector_errors(self, drone, capsys): + assert main(["logs", "download"]) == 1 + assert "--latest" in capsys.readouterr().err + + +class TestFailureHandling: + def test_unreachable_drone_is_friendly(self, drone, capsys): + drone._drone_cls.side_effect = ConnectionError("Could not establish connection with drone") + assert main(["logs", "list"]) == 1 + err = capsys.readouterr().err + assert "Could not reach the drone" in err + assert "Traceback" not in err + + def test_logs_command_needs_no_onnx(self, drone, mocker): + def fake_missing(names): + return [name for name in names if name == "onnx"] + + mocker.patch("blueye.sdk.cli.deps.missing", side_effect=fake_missing) + assert main(["logs", "list"]) == 0 + + +class TestInteractive: + def test_interactive_checkbox_download(self, drone, mocker, tmp_path): + class FakePrompter: + def checkbox(self, question, choices, flag): + return [choices[0]] # Select the first log. + + def text(self, question, default, flag): + return str(tmp_path) + + mocker.patch("sys.stdin.isatty", return_value=True) + mocker.patch("sys.stdout.isatty", return_value=True) + mocker.patch("blueye.sdk.cli.prompts.QuestionaryPrompter", return_value=FakePrompter()) + + assert main(["logs"]) == 0 + LogFile.download.assert_called_once() + assert LogFile.download.call_args.kwargs["output_path"] == tmp_path + + def test_interactive_empty_selection(self, drone, mocker, capsys): + class FakePrompter: + def checkbox(self, question, choices, flag): + return [] + + mocker.patch("sys.stdin.isatty", return_value=True) + mocker.patch("sys.stdout.isatty", return_value=True) + mocker.patch("blueye.sdk.cli.prompts.QuestionaryPrompter", return_value=FakePrompter()) + + assert main(["logs"]) == 0 + LogFile.download.assert_not_called() + assert "Nothing selected" in capsys.readouterr().out From c34cc573fcdb55ffbec2e0e60a6c5cf376b7a026 Mon Sep 17 00:00:00 2001 From: Juan Pablo Pino Bravo Date: Fri, 10 Jul 2026 13:04:16 +0200 Subject: [PATCH 2/7] feat: add --mcap conversion to `blueye logs download` `blueye logs download ... --mcap` converts each downloaded .bez to a Foxglove-ready .mcap next to it; the interactive picker offers the same conversion after selecting logs. The converter (commands/logs/mcap.py) is adapted from examples/foxglove_bez_to_mcap.py: a first LogStream pass anchors the dive start time (last record's wall clock minus its monotonic delta, so logs where the clock was set mid-dive stay continuous), a second pass writes every protobuf message via mcap_protobuf.writer. mcap-protobuf-support joins the [cli] extra (and dev group); the --mcap path gates on it at runtime with the standard install guidance, so the rest of the logs command works without it. Verified against the bench drone: `blueye logs download --latest 1 --mcap` produced a valid 8.5 MB .mcap (217k messages, correct MCAP magic) from a real dive log. +6 tests incl. a real protobuf-built .bez round trip (438 total); the Foxglove doc gains a one-step download-and-convert tip. Co-Authored-By: Claude Fable 5 --- blueye/sdk/cli/commands/logs/command.py | 48 +++++++++++- blueye/sdk/cli/commands/logs/mcap.py | 61 +++++++++++++++ docs/logs/foxglove-bez-to-mcap.md | 11 +++ docs/logs/listing-and-downloading.md | 1 + pyproject.toml | 3 + tests/test_cli_logs_command.py | 98 +++++++++++++++++++++++++ uv.lock | 4 + 7 files changed, 223 insertions(+), 3 deletions(-) create mode 100644 blueye/sdk/cli/commands/logs/mcap.py diff --git a/blueye/sdk/cli/commands/logs/command.py b/blueye/sdk/cli/commands/logs/command.py index 49efc587..7d6cca7f 100644 --- a/blueye/sdk/cli/commands/logs/command.py +++ b/blueye/sdk/cli/commands/logs/command.py @@ -52,6 +52,11 @@ def add_parser(subparsers) -> None: help="Download the N most recent logs", ) download.add_argument("--all", action="store_true", help="Download every log") + download.add_argument( + "--mcap", + action="store_true", + help="Also convert each downloaded log to .mcap (for Foxglove)", + ) def _connect(args): @@ -87,9 +92,25 @@ def _print_logs_table(console, log_files) -> None: console.print(table) -def _download_logs(console, log_files, output_dir: Path, timeout: float) -> None: +def _ensure_mcap_support() -> None: + """Gate the --mcap path on its optional dependency, with install guidance.""" + from ... import deps + + missing = deps.missing(("mcap_protobuf",)) + if missing: + deps.print_install_guidance(missing) + raise CliError("Converting to .mcap requires the mcap-protobuf-support package.") + + +def _download_logs( + console, log_files, output_dir: Path, timeout: float, convert_mcap: bool = False +) -> None: from blueye.sdk.logs import human_readable_filesize + if convert_mcap: + _ensure_mcap_support() + from .mcap import convert_bez_to_mcap + output_dir.mkdir(parents=True, exist_ok=True) for log in log_files: with console.status(f"[cyan]Downloading {log.name}..."): @@ -98,6 +119,14 @@ def _download_logs(console, log_files, output_dir: Path, timeout: float) -> None f"Downloaded {log.name}.bez ({human_readable_filesize(log.filesize)}) " f"to {output_dir}" ) + if convert_mcap: + bez_path = output_dir / f"{log.name}.bez" + mcap_path = output_dir / f"{log.name}.mcap" + with console.status(f"[cyan]Converting {log.name} to .mcap..."): + message_count = convert_bez_to_mcap(bez_path, mcap_path) + console.print( + f"Converted to {mcap_path.name} ({message_count} messages) — open it in " "Foxglove" + ) def _select_downloads(args, log_files) -> list: @@ -139,7 +168,14 @@ def _run_interactive(console, args, prompter, drone) -> int: console.print("Nothing selected.") return 0 output_dir = Path(prompter.text("Download to directory:", ".", "--output")).expanduser() - _download_logs(console, [by_label[label] for label in selected], output_dir, args.timeout) + convert_mcap = prompter.confirm("Also convert to .mcap for Foxglove?", False, "--mcap") + _download_logs( + console, + [by_label[label] for label in selected], + output_dir, + args.timeout, + convert_mcap=convert_mcap, + ) return 0 @@ -155,7 +191,13 @@ def run(args: argparse.Namespace) -> int: if action == "download": log_files = _log_rows(drone.logs) selection = _select_downloads(args, log_files) - _download_logs(console, selection, Path(args.output).expanduser(), args.timeout) + _download_logs( + console, + selection, + Path(args.output).expanduser(), + args.timeout, + convert_mcap=args.mcap, + ) return 0 if action is None and sys.stdin.isatty() and sys.stdout.isatty(): diff --git a/blueye/sdk/cli/commands/logs/mcap.py b/blueye/sdk/cli/commands/logs/mcap.py new file mode 100644 index 00000000..6d81b8e0 --- /dev/null +++ b/blueye/sdk/cli/commands/logs/mcap.py @@ -0,0 +1,61 @@ +"""Conversion of .bez dive logs to Foxglove-compatible .mcap files. + +Adapted from examples/foxglove_bez_to_mcap.py: the log is streamed twice — a first +pass finds the true dive start time (the drone's clock may be set mid-log, so the +last record's wall time minus its monotonic delta is the reliable anchor), and a +second pass writes every protobuf message with continuous timestamps. +""" + +from __future__ import annotations + +import logging +from pathlib import Path + +from ...errors import CliError + +logger = logging.getLogger(__name__) + + +def convert_bez_to_mcap(bez_path: Path, mcap_path: Path) -> int: + """Convert a downloaded .bez log to an .mcap file for Foxglove. + + Args: + bez_path: The .bez file to convert. + mcap_path: Destination .mcap path (overwritten if present). + + Returns: + The number of messages written. + + Raises: + CliError: When the log contains no readable records. + """ + from mcap_protobuf.writer import Writer + + from blueye.sdk.logs import LogStream + + log_bytes = bez_path.read_bytes() + + # First pass: the last record's wall clock minus its monotonic delta gives the + # dive start time even when the drone's clock was set partway through the log. + last_time = None + last_delta = None + for last_time, last_delta, _, _ in LogStream(log_bytes): + continue + if last_time is None: + raise CliError(f"{bez_path.name} contains no readable log records.") + start_time = last_time - last_delta + + count = 0 + with mcap_path.open("wb") as mcap_file: + writer = Writer(mcap_file) + for _, delta, msg_type, msg in LogStream(log_bytes): + timestamp_ns = int((start_time + delta).timestamp() * 1e9) + writer.write_message( + topic=msg_type.__name__, + message=msg._pb, + log_time=timestamp_ns, + publish_time=timestamp_ns, + ) + count += 1 + writer.finish() + return count diff --git a/docs/logs/foxglove-bez-to-mcap.md b/docs/logs/foxglove-bez-to-mcap.md index 46f8bb2d..942056f7 100644 --- a/docs/logs/foxglove-bez-to-mcap.md +++ b/docs/logs/foxglove-bez-to-mcap.md @@ -1,6 +1,17 @@ # Visualize dive log sensor data with Foxglove With some simple steps you can visualize dive log data with ease in Foxglove. This is a great tool to play back and visualize control signals and estimated states and other sensor data from the dive. +!!! tip "One-step download and convert" + The `blueye` CLI (installed with `pip install "blueye.sdk[cli]"`) can download and + convert in one go: + + ```shell + blueye logs download --latest 1 --mcap + ``` + + This fetches the newest log from the drone and writes both the `.bez` and a + Foxglove-ready `.mcap` next to it. Then continue from step 5 below. + 1. Download foxglove [here](https://foxglove.dev/download) and create an account. 2. Download a divelog from the drone as shown [here](https://blueye-robotics.github.io/blueye.sdk/latest/logs/listing-and-downloading/). 3. Run `pip install "blueye.sdk[examples]"` to get the necessary dependencies, if you have not done so already. diff --git a/docs/logs/listing-and-downloading.md b/docs/logs/listing-and-downloading.md index 37cc0981..1c6e6318 100644 --- a/docs/logs/listing-and-downloading.md +++ b/docs/logs/listing-and-downloading.md @@ -14,6 +14,7 @@ SDK's `[cli]` extra), which connects to the drone as an observer — taking no c ```shell blueye logs list # table of logs on the drone blueye logs download --latest 1 # newest log to the current directory +blueye logs download --latest 1 --mcap # ...and convert it for Foxglove blueye logs download BYEDP000000_ea9ac92e1817a1d4_00002 -o ~/dives blueye logs # interactive: pick logs to download ``` diff --git a/pyproject.toml b/pyproject.toml index e5173fec..bfe5015c 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -36,6 +36,8 @@ cli = [ "questionary>=2.0,<3", # TOML parser for reading third-party tools' PEP 723 metadata; stdlib from 3.11. "tomli>=2,<3; python_version < '3.11'", + # .bez -> .mcap conversion (`blueye logs download --mcap`). + "mcap-protobuf-support>=0.5.3,<0.6", ] # These are dependencies that are not necessary for the core functionality of the SDK, but are # necessary for some of the examples. @@ -61,6 +63,7 @@ dev = [ "rich>=13,<15", "questionary>=2.0,<3", "tomli>=2,<3; python_version < '3.11'", + "mcap-protobuf-support>=0.5.3,<0.6", "pytest~=8.3", "pytest-mock~=3.11", "mike~=2.1", diff --git a/tests/test_cli_logs_command.py b/tests/test_cli_logs_command.py index e3934bd7..d683fc3f 100644 --- a/tests/test_cli_logs_command.py +++ b/tests/test_cli_logs_command.py @@ -116,6 +116,9 @@ def checkbox(self, question, choices, flag): def text(self, question, default, flag): return str(tmp_path) + def confirm(self, question, default, flag): + return default # Decline the .mcap conversion. + mocker.patch("sys.stdin.isatty", return_value=True) mocker.patch("sys.stdout.isatty", return_value=True) mocker.patch("blueye.sdk.cli.prompts.QuestionaryPrompter", return_value=FakePrompter()) @@ -136,3 +139,98 @@ def checkbox(self, question, choices, flag): assert main(["logs"]) == 0 LogFile.download.assert_not_called() assert "Nothing selected" in capsys.readouterr().out + + +class TestMcapConversion: + @pytest.fixture + def bez_file(self, tmp_path): + """A tiny real .bez (uncompressed binlog records) built with protobuf.""" + import blueye.protocol as bp + + from tests.test_logs import create_real_binlog_record + + records = b"" + for seconds in (100, 101, 102): + payload = bp.DepthTel(depth=bp.Depth(value=float(seconds))) + records += create_real_binlog_record(1700000000 + seconds, seconds, payload) + path = tmp_path / "dive.bez" + path.write_bytes(records) + return path + + def test_convert_writes_valid_mcap(self, bez_file, tmp_path): + from blueye.sdk.cli.commands.logs.mcap import convert_bez_to_mcap + + mcap_path = tmp_path / "dive.mcap" + count = convert_bez_to_mcap(bez_file, mcap_path) + assert count == 3 + content = mcap_path.read_bytes() + assert content.startswith(b"\x89MCAP") # MCAP magic bytes. + assert len(content) > 100 + + def test_convert_empty_log_errors(self, tmp_path): + from blueye.sdk.cli.commands.logs.mcap import convert_bez_to_mcap + from blueye.sdk.cli.errors import CliError + + empty = tmp_path / "empty.bez" + empty.write_bytes(b"") + with pytest.raises(CliError, match="no readable log records"): + convert_bez_to_mcap(empty, tmp_path / "empty.mcap") + + def test_download_mcap_flag_converts(self, drone, mocker, tmp_path): + convert = mocker.patch( + "blueye.sdk.cli.commands.logs.mcap.convert_bez_to_mcap", return_value=5 + ) + assert ( + main( + [ + "logs", + "download", + "BYEDP000000_aaaa_00000", + "-o", + str(tmp_path), + "--mcap", + ] + ) + == 0 + ) + convert.assert_called_once_with( + tmp_path / "BYEDP000000_aaaa_00000.bez", tmp_path / "BYEDP000000_aaaa_00000.mcap" + ) + + def test_download_without_mcap_flag_does_not_convert(self, drone, mocker, tmp_path): + convert = mocker.patch("blueye.sdk.cli.commands.logs.mcap.convert_bez_to_mcap") + assert main(["logs", "download", "--all", "-o", str(tmp_path)]) == 0 + convert.assert_not_called() + + def test_missing_mcap_dependency_gives_guidance(self, drone, mocker, tmp_path, capsys): + def fake_missing(names): + return [name for name in names if name == "mcap_protobuf"] + + mocker.patch("blueye.sdk.cli.deps.missing", side_effect=fake_missing) + exit_code = main(["logs", "download", "--all", "-o", str(tmp_path), "--mcap"]) + assert exit_code == 1 + captured = capsys.readouterr() + assert "blueye.sdk[cli]" in captured.out + assert "mcap" in captured.err + + def test_interactive_offers_mcap_conversion(self, drone, mocker, tmp_path): + convert = mocker.patch( + "blueye.sdk.cli.commands.logs.mcap.convert_bez_to_mcap", return_value=5 + ) + + class FakePrompter: + def checkbox(self, question, choices, flag): + return [choices[0]] + + def text(self, question, default, flag): + return str(tmp_path) + + def confirm(self, question, default, flag): + return "mcap" in question # Say yes to the conversion confirm. + + mocker.patch("sys.stdin.isatty", return_value=True) + mocker.patch("sys.stdout.isatty", return_value=True) + mocker.patch("blueye.sdk.cli.prompts.QuestionaryPrompter", return_value=FakePrompter()) + + assert main(["logs"]) == 0 + convert.assert_called_once() diff --git a/uv.lock b/uv.lock index 460fcb65..b5b84c66 100644 --- a/uv.lock +++ b/uv.lock @@ -133,6 +133,7 @@ dependencies = [ [package.optional-dependencies] cli = [ + { name = "mcap-protobuf-support" }, { name = "onnx" }, { name = "questionary" }, { name = "rich" }, @@ -154,6 +155,7 @@ dev = [ { name = "black" }, { name = "essentials-openapi" }, { name = "freezegun" }, + { name = "mcap-protobuf-support" }, { name = "mike" }, { name = "mkdocs" }, { name = "mkdocs-gen-files" }, @@ -180,6 +182,7 @@ requires-dist = [ { name = "foxglove-websocket", marker = "extra == 'examples'", specifier = ">=0.1.2,<0.2" }, { name = "inputs", marker = "extra == 'examples'", specifier = ">=0.5,<0.6" }, { name = "matplotlib", marker = "extra == 'examples'", specifier = "~=3.10" }, + { name = "mcap-protobuf-support", marker = "extra == 'cli'", specifier = ">=0.5.3,<0.6" }, { name = "mcap-protobuf-support", marker = "extra == 'examples'", specifier = ">=0.5.3,<0.6" }, { name = "onnx", marker = "extra == 'cli'", specifier = ">=1.16,<2" }, { name = "packaging", specifier = ">=24.2" }, @@ -202,6 +205,7 @@ dev = [ { name = "black", specifier = "~=26.5" }, { name = "essentials-openapi", specifier = ">=1.3.0,<2" }, { name = "freezegun", specifier = "~=1.2" }, + { name = "mcap-protobuf-support", specifier = ">=0.5.3,<0.6" }, { name = "mike", specifier = "~=2.1" }, { name = "mkdocs", specifier = "~=1.5" }, { name = "mkdocs-gen-files", specifier = ">=0.5.0,<0.6" }, From 3ddfab8b932cd9601333f0c39142b378c57da81d Mon Sep 17 00:00:00 2001 From: Juan Pablo Pino Bravo Date: Fri, 10 Jul 2026 14:13:19 +0200 Subject: [PATCH 3/7] docs: fold `blueye logs` into the CLI page, add quick-start example Follow-up to the review changes merged from jp-pino/bundle-model-cli: - The logs command joins the consolidated docs/cli.md as a "Downloading dive logs" section (incl. --mcap); the logs guide and the Foxglove doc keep short pointers to the CLI page. - The quick start gains a "Try the command line interface" section showcasing `blueye logs download --latest 1` with links to the CLI page. - With rich/questionary in the core dependencies, the logs command declares requires=() (the --mcap path keeps its runtime gate); mcap-protobuf-support stays in the [cli] extra alongside onnx. Co-Authored-By: Claude Fable 5 --- blueye/sdk/cli/commands/logs/__init__.py | 2 +- docs/cli.md | 18 ++++++++++++++++++ docs/logs/foxglove-bez-to-mcap.md | 4 ++-- docs/logs/listing-and-downloading.md | 4 ++-- docs/quick_start.md | 11 +++++++++++ 5 files changed, 34 insertions(+), 5 deletions(-) diff --git a/blueye/sdk/cli/commands/logs/__init__.py b/blueye/sdk/cli/commands/logs/__init__.py index e033f418..a217b55e 100644 --- a/blueye/sdk/cli/commands/logs/__init__.py +++ b/blueye/sdk/cli/commands/logs/__init__.py @@ -8,7 +8,7 @@ COMMAND = CommandSpec( name="logs", help="List and download dive logs from the drone", - requires=("rich", "questionary"), + requires=(), # rich/questionary are core SDK dependencies; --mcap gates at runtime. add_parser=add_parser, run=run, ) diff --git a/docs/cli.md b/docs/cli.md index fc20fc30..bae0de03 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -141,6 +141,24 @@ Model names are the directory slugs shown by `blueye models list`. Note: the `enabled` state is the autolaunch configuration; the API does not expose a live "running" status. +## Downloading dive logs — `blueye logs` + +The drone's binary dive logs (`.bez`) can be listed and downloaded from the terminal. +The command connects to the drone as an observer — taking no control: + +```shell +blueye logs list # table of logs on the drone +blueye logs download --latest 1 # newest log to the current directory +blueye logs download --latest 1 --mcap # ...and convert it for Foxglove +blueye logs download BYEDP000000_ea9ac92e1817a1d4_00002 -o ~/dives +blueye logs # interactive: pick logs to download +``` + +`--mcap` converts each downloaded log to a Foxglove-ready `.mcap` next to the +`.bez` — see [visualizing dive logs with Foxglove](logs/foxglove-bez-to-mcap.md). +For working with logs from Python (streaming, filtering, plotting), see +[logs from the drone](logs/listing-and-downloading.md). + ## Third-party tools — `blueye tools` The `blueye` command is built to grow: besides the built-in commands, anyone can drop diff --git a/docs/logs/foxglove-bez-to-mcap.md b/docs/logs/foxglove-bez-to-mcap.md index 942056f7..8beda155 100644 --- a/docs/logs/foxglove-bez-to-mcap.md +++ b/docs/logs/foxglove-bez-to-mcap.md @@ -2,8 +2,8 @@ With some simple steps you can visualize dive log data with ease in Foxglove. This is a great tool to play back and visualize control signals and estimated states and other sensor data from the dive. !!! tip "One-step download and convert" - The `blueye` CLI (installed with `pip install "blueye.sdk[cli]"`) can download and - convert in one go: + The [`blueye` CLI](../cli.md) (installed with the SDK) can download and convert + in one go: ```shell blueye logs download --latest 1 --mcap diff --git a/docs/logs/listing-and-downloading.md b/docs/logs/listing-and-downloading.md index 1c6e6318..4da9199d 100644 --- a/docs/logs/listing-and-downloading.md +++ b/docs/logs/listing-and-downloading.md @@ -8,8 +8,8 @@ When the drone is powered on a new log file is created, where it stores telemetr ## From the command line -The binary logs are also available through the `blueye` CLI (installed with the -SDK's `[cli]` extra), which connects to the drone as an observer — taking no control: +The binary logs are also available through the [`blueye` CLI](../cli.md), which is +installed with the SDK and connects to the drone as an observer — taking no control: ```shell blueye logs list # table of logs on the drone diff --git a/docs/quick_start.md b/docs/quick_start.md index 34fb16a8..d882b7c7 100644 --- a/docs/quick_start.md +++ b/docs/quick_start.md @@ -152,6 +152,17 @@ The normal Blueye app cannot be used to spectate when controlling the drone from it will interfere with the commands sent from the SDK. The Observer app, however, is only a spectator and can be used together with the SDK. +### Try the command line interface +The SDK also installs a [`blueye` command](cli.md) for common tasks straight from the +terminal — for example, grabbing the newest dive log from the drone: + +```shell +blueye logs download --latest 1 +``` + +Run `blueye --help` to see everything it can do, or read more on +[the blueye CLI page](cli.md). + ### Explore the examples For further examples on how to use the SDK to control the drone have a look at the [motion examples](movement/from-the-CLI.md). From 5829fd2033fba024d6d52cb84d8206ebf64fb66c Mon Sep 17 00:00:00 2001 From: Juan Pablo Pino Bravo Date: Fri, 10 Jul 2026 17:05:37 +0200 Subject: [PATCH 4/7] feat: local log conversion, filterable interactive view, retire mcap example MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-ups on the logs command: - New `blueye logs convert [-o DIR]` converts already-downloaded logs to .mcap without touching the drone (dispatch happens before the observer connection; same runtime mcap gate). - Interactive view redesigned: instead of printing the full table and then a duplicate checkbox list, it is now a single scrollable multi-select table (each choice is a column-aligned row under one header line) with type-to-filter (questionary use_search_filter), sorted descending alphabetically — newest logs first. An explicit instruction string works around questionary 2.1.1 showing for both toggle-all and invert (the real invert binding is ctrl-i/tab). - New --dives-only / --since / --until filters shared by list, download, and the interactive view; list output is sorted descending too. - The example converter (examples/foxglove_bez_to_mcap.py) is retired in favor of the first-party command: the Foxglove doc now walks through `blueye logs download --latest 1 --mcap` and `blueye logs convert`, and mcap-protobuf-support leaves the [examples] extra (its only consumer). Verified against the bench drone: filtered+sorted list, download, and a local `blueye logs convert` producing a valid .mcap (1.19M messages). +13 tests (451 total); docs build clean with the embed removed. Co-Authored-By: Claude Fable 5 --- blueye/sdk/cli/commands/logs/command.py | 156 ++++++++++++++++++++---- blueye/sdk/cli/prompts.py | 16 ++- docs/cli.md | 7 +- docs/logs/foxglove-bez-to-mcap.md | 31 ++--- examples/foxglove_bez_to_mcap.py | 65 ---------- pyproject.toml | 1 - tests/test_cli_logs_command.py | 118 +++++++++++++++++- uv.lock | 2 - 8 files changed, 282 insertions(+), 114 deletions(-) delete mode 100644 examples/foxglove_bez_to_mcap.py diff --git a/blueye/sdk/cli/commands/logs/command.py b/blueye/sdk/cli/commands/logs/command.py index 7d6cca7f..56367cec 100644 --- a/blueye/sdk/cli/commands/logs/command.py +++ b/blueye/sdk/cli/commands/logs/command.py @@ -2,8 +2,9 @@ Follows the documented log workflow (docs/logs/listing-and-downloading.md): connect to the drone **as an observer** (taking no control), read the binary log index from -`drone.logs`, and download `.bez` files with `LogFile.download`. Legacy CSV logs are -not covered — use `drone.legacy_logs` from the SDK for those. +`drone.logs`, and download `.bez` files with `LogFile.download`. `convert` works on +already-downloaded files and never touches the drone. Legacy CSV logs are not covered +— use `drone.legacy_logs` from the SDK for those. Argument definitions are stdlib-only; rich/questionary/blueye.sdk imports happen inside `run` (after the dependency gate). @@ -12,6 +13,7 @@ from __future__ import annotations import argparse +import datetime import logging import sys from pathlib import Path @@ -22,6 +24,16 @@ logger = logging.getLogger(__name__) +def _add_filter_options(parser: argparse.ArgumentParser) -> None: + parser.add_argument("--dives-only", action="store_true", help="Only logs classified as dives") + parser.add_argument( + "--since", metavar="YYYY-MM-DD", help="Only logs starting on or after this date" + ) + parser.add_argument( + "--until", metavar="YYYY-MM-DD", help="Only logs starting on or before this date" + ) + + def add_parser(subparsers) -> None: """Register the ``logs`` subcommand and its sub-subcommands.""" common = drone_options_parser(timeout_default=10.0) @@ -29,16 +41,18 @@ def add_parser(subparsers) -> None: parser = subparsers.add_parser( "logs", parents=[common], - help="List and download dive logs from the drone", + help="List, download, and convert dive logs", description=( - "List and download the drone's binary dive logs (.bez). Connects to the " - "drone as an observer, taking no control. Run without an action on a " - "terminal to pick logs interactively." + "List and download the drone's binary dive logs (.bez), and convert them " + "to .mcap for Foxglove. Drone actions connect as an observer, taking no " + "control. Run without an action on a terminal to pick logs interactively." ), ) + _add_filter_options(parser) actions = parser.add_subparsers(dest="logs_command", metavar="ACTION") - actions.add_parser("list", parents=[common], help="List the logs on the drone") + list_parser = actions.add_parser("list", parents=[common], help="List the logs on the drone") + _add_filter_options(list_parser) download = actions.add_parser("download", parents=[common], help="Download logs from the drone") download.add_argument("names", nargs="*", help="Log names to download") @@ -57,6 +71,16 @@ def add_parser(subparsers) -> None: action="store_true", help="Also convert each downloaded log to .mcap (for Foxglove)", ) + _add_filter_options(download) + + convert = actions.add_parser( + "convert", + help="Convert already-downloaded .bez logs to .mcap (local, no drone needed)", + ) + convert.add_argument("files", nargs="+", help="Paths to .bez files") + convert.add_argument( + "-o", "--output", help="Destination directory (default: next to each input file)" + ) def _connect(args): @@ -68,9 +92,50 @@ def _connect(args): ) -def _log_rows(logs) -> list: - """The drone's logs as a list of LogFile objects (index fetched lazily).""" - return friendly_errors(lambda: list(logs)) +def _parse_date(value: str, flag: str) -> datetime.date: + try: + return datetime.date.fromisoformat(value) + except ValueError as error: + raise CliError(f'{flag} must be a date like "2026-06-01", got "{value}"') from error + + +def _filter_logs(args, log_files) -> list: + """Apply the --dives-only/--since/--until filters.""" + filtered = list(log_files) + if getattr(args, "dives_only", False): + filtered = [log for log in filtered if log.is_dive] + since = getattr(args, "since", None) + if since: + since_date = _parse_date(since, "--since") + filtered = [log for log in filtered if log.start_time.date() >= since_date] + until = getattr(args, "until", None) + if until: + until_date = _parse_date(until, "--until") + filtered = [log for log in filtered if log.start_time.date() <= until_date] + return filtered + + +def _log_rows(logs, args) -> list: + """The drone's logs, filtered and sorted descending alphabetically.""" + log_files = friendly_errors(lambda: list(logs)) + return sorted(_filter_logs(args, log_files), key=lambda log: log.name, reverse=True) + + +#: Column widths for the interactive table rows (monospace-aligned). +_NAME_WIDTH = 36 +_TIME_WIDTH = 18 +_SIZE_WIDTH = 10 + + +def _format_row(log) -> str: + from blueye.sdk.logs import human_readable_filesize + + return ( + f"{log.name.ljust(_NAME_WIDTH)}" + f"{log.start_time.strftime('%d. %b %Y %H:%M').ljust(_TIME_WIDTH)}" + f"{human_readable_filesize(log.filesize).ljust(_SIZE_WIDTH)}" + f"{'dive' if log.is_dive else ''}" + ) def _print_logs_table(console, log_files) -> None: @@ -93,7 +158,7 @@ def _print_logs_table(console, log_files) -> None: def _ensure_mcap_support() -> None: - """Gate the --mcap path on its optional dependency, with install guidance.""" + """Gate the .mcap paths on their optional dependency, with install guidance.""" from ... import deps missing = deps.missing(("mcap_protobuf",)) @@ -129,6 +194,28 @@ def _download_logs( ) +def _run_convert(console, args) -> int: + """Convert already-downloaded .bez files to .mcap. Purely local.""" + _ensure_mcap_support() + from .mcap import convert_bez_to_mcap + + inputs = [Path(name).expanduser() for name in args.files] + missing = [str(path) for path in inputs if not path.is_file()] + if missing: + raise CliError(f"No such file: {', '.join(missing)}") + + output_dir = Path(args.output).expanduser() if args.output else None + if output_dir is not None: + output_dir.mkdir(parents=True, exist_ok=True) + + for bez_path in inputs: + mcap_path = (output_dir or bez_path.parent) / f"{bez_path.stem}.mcap" + with console.status(f"[cyan]Converting {bez_path.name}..."): + message_count = convert_bez_to_mcap(bez_path, mcap_path) + console.print(f"Converted {bez_path.name} to {mcap_path} ({message_count} messages)") + return 0 + + def _select_downloads(args, log_files) -> list: """Resolve the download selection from names/--latest/--all.""" by_name = {log.name: log for log in log_files} @@ -149,21 +236,22 @@ def _select_downloads(args, log_files) -> list: def _run_interactive(console, args, prompter, drone) -> int: - """Show the table, pick logs with a checkbox, download to a chosen directory.""" - log_files = _log_rows(drone.logs) + """One scrollable, filterable, multi-select table of logs to download.""" + log_files = _log_rows(drone.logs, args) if not log_files: - console.print("No logs on the drone.") + console.print( + "No logs match the filters." if _has_filters(args) else "No logs on the drone." + ) return 0 - _print_logs_table(console, log_files) - from blueye.sdk.logs import human_readable_filesize - - by_label = { - f"{log.name} {log.start_time.strftime('%d. %b %Y %H:%M')} " - f"{human_readable_filesize(log.filesize)}": log - for log in log_files - } - selected = prompter.checkbox("Select logs to download:", list(by_label), "--latest/--all") + by_row = {_format_row(log): log for log in log_files} + header = ( + f"{'NAME'.ljust(_NAME_WIDTH)}{'TIME'.ljust(_TIME_WIDTH)}{'SIZE'.ljust(_SIZE_WIDTH)}DIVE" + ) + console.print(f"[bold] {header}[/bold]") + selected = prompter.checkbox( + "Select logs to download (type to filter):", list(by_row), "--latest/--all" + ) if not selected: console.print("Nothing selected.") return 0 @@ -171,7 +259,7 @@ def _run_interactive(console, args, prompter, drone) -> int: convert_mcap = prompter.confirm("Also convert to .mcap for Foxglove?", False, "--mcap") _download_logs( console, - [by_label[label] for label in selected], + [by_row[row] for row in selected], output_dir, args.timeout, convert_mcap=convert_mcap, @@ -179,6 +267,14 @@ def _run_interactive(console, args, prompter, drone) -> int: return 0 +def _has_filters(args) -> bool: + return bool( + getattr(args, "dives_only", False) + or getattr(args, "since", None) + or getattr(args, "until", None) + ) + + def run(args: argparse.Namespace) -> int: """Dispatch the logs sub-subcommand.""" from ... import prompts, ui @@ -186,10 +282,14 @@ def run(args: argparse.Namespace) -> int: console = ui.make_console() action = getattr(args, "logs_command", None) + # `convert` is purely local — no drone connection. + if action == "convert": + return _run_convert(console, args) + drone = _connect(args) try: if action == "download": - log_files = _log_rows(drone.logs) + log_files = _log_rows(drone.logs, args) selection = _select_downloads(args, log_files) _download_logs( console, @@ -208,9 +308,11 @@ def run(args: argparse.Namespace) -> int: return 130 # `logs list` and non-TTY bare invocation. - log_files = _log_rows(drone.logs) + log_files = _log_rows(drone.logs, args) if not log_files: - console.print("No logs on the drone.") + console.print( + "No logs match the filters." if _has_filters(args) else "No logs on the drone." + ) return 0 _print_logs_table(console, log_files) return 0 diff --git a/blueye/sdk/cli/prompts.py b/blueye/sdk/cli/prompts.py index aeef060f..4b9fb845 100644 --- a/blueye/sdk/cli/prompts.py +++ b/blueye/sdk/cli/prompts.py @@ -71,7 +71,21 @@ def path(self, question: str, default: str | None, flag: str) -> str: return str(_require(questionary.path(question, default=default or "").ask())) def checkbox(self, question: str, choices: Sequence[str], flag: str) -> list[str]: - answer = _require(questionary.checkbox(question, choices=list(choices)).ask()) + answer = _require( + questionary.checkbox( + question, + choices=list(choices), + use_search_filter=True, + use_jk_keys=False, + # questionary 2.1.1's default instruction wrongly shows for + # both actions when the search filter is on; the real bindings are + # ctrl-a = toggle all and ctrl-i (tab) = invert. + instruction=( + "(use arrow keys to move, to select, to toggle " + "all, to invert, type to filter)" + ), + ).ask() + ) return [str(item) for item in answer] diff --git a/docs/cli.md b/docs/cli.md index b9e1dbe2..679f6cb6 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -151,11 +151,16 @@ blueye logs list # table of logs on the drone blueye logs download --latest 1 # newest log to the current directory blueye logs download --latest 1 --mcap # ...and convert it for Foxglove blueye logs download BYEDP000000_ea9ac92e1817a1d4_00002 -o ~/dives +blueye logs convert mydive.bez # convert an already-downloaded log (no drone) blueye logs # interactive: pick logs to download ``` +`list`, `download`, and the interactive view accept `--dives-only`, `--since +YYYY-MM-DD`, and `--until YYYY-MM-DD` to narrow the selection; the interactive view +is a single scrollable table (type to filter, space to select, sorted newest first). `--mcap` converts each downloaded log to a Foxglove-ready `.mcap` next to the -`.bez` — see [visualizing dive logs with Foxglove](logs/foxglove-bez-to-mcap.md). +`.bez` — `blueye logs convert` does the same for files already on disk. See +[visualizing dive logs with Foxglove](logs/foxglove-bez-to-mcap.md). For working with logs from Python (streaming, filtering, plotting), see [logs from the drone](logs/listing-and-downloading.md). diff --git a/docs/logs/foxglove-bez-to-mcap.md b/docs/logs/foxglove-bez-to-mcap.md index 8beda155..5981d875 100644 --- a/docs/logs/foxglove-bez-to-mcap.md +++ b/docs/logs/foxglove-bez-to-mcap.md @@ -1,25 +1,26 @@ # Visualize dive log sensor data with Foxglove + With some simple steps you can visualize dive log data with ease in Foxglove. This is a great tool to play back and visualize control signals and estimated states and other sensor data from the dive. -!!! tip "One-step download and convert" - The [`blueye` CLI](../cli.md) (installed with the SDK) can download and convert - in one go: +1. Download Foxglove [here](https://foxglove.dev/download) and create an account. +2. Install the SDK with the `cli` extra to get the [`blueye` CLI](../cli.md) and the + `.mcap` converter: `pip install "blueye.sdk[cli]"`. +3. Download a dive log from the drone and convert it in one step: ```shell blueye logs download --latest 1 --mcap ``` - This fetches the newest log from the drone and writes both the `.bez` and a - Foxglove-ready `.mcap` next to it. Then continue from step 5 below. + Already have `.bez` files on disk? Convert them directly — no drone needed: + + ```shell + blueye logs convert mydive.bez + ``` -1. Download foxglove [here](https://foxglove.dev/download) and create an account. -2. Download a divelog from the drone as shown [here](https://blueye-robotics.github.io/blueye.sdk/latest/logs/listing-and-downloading/). -3. Run `pip install "blueye.sdk[examples]"` to get the necessary dependencies, if you have not done so already. -4. Clone the [blueye.sdk repository](https://github.com/BluEye-Robotics/blueye.sdk) to get the examples, or copy the script below into a file. In the examples folder you simply run `python foxglove_bez_to_mcap.py [output_filename.mcap]` to convert your .bez-file. -5. Open foxglove, in the top left menu, click on `Open local file`, and pick your newly created .mcap-file. -6. Click on `Add panel`, and `Raw message`, or `Plot` and select the signal you want to display. -7. Start typing `DepthTel.depth.value` to get auto-complete on all available messages in the protocol. -8. You can also get a nice overview of the logged messages with this command: `mcap info logfile.mcap` in your terminal. +4. Open Foxglove, in the top left menu, click on `Open local file`, and pick your newly created .mcap-file. +5. Click on `Add panel`, and `Raw message`, or `Plot` and select the signal you want to display. +6. Start typing `DepthTel.depth.value` to get auto-complete on all available messages in the protocol. +7. You can also get a nice overview of the logged messages with this command: `mcap info logfile.mcap` in your terminal. -### The .bez to .mcap log file converter: -{{code_from_file("../examples/foxglove_bez_to_mcap.py", "python")}} +For programmatic access to the log records (the converter is built on the same +parser), see [`LogStream`][blueye.sdk.logs.LogStream]. diff --git a/examples/foxglove_bez_to_mcap.py b/examples/foxglove_bez_to_mcap.py deleted file mode 100644 index 0dc34db3..00000000 --- a/examples/foxglove_bez_to_mcap.py +++ /dev/null @@ -1,65 +0,0 @@ -import os -import time -from mcap_protobuf.writer import Writer -import sys -from blueye.sdk.logs import LogStream -from pathlib import Path - - -def parse_logfile(log: Path) -> LogStream: - log_bytes = b"" - with open(log, "rb") as f: - log_bytes = f.read() - return LogStream(log_bytes) - - -def main(logfile_path, output_mcap_path): - start_time_tic = time.time() - print(f"Converting {logfile_path} to {output_mcap_path}...") - - # Prepare MCAP writer - with open(output_mcap_path, "wb") as mcap_file: - writer = Writer(mcap_file) - - # Read messages from the log file, deserialize, and forward the protobuf object to the MCAP file. - path = Path(logfile_path) - - # We need to get the last message's timestamp and delta in order to get the correct start time - # after the clock is set. The delta time is then added to the start time to get a continuous timeline in foxglove. - last_time = 0 - last_delta = 0 - for last_time, last_delta, _, _ in parse_logfile(path): - continue - - start_time = last_time - last_delta - - count = 0 - for unix_ts, delta, msg_type, msg in parse_logfile(path): - writer.write_message( - topic=msg_type.__name__, - message=msg._pb, - log_time=int((start_time + delta).timestamp() * 1e9), - publish_time=int((start_time + delta).timestamp() * 1e9), - ) - count += 1 - - # Add indexes to the MCAP file - writer.finish() - - print(f"MCAP file successfully created!") - print( - f"Total of messages written: {count} in {round(time.time() - start_time_tic, 3)} seconds" - ) - print(f"MCAP file name: {output_mcap_path}") - print(f"MCAP file size: {round(os.path.getsize(output_mcap_path)/1000000, 2)} MB") - print(f"Start of dive time: {unix_ts - delta}") - print(f"Duration of dive log: {delta}") - - -if __name__ == "__main__": - if len(sys.argv) < 2: - print("Usage: python bez_to_mcap.py [output_filename.mcap]") - sys.exit(1) - logfile = sys.argv[1] - output = sys.argv[2] if len(sys.argv) > 2 else sys.argv[1].replace(".bez", ".mcap") - main(logfile, output) diff --git a/pyproject.toml b/pyproject.toml index 2e079926..0098dd27 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -51,7 +51,6 @@ examples = [ "webdavclient3>=3.14.6,<4", "foxglove_websocket>=0.1.2,<0.2", "pyserial~=3.5", - "mcap-protobuf-support>=0.5.3,<0.6", ] [project.urls] diff --git a/tests/test_cli_logs_command.py b/tests/test_cli_logs_command.py index d683fc3f..7a75478e 100644 --- a/tests/test_cli_logs_command.py +++ b/tests/test_cli_logs_command.py @@ -108,10 +108,13 @@ def fake_missing(names): class TestInteractive: - def test_interactive_checkbox_download(self, drone, mocker, tmp_path): + def test_interactive_checkbox_download(self, drone, mocker, tmp_path, capsys): + seen_choices = [] + class FakePrompter: def checkbox(self, question, choices, flag): - return [choices[0]] # Select the first log. + seen_choices.extend(choices) + return [choices[0]] # Select the first (newest) log. def text(self, question, default, flag): return str(tmp_path) @@ -126,6 +129,15 @@ def confirm(self, question, default, flag): assert main(["logs"]) == 0 LogFile.download.assert_called_once() assert LogFile.download.call_args.kwargs["output_path"] == tmp_path + # Sorted descending alphabetically: _00001 before _00000. + assert "BYEDP000000_aaaa_00001" in seen_choices[0] + assert "BYEDP000000_aaaa_00000" in seen_choices[1] + # The choices themselves are the table rows (name + time + size columns). + assert "KiB" in seen_choices[0] + # No duplicated full table before the picker — only the header line. + out = capsys.readouterr().out + assert "MAX DEPTH" not in out + assert "NAME" in out def test_interactive_empty_selection(self, drone, mocker, capsys): class FakePrompter: @@ -234,3 +246,105 @@ def confirm(self, question, default, flag): assert main(["logs"]) == 0 convert.assert_called_once() + + +class TestFilters: + def test_dives_only(self, drone, capsys): + assert main(["logs", "list", "--dives-only"]) == 0 + out = capsys.readouterr().out + assert "BYEDP000000_aaaa_00000" in out # is_dive=True + assert "BYEDP000000_aaaa_00001" not in out # is_dive=False + + def test_since_filters_older_logs(self, drone, capsys): + # Log _00001 starts at 1700100000 (~2023-11-16); _00000 at 1700000000 (~11-14). + assert main(["logs", "list", "--since", "2023-11-16"]) == 0 + out = capsys.readouterr().out + assert "BYEDP000000_aaaa_00001" in out + assert "BYEDP000000_aaaa_00000" not in out + + def test_until_filters_newer_logs(self, drone, capsys): + assert main(["logs", "list", "--until", "2023-11-15"]) == 0 + out = capsys.readouterr().out + assert "BYEDP000000_aaaa_00000" in out + assert "BYEDP000000_aaaa_00001" not in out + + def test_bad_date_errors_cleanly(self, drone, capsys): + assert main(["logs", "list", "--since", "tomorrow"]) == 1 + err = capsys.readouterr().err + assert "--since must be a date" in err # The message names the expected format. + + def test_filters_apply_to_download_all(self, drone, tmp_path): + assert main(["logs", "download", "--all", "--dives-only", "-o", str(tmp_path)]) == 0 + assert LogFile.download.call_count == 1 + + def test_no_match_message(self, drone, capsys): + assert main(["logs", "list", "--since", "2030-01-01"]) == 0 + assert "No logs match the filters" in capsys.readouterr().out + + +class TestSorting: + def test_list_sorted_descending(self, drone, capsys): + assert main(["logs", "list"]) == 0 + out = capsys.readouterr().out + assert out.index("BYEDP000000_aaaa_00001") < out.index("BYEDP000000_aaaa_00000") + + +class TestConvert: + @pytest.fixture + def bez_on_disk(self, tmp_path): + path = tmp_path / "mydive.bez" + path.write_bytes(b"bez-bytes") + return path + + def test_convert_writes_sibling_mcap(self, drone, mocker, bez_on_disk, capsys): + convert = mocker.patch( + "blueye.sdk.cli.commands.logs.mcap.convert_bez_to_mcap", return_value=7 + ) + assert main(["logs", "convert", str(bez_on_disk)]) == 0 + convert.assert_called_once_with(bez_on_disk, bez_on_disk.parent / "mydive.mcap") + assert "7 messages" in capsys.readouterr().out + # Purely local: the Drone class must never be constructed. + drone._drone_cls.assert_not_called() + + def test_convert_with_output_dir(self, drone, mocker, bez_on_disk, tmp_path): + convert = mocker.patch( + "blueye.sdk.cli.commands.logs.mcap.convert_bez_to_mcap", return_value=7 + ) + out_dir = tmp_path / "converted" + assert main(["logs", "convert", str(bez_on_disk), "-o", str(out_dir)]) == 0 + convert.assert_called_once_with(bez_on_disk, out_dir / "mydive.mcap") + assert out_dir.is_dir() + + def test_convert_multiple_files(self, drone, mocker, tmp_path): + convert = mocker.patch( + "blueye.sdk.cli.commands.logs.mcap.convert_bez_to_mcap", return_value=1 + ) + files = [] + for name in ("a.bez", "b.bez"): + path = tmp_path / name + path.write_bytes(b"x") + files.append(str(path)) + assert main(["logs", "convert", *files]) == 0 + assert convert.call_count == 2 + + def test_convert_missing_file_errors(self, drone, tmp_path, capsys): + assert main(["logs", "convert", str(tmp_path / "nope.bez")]) == 1 + assert "No such file" in capsys.readouterr().err + + def test_convert_missing_dependency_guidance(self, drone, mocker, bez_on_disk, capsys): + def fake_missing(names): + return [name for name in names if name == "mcap_protobuf"] + + mocker.patch("blueye.sdk.cli.deps.missing", side_effect=fake_missing) + assert main(["logs", "convert", str(bez_on_disk)]) == 1 + assert "blueye.sdk[cli]" in capsys.readouterr().out + + +class TestSearchFilterEnabled: + def test_questionary_checkbox_gets_search_filter(self, mocker): + from blueye.sdk.cli.prompts import QuestionaryPrompter + + checkbox = mocker.patch("questionary.checkbox") + checkbox.return_value.ask.return_value = [] + QuestionaryPrompter().checkbox("Pick:", ["a", "b"], "--flag") + assert checkbox.call_args.kwargs["use_search_filter"] is True diff --git a/uv.lock b/uv.lock index b797d3a1..b40bbd3b 100644 --- a/uv.lock +++ b/uv.lock @@ -144,7 +144,6 @@ examples = [ { name = "foxglove-websocket" }, { name = "inputs" }, { name = "matplotlib" }, - { name = "mcap-protobuf-support" }, { name = "pandas" }, { name = "pyserial" }, { name = "webdavclient3" }, @@ -180,7 +179,6 @@ requires-dist = [ { name = "inputs", marker = "extra == 'examples'", specifier = ">=0.5,<0.6" }, { name = "matplotlib", marker = "extra == 'examples'", specifier = "~=3.10" }, { name = "mcap-protobuf-support", marker = "extra == 'cli'", specifier = ">=0.5.3,<0.6" }, - { name = "mcap-protobuf-support", marker = "extra == 'examples'", specifier = ">=0.5.3,<0.6" }, { name = "onnx", marker = "extra == 'cli'", specifier = ">=1.16,<2" }, { name = "packaging", specifier = ">=24.2" }, { name = "pandas", marker = "extra == 'examples'", specifier = "~=2.2" }, From e3c2bd6ea34b926a4eae8b6e68401566c9b23d68 Mon Sep 17 00:00:00 2001 From: Juan Pablo Pino Bravo Date: Fri, 10 Jul 2026 17:15:08 +0200 Subject: [PATCH 5/7] fix: scope checkbox toggle-all/invert to the filtered rows MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Works around a second questionary 2.1.1 search-filter bug (user-found): with a filter active, ctrl-a (toggle all) and ctrl-i/tab (invert) operated on every choice — selecting files not even in view — because questionary's handlers iterate ic.choices instead of ic.filtered_choices. The prompter now replaces both key bindings on the constructed prompt with versions scoped to the visible rows: toggle-all selects/deselects only what the filter shows, invert flips only the visible rows, and selections hidden by the filter are left untouched. The selection logic lives in pure helpers (_toggle_all_visible, _invert_visible) with unit tests, plus an integration test asserting the rebinding attaches to a real questionary prompt. +5 tests (456 total). Co-Authored-By: Claude Fable 5 --- blueye/sdk/cli/prompts.py | 90 ++++++++++++++++++++++++++++------ tests/test_cli_logs_command.py | 3 ++ tests/test_cli_prompts.py | 68 +++++++++++++++++++++++++ 3 files changed, 147 insertions(+), 14 deletions(-) diff --git a/blueye/sdk/cli/prompts.py b/blueye/sdk/cli/prompts.py index 4b9fb845..8815e78d 100644 --- a/blueye/sdk/cli/prompts.py +++ b/blueye/sdk/cli/prompts.py @@ -44,6 +44,68 @@ def _require(answer: object) -> object: return answer +def _visible_values(inquirer_control) -> list: + """The selectable values currently shown (respecting the active search filter).""" + from questionary.prompts.common import Separator + + return [ + choice.value + for choice in inquirer_control.filtered_choices + if not isinstance(choice, Separator) and not choice.disabled + ] + + +def _toggle_all_visible(inquirer_control) -> None: + """Select every visible row, or deselect them when all are already selected.""" + visible = _visible_values(inquirer_control) + if visible and all(value in inquirer_control.selected_options for value in visible): + for value in visible: + inquirer_control.selected_options.remove(value) + else: + for value in visible: + if value not in inquirer_control.selected_options: + inquirer_control.selected_options.append(value) + + +def _invert_visible(inquirer_control) -> None: + """Invert the selection of the visible rows, leaving hidden selections intact.""" + for value in _visible_values(inquirer_control): + if value in inquirer_control.selected_options: + inquirer_control.selected_options.remove(value) + else: + inquirer_control.selected_options.append(value) + + +def _scope_bulk_bindings_to_filter(question) -> None: + """Make ctrl-a (toggle all) and ctrl-i/tab (invert) respect the search filter. + + Works around a questionary 2.1.1 bug: with ``use_search_filter=True`` its + toggle-all/invert handlers iterate every choice instead of the filtered view, so + filtering and then pressing ctrl-a selected files the user could not even see. + The original bindings are replaced with ones scoped to `filtered_choices`. + """ + from prompt_toolkit.keys import Keys + from questionary.prompts.common import InquirerControl + + application = question.application + inquirer_control = next( + control + for control in application.layout.find_all_controls() + if isinstance(control, InquirerControl) + ) + bindings = application.key_bindings + bindings.remove(Keys.ControlA) + bindings.remove(Keys.ControlI) + + @bindings.add(Keys.ControlA, eager=True) + def _toggle_all(_event): + _toggle_all_visible(inquirer_control) + + @bindings.add(Keys.ControlI, eager=True) + def _invert(_event): + _invert_visible(inquirer_control) + + class QuestionaryPrompter: """Interactive prompts with arrow-key selection and path autocompletion.""" @@ -71,21 +133,21 @@ def path(self, question: str, default: str | None, flag: str) -> str: return str(_require(questionary.path(question, default=default or "").ask())) def checkbox(self, question: str, choices: Sequence[str], flag: str) -> list[str]: - answer = _require( - questionary.checkbox( - question, - choices=list(choices), - use_search_filter=True, - use_jk_keys=False, - # questionary 2.1.1's default instruction wrongly shows for - # both actions when the search filter is on; the real bindings are - # ctrl-a = toggle all and ctrl-i (tab) = invert. - instruction=( - "(use arrow keys to move, to select, to toggle " - "all, to invert, type to filter)" - ), - ).ask() + prompt = questionary.checkbox( + question, + choices=list(choices), + use_search_filter=True, + use_jk_keys=False, + # questionary 2.1.1's default instruction wrongly shows for + # both actions when the search filter is on; the real bindings are + # ctrl-a = toggle all and ctrl-i (tab) = invert. + instruction=( + "(use arrow keys to move, to select, to toggle " + "all, to invert, type to filter)" + ), ) + _scope_bulk_bindings_to_filter(prompt) + answer = _require(prompt.ask()) return [str(item) for item in answer] diff --git a/tests/test_cli_logs_command.py b/tests/test_cli_logs_command.py index 7a75478e..c68a0cde 100644 --- a/tests/test_cli_logs_command.py +++ b/tests/test_cli_logs_command.py @@ -346,5 +346,8 @@ def test_questionary_checkbox_gets_search_filter(self, mocker): checkbox = mocker.patch("questionary.checkbox") checkbox.return_value.ask.return_value = [] + # The binding rework runs on the real prompt object; not exercisable on a Mock. + scope = mocker.patch("blueye.sdk.cli.prompts._scope_bulk_bindings_to_filter") QuestionaryPrompter().checkbox("Pick:", ["a", "b"], "--flag") assert checkbox.call_args.kwargs["use_search_filter"] is True + scope.assert_called_once_with(checkbox.return_value) diff --git a/tests/test_cli_prompts.py b/tests/test_cli_prompts.py index 40767f98..4adb4395 100644 --- a/tests/test_cli_prompts.py +++ b/tests/test_cli_prompts.py @@ -72,3 +72,71 @@ def test_guidance_uses_single_quotes_with_uv_on_windows(self, mocker, capsys): deps.print_install_guidance(["onnx"]) out = capsys.readouterr().out assert "uv pip install 'blueye.sdk[cli]'" in out + + +class _StubChoice: + def __init__(self, value, disabled=False): + self.value = value + self.disabled = disabled + + +class _StubControl: + """Mimics questionary's InquirerControl selection state for the bulk helpers.""" + + def __init__(self, visible, selected=()): + self.filtered_choices = [_StubChoice(value) for value in visible] + self.selected_options = list(selected) + + +class TestFilterScopedBulkActions: + def test_toggle_all_selects_only_visible(self): + from blueye.sdk.cli.prompts import _toggle_all_visible + + control = _StubControl(visible=["a", "b"], selected=["hidden"]) + _toggle_all_visible(control) + assert sorted(control.selected_options) == ["a", "b", "hidden"] + + def test_toggle_all_deselects_when_all_visible_selected(self): + from blueye.sdk.cli.prompts import _toggle_all_visible + + control = _StubControl(visible=["a", "b"], selected=["a", "b", "hidden"]) + _toggle_all_visible(control) + assert control.selected_options == ["hidden"] + + def test_invert_only_touches_visible(self): + from blueye.sdk.cli.prompts import _invert_visible + + control = _StubControl(visible=["a", "b"], selected=["a", "hidden"]) + _invert_visible(control) + assert sorted(control.selected_options) == ["b", "hidden"] + + def test_disabled_choices_are_skipped(self): + from blueye.sdk.cli.prompts import _toggle_all_visible + + control = _StubControl(visible=["a"]) + control.filtered_choices.append(_StubChoice("locked", disabled=True)) + _toggle_all_visible(control) + assert control.selected_options == ["a"] + + def test_rebinding_attaches_to_real_prompt(self): + """The workaround must find the control and replace both key bindings.""" + import questionary + from prompt_toolkit.keys import Keys + + from blueye.sdk.cli.prompts import _scope_bulk_bindings_to_filter + + prompt = questionary.checkbox( + "Pick:", choices=["a", "b"], use_search_filter=True, use_jk_keys=False + ) + _scope_bulk_bindings_to_filter(prompt) + bindings = prompt.application.key_bindings + + def exact(keys): + # get_bindings_for_keys also returns the search-character catch-all; + # only the exact-key binding handles the shortcut at dispatch time. + return [b for b in bindings.get_bindings_for_keys(keys) if b.keys == keys] + + toggle = exact((Keys.ControlA,)) + invert = exact((Keys.ControlI,)) + assert len(toggle) == 1 and toggle[0].handler.__name__ == "_toggle_all" + assert len(invert) == 1 and invert[0].handler.__name__ == "_invert" From bb7e5fba5ba3335c6291c7a8c03774ec522663ba Mon Sep 17 00:00:00 2001 From: Juan Pablo Pino Bravo Date: Fri, 10 Jul 2026 17:18:35 +0200 Subject: [PATCH 6/7] fix: construct the rebinding test prompt headlessly for Windows CI prompt_toolkit raises NoConsoleScreenBufferError when questionary builds a prompt without a console; the test now uses a pipe input and DummyOutput app session. Co-Authored-By: Claude Fable 5 --- tests/test_cli_prompts.py | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/tests/test_cli_prompts.py b/tests/test_cli_prompts.py index 4adb4395..b0d20066 100644 --- a/tests/test_cli_prompts.py +++ b/tests/test_cli_prompts.py @@ -120,15 +120,25 @@ def test_disabled_choices_are_skipped(self): def test_rebinding_attaches_to_real_prompt(self): """The workaround must find the control and replace both key bindings.""" + import contextlib + import questionary + from prompt_toolkit.application import create_app_session + from prompt_toolkit.input import create_pipe_input from prompt_toolkit.keys import Keys + from prompt_toolkit.output import DummyOutput from blueye.sdk.cli.prompts import _scope_bulk_bindings_to_filter - prompt = questionary.checkbox( - "Pick:", choices=["a", "b"], use_search_filter=True, use_jk_keys=False - ) - _scope_bulk_bindings_to_filter(prompt) + with contextlib.ExitStack() as stack: + # Windows CI has no console; give prompt_toolkit a pipe input and a + # dummy output so the prompt can be constructed headlessly. + pipe_input = stack.enter_context(create_pipe_input()) + stack.enter_context(create_app_session(input=pipe_input, output=DummyOutput())) + prompt = questionary.checkbox( + "Pick:", choices=["a", "b"], use_search_filter=True, use_jk_keys=False + ) + _scope_bulk_bindings_to_filter(prompt) bindings = prompt.application.key_bindings def exact(keys): From 44cdcad561cf68f473595647100856c971722151 Mon Sep 17 00:00:00 2001 From: Juan Pablo Pino Bravo Date: Fri, 10 Jul 2026 17:38:10 +0200 Subject: [PATCH 7/7] fix: reject combined download selectors and non-positive --latest Addresses the Copilot review on #220: names/--latest/--all are now mutually exclusive instead of silently prioritized, and --latest must be at least 1 rather than succeeding with an empty selection. Co-Authored-By: Claude Fable 5 --- blueye/sdk/cli/commands/logs/command.py | 5 +++++ tests/test_cli_logs_command.py | 18 ++++++++++++++++++ 2 files changed, 23 insertions(+) diff --git a/blueye/sdk/cli/commands/logs/command.py b/blueye/sdk/cli/commands/logs/command.py index 56367cec..12a31f88 100644 --- a/blueye/sdk/cli/commands/logs/command.py +++ b/blueye/sdk/cli/commands/logs/command.py @@ -218,10 +218,15 @@ def _run_convert(console, args) -> int: def _select_downloads(args, log_files) -> list: """Resolve the download selection from names/--latest/--all.""" + selectors = [bool(args.names), args.latest is not None, args.all] + if sum(selectors) > 1: + raise CliError("Pass only one of log names, --latest N, or --all.") by_name = {log.name: log for log in log_files} if args.all: return list(log_files) if args.latest is not None: + if args.latest < 1: + raise CliError("--latest must be a positive number of logs.") newest_first = sorted(log_files, key=lambda log: log.start_time, reverse=True) return newest_first[: args.latest] if args.names: diff --git a/tests/test_cli_logs_command.py b/tests/test_cli_logs_command.py index c68a0cde..c1b73329 100644 --- a/tests/test_cli_logs_command.py +++ b/tests/test_cli_logs_command.py @@ -90,6 +90,24 @@ def test_no_selector_errors(self, drone, capsys): assert main(["logs", "download"]) == 1 assert "--latest" in capsys.readouterr().err + def test_combined_selectors_error(self, drone, capsys): + assert main(["logs", "download", "BYEDP000000_aaaa_00000", "--all"]) == 1 + assert "only one of" in capsys.readouterr().err + LogFile.download.assert_not_called() + + def test_latest_with_names_errors(self, drone, capsys): + assert main(["logs", "download", "BYEDP000000_aaaa_00000", "--latest", "1"]) == 1 + assert "only one of" in capsys.readouterr().err + LogFile.download.assert_not_called() + + def test_latest_zero_errors(self, drone, capsys): + assert main(["logs", "download", "--latest", "0"]) == 1 + assert "--latest must be a positive number" in capsys.readouterr().err + + def test_latest_negative_errors(self, drone, capsys): + assert main(["logs", "download", "--latest", "-1"]) == 1 + assert "--latest must be a positive number" in capsys.readouterr().err + class TestFailureHandling: def test_unreachable_drone_is_friendly(self, drone, capsys):