From fc17feb73b779ab7669b5276f42b90baea754e7a Mon Sep 17 00:00:00 2001 From: xinbingnan Date: Fri, 5 Jun 2026 21:13:10 +0800 Subject: [PATCH 1/2] fix(cli): honor optional --port via lazy local-server attach The v1.6.6 docs and feat(cli) commit (2c1bf58) state '--port is optional in proxy mode (server owns the serial port)'. This was implemented for remote --server-url, but local mode still returned offline immediately when --port was omitted, so device commands run through 'fpb_cli.py' without --port failed against a locally-running server with a connected device. Approach: - Init no longer probes the network. Without --port, it just remembers server_url/token/baudrate ('pending local server'). This keeps FPBCLI() instantiation side-effect-free, so unit tests do not depend on ambient network state. - Add try_attach_local_server(), called from main() before dispatching any device-needing command, which lazily probes the local server and attaches if /api/status reports connected=true. - Add ServerProxy.probe_status(): single short-timeout probe returning the raw /api/status dict. Replaces the old is_server_running() + is_device_connected() pair so we hit /api/status only once and avoid any inconsistency between the two responses (per Copilot review). - Verbose 'Using WebServer proxy mode' log is gated on _device_state.connected so a disconnect race during attach does not produce a misleading message. Tests: - test_init_no_port_no_probe: FPBCLI() must not call ServerProxy at all. - test_try_attach_local_server_*: device connected / no device / unreachable. Mocks probe_status() and asserts is_server_running() is not called, locking in the single-probe contract. - TestFPBCLICommands.run_cli pins --server-url to http://127.0.0.1:1 so subprocess-based command tests do not pick up an unrelated local WebServer in the developer's environment. --- Tools/WebServer/cli/fpb_cli.py | 76 +++++++++++++++++++++++--- Tools/WebServer/cli/server_proxy.py | 13 +++++ Tools/WebServer/tests/test_fpb_cli.py | 79 ++++++++++++++++++++++++++- 3 files changed, 158 insertions(+), 10 deletions(-) diff --git a/Tools/WebServer/cli/fpb_cli.py b/Tools/WebServer/cli/fpb_cli.py index 935c7af5..ecc2f302 100755 --- a/Tools/WebServer/cli/fpb_cli.py +++ b/Tools/WebServer/cli/fpb_cli.py @@ -126,6 +126,9 @@ def __init__( # Proxy and lock state self._proxy = None self._port_lock = None + self._pending_local_server_url = None + self._pending_local_token = None + self._pending_local_baudrate = baudrate remote = self._is_remote_url(server_url) @@ -149,11 +152,15 @@ def __init__( self._init_remote_proxy(server_url, port, baudrate, token) return - # Local mode. A serial port is needed to engage the proxy/device: - # locally the device is attached to this machine, and auto-launch - # needs to know which port to open. Without a port we stay offline so - # ELF analysis / compile commands run fast and self-contained. + # Local mode. Without --port we stay offline by default so ELF + # analysis / compile commands run fast and self-contained. Callers + # that need a device without --port can call + # try_attach_local_server() to opportunistically attach to a + # locally-running server that already owns a connected device. if not port: + self._pending_local_server_url = server_url + self._pending_local_token = token + self._pending_local_baudrate = baudrate return proxy = ServerProxy(base_url=server_url, token=token) @@ -179,6 +186,39 @@ def __init__( logging.warning("Auto-launch failed, falling back to direct mode") self._direct_connect(port, baudrate) + def try_attach_local_server(self) -> bool: + """Opportunistically attach to a local WebServer with a connected device. + + Called from main() before dispatching device-needing commands when the + user did not supply --port. Returns True if a proxy was attached or + was already attached; False if no usable local server is reachable. + Init keeps this lazy so unit tests instantiating FPBCLI() never trigger + a network probe. + """ + if self._proxy is not None: + return True + if not self._pending_local_server_url: + return False + + proxy = ServerProxy( + base_url=self._pending_local_server_url, + token=self._pending_local_token, + ) + # Single probe instead of is_server_running()+is_device_connected() + # so we avoid two /api/status round-trips and any inconsistency + # between them under races. + status = proxy.probe_status() + if not (status.get("success") and status.get("connected")): + return False + + self._attach_proxy(proxy, None, self._pending_local_baudrate) + if self.verbose and self._device_state.connected: + logging.info( + f"Using WebServer proxy mode " + f"({self._pending_local_server_url}, server-owned port)" + ) + return self._device_state.connected + @staticmethod def _is_remote_url(server_url: str) -> bool: """Return True if server_url points to a non-localhost host. @@ -1182,19 +1222,25 @@ def main(): fpb_cli.py search firmware.elf gpio fpb_cli.py compile patch.c --elf firmware.elf --compile-commands build/compile_commands.json - # Local device commands — auto-launch/attach to a WebServer: + # Local device commands — first time, --port triggers auto-launch: fpb_cli.py --port /dev/ttyACM0 info fpb_cli.py --port /dev/ttyACM0 inject digitalWrite patch.c --elf firmware.elf + # Once a local server has the device connected, --port is no longer needed: + fpb_cli.py info + fpb_cli.py file-stat /etc/init.d/rcS + # Remote control — the device lives on the server, so no --port needed: fpb_cli.py --server-url http://192.168.1.20:5500 --token TOKEN info # Only pass --port to tell the remote server which port to open if it has none: fpb_cli.py --server-url http://192.168.1.20:5500 --token TOKEN --port /dev/ttyACM0 connect Notes: - The serial port belongs to the WebServer, not the CLI. In proxy mode --port - is optional and only used to ask the server to open a port when no device is - connected yet. --token (or FPB_TOKEN env) is required for remote servers. + The serial port belongs to the WebServer, not the CLI. In proxy mode (local + or remote) --port is optional whenever the server already has a connected + device; it is only required to open a port when no device is connected yet, + or for direct/auto-launch on a fresh local environment. + --token (or FPB_TOKEN env) is required for remote servers. Output is JSON on stdout; pipe to jq for filtering. Run 'fpb_cli.py --help' for command-specific options. """, @@ -1540,6 +1586,20 @@ def main(): token=args.token, ) + OFFLINE_COMMANDS = { + "analyze", + "disasm", + "decompile", + "signature", + "search", + "get-symbols", + "compile", + "server-stop", + "disconnect", + } + if args.command not in OFFLINE_COMMANDS: + cli.try_attach_local_server() + try: if args.command == "analyze": cli.analyze(args.elf_path, args.func_name) diff --git a/Tools/WebServer/cli/server_proxy.py b/Tools/WebServer/cli/server_proxy.py index a04e0957..0643efbb 100644 --- a/Tools/WebServer/cli/server_proxy.py +++ b/Tools/WebServer/cli/server_proxy.py @@ -231,6 +231,19 @@ def get_status(self) -> dict: """Get full WebServer status.""" return self._get("/api/status") + def probe_status(self) -> dict: + """One-shot status probe with the short PROBE timeout. + + Returns the raw /api/status response (with at least 'success' and + 'connected' keys), or an empty dict on any error. Use this when a + caller needs both reachability and connection state in one call; + avoids issuing two probes that may also disagree under races. + """ + try: + return self._get("/api/status", timeout=_PROBE_TIMEOUT) + except Exception: + return {} + def ensure_server(self) -> bool: """Ensure the WebServer is running, auto-launching if needed. diff --git a/Tools/WebServer/tests/test_fpb_cli.py b/Tools/WebServer/tests/test_fpb_cli.py index 6958a03c..7e863f69 100644 --- a/Tools/WebServer/tests/test_fpb_cli.py +++ b/Tools/WebServer/tests/test_fpb_cli.py @@ -140,6 +140,71 @@ def test_init_with_port(self, mock_serial, mock_port_lock): self.assertTrue(cli._device_state.connected) cli.cleanup() + def test_init_no_port_no_probe(self): + """FPBCLI() without --port must not perform any network probe. + + This guarantees offline ELF/compile commands stay self-contained + and unit tests do not depend on ambient network state. + """ + with patch("cli.fpb_cli.ServerProxy") as mock_proxy_cls: + cli = FPBCLI() + try: + mock_proxy_cls.assert_not_called() + self.assertIsNone(cli._proxy) + self.assertFalse(cli._device_state.connected) + finally: + cli.cleanup() + + @patch("cli.fpb_cli.ServerProxy") + def test_try_attach_local_server_with_device(self, mock_proxy_cls): + """try_attach_local_server attaches when local server has a connected device.""" + proxy = MagicMock() + proxy.probe_status.return_value = {"success": True, "connected": True} + proxy.is_device_connected.return_value = True + mock_proxy_cls.return_value = proxy + + cli = FPBCLI() + try: + self.assertTrue(cli.try_attach_local_server()) + self.assertIs(cli._proxy, proxy) + self.assertTrue(cli._device_state.connected) + proxy.connect.assert_not_called() + # Single status probe, no extra is_server_running()/is_device_connected() round-trip. + proxy.probe_status.assert_called_once() + proxy.is_server_running.assert_not_called() + finally: + cli.cleanup() + + @patch("cli.fpb_cli.ServerProxy") + def test_try_attach_local_server_without_device(self, mock_proxy_cls): + """Server running but no device → try_attach_local_server stays offline.""" + proxy = MagicMock() + proxy.probe_status.return_value = {"success": True, "connected": False} + mock_proxy_cls.return_value = proxy + + cli = FPBCLI() + try: + self.assertFalse(cli.try_attach_local_server()) + self.assertIsNone(cli._proxy) + self.assertFalse(cli._device_state.connected) + proxy.connect.assert_not_called() + finally: + cli.cleanup() + + @patch("cli.fpb_cli.ServerProxy") + def test_try_attach_local_server_unreachable(self, mock_proxy_cls): + """No server running → try_attach_local_server returns False, no exception.""" + proxy = MagicMock() + proxy.probe_status.return_value = {} + mock_proxy_cls.return_value = proxy + + cli = FPBCLI() + try: + self.assertFalse(cli.try_attach_local_server()) + self.assertIsNone(cli._proxy) + finally: + cli.cleanup() + def test_output_json(self): """Test JSON output formatting""" data = {"success": True, "message": "Test"} @@ -1731,8 +1796,18 @@ def setUpClass(cls): cls.cli_path = Path(__file__).parent.parent / "fpb_cli.py" def run_cli(self, *args): - """Helper to run CLI and parse JSON output""" - cmd = [sys.executable, str(self.cli_path)] + list(args) + """Helper to run CLI and parse JSON output. + + Pins --server-url to an unreachable local port so tests stay + deterministic even when an unrelated WebServer is listening on + the default 5500 on the developer's machine. + """ + cmd = [ + sys.executable, + str(self.cli_path), + "--server-url", + "http://127.0.0.1:1", + ] + list(args) result = subprocess.run(cmd, capture_output=True, text=True) try: return json.loads(result.stdout), result.returncode From 593f648d613f7d1b9ed86716e4c671dc7a3a8be8 Mon Sep 17 00:00:00 2001 From: xinbingnan Date: Fri, 5 Jun 2026 21:13:10 +0800 Subject: [PATCH 2/2] docs(cli): clarify port-less mode for local servers The 'About --port' section already promised that --port is optional when the server has a connected device, but the operating modes table only listed port-less attach implicitly via the remote-proxy row. - State that port-less attach applies to BOTH local and remote. - Add a 'Local proxy (port-less)' row to the modes table. - Clarify that 'offline fallback' is local-only: in remote mode the CLI still attaches the proxy when the server is reachable even if no device is connected (per Copilot review). --- Docs/CLI.md | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/Docs/CLI.md b/Docs/CLI.md index 7c4639f3..42f91fd8 100644 --- a/Docs/CLI.md +++ b/Docs/CLI.md @@ -47,21 +47,24 @@ Options: ### About `--port` -The serial port belongs to the **WebServer**, not the CLI. When a server is already running and has a device connected, `--port` is not needed — the CLI attaches to the server's existing connection. +The serial port belongs to the **WebServer**, not the CLI. When a server is already running and has a device connected, `--port` is not needed — the CLI attaches to the server's existing connection. This applies to **both local and remote** servers: as long as `/api/status` reports `connected=true`, you can run device commands without `--port`. `--port` is only required when: - No server is running locally (triggers auto-launch + direct fallback). -- The remote server has no device connected yet (tells it which port to open). +- The local or remote server is reachable but has no device connected yet (tells it which port to open). + +Without `--port` the CLI never opens a serial port directly — it either attaches to a server that already owns one, or stays offline (ELF analysis / compile commands always work). In remote mode the CLI still attaches to a reachable server even before a device is connected; device commands will fail until you connect a device, but offline ELF/compile commands remain available. ## Operating Modes | Mode | Trigger | Behavior | |------|---------|----------| -| Offline | No `--port`, no server | ELF analysis / compile only | +| Offline (local) | No `--port`, no local server (or server has no device) | ELF analysis / compile only | +| Local proxy (port-less) | No `--port`, local server already has a device | Attach to server's existing connection | | Local proxy | `--port` + local server running | Attach to server, forward device ops | | Local auto-launch | `--port` + no local server | Auto-launch server, then proxy | | Local direct | `--direct --port` | Open serial directly (bypass server) | -| Remote proxy | `--server-url http://remote:port` | Pure proxy to remote server, no auto-launch | +| Remote proxy | `--server-url http://remote:port` | Pure proxy to remote server, no auto-launch (attaches even if no device yet) | ## Remote Control