Skip to content

Commit 3482839

Browse files
authored
fix(cli): honor optional --port in local proxy mode (#11)
* 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. * 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).
1 parent 36055f3 commit 3482839

4 files changed

Lines changed: 165 additions & 14 deletions

File tree

Docs/CLI.md

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -47,21 +47,24 @@ Options:
4747

4848
### About `--port`
4949

50-
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.
50+
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`.
5151

5252
`--port` is only required when:
5353
- No server is running locally (triggers auto-launch + direct fallback).
54-
- The remote server has no device connected yet (tells it which port to open).
54+
- The local or remote server is reachable but has no device connected yet (tells it which port to open).
55+
56+
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.
5557

5658
## Operating Modes
5759

5860
| Mode | Trigger | Behavior |
5961
|------|---------|----------|
60-
| Offline | No `--port`, no server | ELF analysis / compile only |
62+
| Offline (local) | No `--port`, no local server (or server has no device) | ELF analysis / compile only |
63+
| Local proxy (port-less) | No `--port`, local server already has a device | Attach to server's existing connection |
6164
| Local proxy | `--port` + local server running | Attach to server, forward device ops |
6265
| Local auto-launch | `--port` + no local server | Auto-launch server, then proxy |
6366
| Local direct | `--direct --port` | Open serial directly (bypass server) |
64-
| Remote proxy | `--server-url http://remote:port` | Pure proxy to remote server, no auto-launch |
67+
| Remote proxy | `--server-url http://remote:port` | Pure proxy to remote server, no auto-launch (attaches even if no device yet) |
6568

6669
## Remote Control
6770

Tools/WebServer/cli/fpb_cli.py

Lines changed: 68 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,9 @@ def __init__(
126126
# Proxy and lock state
127127
self._proxy = None
128128
self._port_lock = None
129+
self._pending_local_server_url = None
130+
self._pending_local_token = None
131+
self._pending_local_baudrate = baudrate
129132

130133
remote = self._is_remote_url(server_url)
131134

@@ -149,11 +152,15 @@ def __init__(
149152
self._init_remote_proxy(server_url, port, baudrate, token)
150153
return
151154

152-
# Local mode. A serial port is needed to engage the proxy/device:
153-
# locally the device is attached to this machine, and auto-launch
154-
# needs to know which port to open. Without a port we stay offline so
155-
# ELF analysis / compile commands run fast and self-contained.
155+
# Local mode. Without --port we stay offline by default so ELF
156+
# analysis / compile commands run fast and self-contained. Callers
157+
# that need a device without --port can call
158+
# try_attach_local_server() to opportunistically attach to a
159+
# locally-running server that already owns a connected device.
156160
if not port:
161+
self._pending_local_server_url = server_url
162+
self._pending_local_token = token
163+
self._pending_local_baudrate = baudrate
157164
return
158165

159166
proxy = ServerProxy(base_url=server_url, token=token)
@@ -179,6 +186,39 @@ def __init__(
179186
logging.warning("Auto-launch failed, falling back to direct mode")
180187
self._direct_connect(port, baudrate)
181188

189+
def try_attach_local_server(self) -> bool:
190+
"""Opportunistically attach to a local WebServer with a connected device.
191+
192+
Called from main() before dispatching device-needing commands when the
193+
user did not supply --port. Returns True if a proxy was attached or
194+
was already attached; False if no usable local server is reachable.
195+
Init keeps this lazy so unit tests instantiating FPBCLI() never trigger
196+
a network probe.
197+
"""
198+
if self._proxy is not None:
199+
return True
200+
if not self._pending_local_server_url:
201+
return False
202+
203+
proxy = ServerProxy(
204+
base_url=self._pending_local_server_url,
205+
token=self._pending_local_token,
206+
)
207+
# Single probe instead of is_server_running()+is_device_connected()
208+
# so we avoid two /api/status round-trips and any inconsistency
209+
# between them under races.
210+
status = proxy.probe_status()
211+
if not (status.get("success") and status.get("connected")):
212+
return False
213+
214+
self._attach_proxy(proxy, None, self._pending_local_baudrate)
215+
if self.verbose and self._device_state.connected:
216+
logging.info(
217+
f"Using WebServer proxy mode "
218+
f"({self._pending_local_server_url}, server-owned port)"
219+
)
220+
return self._device_state.connected
221+
182222
@staticmethod
183223
def _is_remote_url(server_url: str) -> bool:
184224
"""Return True if server_url points to a non-localhost host.
@@ -1182,19 +1222,25 @@ def main():
11821222
fpb_cli.py search firmware.elf gpio
11831223
fpb_cli.py compile patch.c --elf firmware.elf --compile-commands build/compile_commands.json
11841224
1185-
# Local device commands — auto-launch/attach to a WebServer:
1225+
# Local device commands — first time, --port triggers auto-launch:
11861226
fpb_cli.py --port /dev/ttyACM0 info
11871227
fpb_cli.py --port /dev/ttyACM0 inject digitalWrite patch.c --elf firmware.elf
11881228
1229+
# Once a local server has the device connected, --port is no longer needed:
1230+
fpb_cli.py info
1231+
fpb_cli.py file-stat /etc/init.d/rcS
1232+
11891233
# Remote control — the device lives on the server, so no --port needed:
11901234
fpb_cli.py --server-url http://192.168.1.20:5500 --token TOKEN info
11911235
# Only pass --port to tell the remote server which port to open if it has none:
11921236
fpb_cli.py --server-url http://192.168.1.20:5500 --token TOKEN --port /dev/ttyACM0 connect
11931237
11941238
Notes:
1195-
The serial port belongs to the WebServer, not the CLI. In proxy mode --port
1196-
is optional and only used to ask the server to open a port when no device is
1197-
connected yet. --token (or FPB_TOKEN env) is required for remote servers.
1239+
The serial port belongs to the WebServer, not the CLI. In proxy mode (local
1240+
or remote) --port is optional whenever the server already has a connected
1241+
device; it is only required to open a port when no device is connected yet,
1242+
or for direct/auto-launch on a fresh local environment.
1243+
--token (or FPB_TOKEN env) is required for remote servers.
11981244
Output is JSON on stdout; pipe to jq for filtering.
11991245
Run 'fpb_cli.py <command> --help' for command-specific options.
12001246
""",
@@ -1540,6 +1586,20 @@ def main():
15401586
token=args.token,
15411587
)
15421588

1589+
OFFLINE_COMMANDS = {
1590+
"analyze",
1591+
"disasm",
1592+
"decompile",
1593+
"signature",
1594+
"search",
1595+
"get-symbols",
1596+
"compile",
1597+
"server-stop",
1598+
"disconnect",
1599+
}
1600+
if args.command not in OFFLINE_COMMANDS:
1601+
cli.try_attach_local_server()
1602+
15431603
try:
15441604
if args.command == "analyze":
15451605
cli.analyze(args.elf_path, args.func_name)

Tools/WebServer/cli/server_proxy.py

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -231,6 +231,19 @@ def get_status(self) -> dict:
231231
"""Get full WebServer status."""
232232
return self._get("/api/status")
233233

234+
def probe_status(self) -> dict:
235+
"""One-shot status probe with the short PROBE timeout.
236+
237+
Returns the raw /api/status response (with at least 'success' and
238+
'connected' keys), or an empty dict on any error. Use this when a
239+
caller needs both reachability and connection state in one call;
240+
avoids issuing two probes that may also disagree under races.
241+
"""
242+
try:
243+
return self._get("/api/status", timeout=_PROBE_TIMEOUT)
244+
except Exception:
245+
return {}
246+
234247
def ensure_server(self) -> bool:
235248
"""Ensure the WebServer is running, auto-launching if needed.
236249

Tools/WebServer/tests/test_fpb_cli.py

Lines changed: 77 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,71 @@ def test_init_with_port(self, mock_serial, mock_port_lock):
140140
self.assertTrue(cli._device_state.connected)
141141
cli.cleanup()
142142

143+
def test_init_no_port_no_probe(self):
144+
"""FPBCLI() without --port must not perform any network probe.
145+
146+
This guarantees offline ELF/compile commands stay self-contained
147+
and unit tests do not depend on ambient network state.
148+
"""
149+
with patch("cli.fpb_cli.ServerProxy") as mock_proxy_cls:
150+
cli = FPBCLI()
151+
try:
152+
mock_proxy_cls.assert_not_called()
153+
self.assertIsNone(cli._proxy)
154+
self.assertFalse(cli._device_state.connected)
155+
finally:
156+
cli.cleanup()
157+
158+
@patch("cli.fpb_cli.ServerProxy")
159+
def test_try_attach_local_server_with_device(self, mock_proxy_cls):
160+
"""try_attach_local_server attaches when local server has a connected device."""
161+
proxy = MagicMock()
162+
proxy.probe_status.return_value = {"success": True, "connected": True}
163+
proxy.is_device_connected.return_value = True
164+
mock_proxy_cls.return_value = proxy
165+
166+
cli = FPBCLI()
167+
try:
168+
self.assertTrue(cli.try_attach_local_server())
169+
self.assertIs(cli._proxy, proxy)
170+
self.assertTrue(cli._device_state.connected)
171+
proxy.connect.assert_not_called()
172+
# Single status probe, no extra is_server_running()/is_device_connected() round-trip.
173+
proxy.probe_status.assert_called_once()
174+
proxy.is_server_running.assert_not_called()
175+
finally:
176+
cli.cleanup()
177+
178+
@patch("cli.fpb_cli.ServerProxy")
179+
def test_try_attach_local_server_without_device(self, mock_proxy_cls):
180+
"""Server running but no device → try_attach_local_server stays offline."""
181+
proxy = MagicMock()
182+
proxy.probe_status.return_value = {"success": True, "connected": False}
183+
mock_proxy_cls.return_value = proxy
184+
185+
cli = FPBCLI()
186+
try:
187+
self.assertFalse(cli.try_attach_local_server())
188+
self.assertIsNone(cli._proxy)
189+
self.assertFalse(cli._device_state.connected)
190+
proxy.connect.assert_not_called()
191+
finally:
192+
cli.cleanup()
193+
194+
@patch("cli.fpb_cli.ServerProxy")
195+
def test_try_attach_local_server_unreachable(self, mock_proxy_cls):
196+
"""No server running → try_attach_local_server returns False, no exception."""
197+
proxy = MagicMock()
198+
proxy.probe_status.return_value = {}
199+
mock_proxy_cls.return_value = proxy
200+
201+
cli = FPBCLI()
202+
try:
203+
self.assertFalse(cli.try_attach_local_server())
204+
self.assertIsNone(cli._proxy)
205+
finally:
206+
cli.cleanup()
207+
143208
def test_output_json(self):
144209
"""Test JSON output formatting"""
145210
data = {"success": True, "message": "Test"}
@@ -1731,8 +1796,18 @@ def setUpClass(cls):
17311796
cls.cli_path = Path(__file__).parent.parent / "fpb_cli.py"
17321797

17331798
def run_cli(self, *args):
1734-
"""Helper to run CLI and parse JSON output"""
1735-
cmd = [sys.executable, str(self.cli_path)] + list(args)
1799+
"""Helper to run CLI and parse JSON output.
1800+
1801+
Pins --server-url to an unreachable local port so tests stay
1802+
deterministic even when an unrelated WebServer is listening on
1803+
the default 5500 on the developer's machine.
1804+
"""
1805+
cmd = [
1806+
sys.executable,
1807+
str(self.cli_path),
1808+
"--server-url",
1809+
"http://127.0.0.1:1",
1810+
] + list(args)
17361811
result = subprocess.run(cmd, capture_output=True, text=True)
17371812
try:
17381813
return json.loads(result.stdout), result.returncode

0 commit comments

Comments
 (0)