fix(cli): honor optional --port in local proxy mode#11
Merged
Conversation
W-Mai
force-pushed
the
fix-cli-local-noport-attach
branch
from
4643236 to
ea2ee00
Compare
There was a problem hiding this comment.
Pull request overview
This PR updates the WebServer CLI’s local proxy mode so --port is truly optional when a locally-running WebServer already has a device connected, aligning local behavior with the existing remote --server-url behavior and the documented CLI contract.
Changes:
- In local mode, probe the local WebServer when
--portis omitted and attach to the proxy if/api/statusreportsconnected=true. - Add regression tests covering the local + no-
--portscenarios (server connected / server no device / server unreachable). - Update CLI documentation to clarify port-less proxy behavior and offline fallback expectations.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| Tools/WebServer/cli/fpb_cli.py | Adds local-mode probing/attach logic when --port is omitted. |
| Tools/WebServer/tests/test_fpb_cli.py | Adds regression tests for local proxy behavior without --port. |
| Docs/CLI.md | Updates documentation for port-less proxy usage and offline behavior. |
W-Mai
force-pushed
the
fix-cli-local-noport-attach
branch
from
ea2ee00 to
f47c027
Compare
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.
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).
W-Mai
force-pushed
the
fix-cli-local-noport-attach
branch
from
f47c027 to
593f648
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The v1.6.6 docs and feat(cli) commit (2c1bf58) state "--port is optional in proxy mode (server owns the serial port)" and "When a server is already running and has a device connected, --port is not needed". This was implemented for remote --server-url, but the local mode init still returned offline immediately when --port was omitted, so commands like 'fpb_cli.py info' or 'file-stat' failed against a locally-running server with a connected device.
Add three regression tests covering local + no --port: