Skip to content

fix(serial): preserve Windows port health through probing#1150

Merged
zackees merged 1 commit into
mainfrom
fix/1146-port-health
Jul 22, 2026
Merged

fix(serial): preserve Windows port health through probing#1150
zackees merged 1 commit into
mainfrom
fix/1146-port-health

Conversation

@zackees

@zackees zackees commented Jul 22, 2026

Copy link
Copy Markdown
Member

Closes #1146

What changed

  • Replaced the blessed serial enumeration result with DetectedPort, preserving Windows PnP health, canonical instance ID, and parent identity.
  • Kept phantom/problem records in diagnostics while marking them selectable=no in fbuild port scan.
  • Propagated health through CLI probing/monitor selection, daemon inventory, bootloader watching, and RP2040/Teensy catalogues.
  • Rejected known-unhealthy endpoints from automatic or explicit monitor selection; diagnostic-only helpers still retain them.

Caller inventory

Migrated all user-device selection paths. Remaining raw serialport::available_ports() callers are tool-specific probes in probe_rs, LPC, and ESP32-native code; they do not use the blessed CDC selection path and remain outside this serial health model.

Validation

  • soldr cargo test -p fbuild-serial --lib (153 passed, 1 documented ignore)
  • soldr cargo test -p fbuild-serial health_tests (2 passed)
  • soldr cargo test -p fbuild-deploy rp2040 (80 passed)
  • soldr cargo test -p fbuild-deploy teensy (34 passed)
  • soldr cargo test -p fbuild-cli port_scan (19 passed)
  • soldr cargo test -p fbuild-cli monitor_selection_never_chooses_a_known_unhealthy_endpoint (1 passed)
  • soldr cargo test -p fbuild-daemon device_manager (23 passed)
  • soldr cargo clippy -p fbuild-serial -p fbuild-deploy -p fbuild-cli -p fbuild-daemon --all-targets -- -D warnings

soldr cargo test -p fbuild-serial additionally runs three existing boards.rs doctests against the non-test online USB-profile registry. They fail in this local worktree because no profile artifact is installed; the unchanged doctest bodies and all 153 library tests demonstrate this is unrelated to this change.

Summary by CodeRabbit

  • New Features

    • Serial-port detection now reports endpoint health, problem codes, and device identity details.
    • Port listings include health annotations and clearly identify unavailable or non-selectable endpoints.
    • Deployment diagnostics provide more detailed information when compatible devices cannot be found.
  • Bug Fixes

    • Automatic and explicit deployment selection no longer uses known-unhealthy ports.
    • Device discovery better preserves device identity when ports move or reconnect.
    • Bootloader and post-flash detection now avoids phantom or unhealthy endpoints and prefers valid runtime devices.

@coderabbitai

coderabbitai Bot commented Jul 22, 2026

Copy link
Copy Markdown

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 531eed72-cbd1-4cf9-bf8c-19cdb001bd8c

📥 Commits

Reviewing files that changed from the base of the PR and between 96a128e and b9e946c.

📒 Files selected for processing (13)
  • crates/fbuild-cli/src/cli/deploy.rs
  • crates/fbuild-cli/src/cli/port_scan.rs
  • crates/fbuild-cli/src/cli/serial_probe.rs
  • crates/fbuild-daemon/src/device_manager.rs
  • crates/fbuild-daemon/src/device_manager/tests.rs
  • crates/fbuild-daemon/src/handlers/operations/deploy_port.rs
  • crates/fbuild-deploy/src/rp2040.rs
  • crates/fbuild-deploy/src/rp2040_target.rs
  • crates/fbuild-deploy/src/teensy/halfkay_probe.rs
  • crates/fbuild-deploy/src/teensy/port_discovery.rs
  • crates/fbuild-serial/src/boards.rs
  • crates/fbuild-serial/src/bootloader_watcher.rs
  • crates/fbuild-serial/src/ports.rs

📝 Walkthrough

Walkthrough

Serial enumeration now returns DetectedPort records containing health and host identity metadata. CLI, daemon, RP2040, and Teensy paths consume these records for diagnostics, caching, selection, and deployment port reacquisition.

Changes

Serial port health model

Layer / File(s) Summary
Detected-port health model
crates/fbuild-serial/src/ports.rs
Adds PortHealth and DetectedPort, classifies Windows PnP observations, preserves instance identities, and returns enriched records from available_ports().
CLI and daemon propagation
crates/fbuild-cli/src/cli/*, crates/fbuild-daemon/src/*
CLI scan and probe output health metadata and avoids known-unhealthy matches; monitor selection rejects unhealthy endpoints; daemon state preserves health and host identities.
Deployment discovery and diagnostics
crates/fbuild-deploy/src/*, crates/fbuild-serial/src/boards.rs, crates/fbuild-serial/src/bootloader_watcher.rs
Deployment candidate records carry health and identity metadata, while Teensy, board, and bootloader discovery exclude known-unhealthy ports where applicable.

Estimated code review effort: 4 (Complex) | ~60 minutes

Sequence Diagram(s)

sequenceDiagram
  participant WindowsPnP
  participant available_ports
  participant CLI
  participant DeviceManager
  participant DeploymentDiscovery
  WindowsPnP->>available_ports: provide port observations and identities
  available_ports->>CLI: return DetectedPort records
  available_ports->>DeviceManager: return DetectedPort records
  available_ports->>DeploymentDiscovery: return DetectedPort records
  CLI->>CLI: render health or reject unhealthy selection
  DeviceManager->>DeviceManager: cache health and identities
  DeploymentDiscovery->>DeploymentDiscovery: filter unhealthy candidates
Loading

Possibly related issues

Possibly related PRs

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/1146-port-health

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@zackees
zackees merged commit 66f73ac into main Jul 22, 2026
7 of 93 checks passed
@zackees
zackees deleted the fix/1146-port-health branch July 22, 2026 23:34
@fastled-project-sync fastled-project-sync Bot moved this to Triage in FastLED Tracker Jul 23, 2026
zackees added a commit that referenced this pull request Jul 23, 2026
… gate

The Formatting and Check workflows have been red on main since #1145:
- four files (two from this branch, two pre-existing: board/loaders.rs,
  ch32v_core.rs) carry diffs under the pinned 1.94.1 formatter. Local
  cargo-fmt --all silently skips the daemon handler files on this
  checkout, so they were formatted with direct invocations.
- the #1150 health-classification helpers in fbuild-serial have no
  non-Windows, non-test caller, which RUSTFLAGS=-D warnings promotes to
  a hard error on the ubuntu/macos Check jobs. Annotate them
  cfg_attr(not(windows), allow(dead_code)) — they are fed by the
  Windows PnP enumeration and unit-tested cross-platform.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
zackees added a commit that referenced this pull request Jul 23, 2026
…quisition (#1153)

* fix(rp2040): consume Windows port health in deploy selection and reacquisition

A retained CM_PROB_PHANTOM devnode whose serial still matches the board
could be selected pre-flash, touched at 1200 baud, and returned as the
post-flash deploy port. Health from #1146/#1150 now gates every step:

- rp2040_target: select_cdc_candidate returns a health-eligible
  PicoCdcPort (phantom/present-problem records are never selected);
  resolve_requested_runtime_target fails an explicit selector that
  matches a known-unhealthy record, with health/problem/instance detail
- rp2040: the post-flash wait adds a bounded openability probe seam;
  timeout diagnostics carry the last open error and phantom-aware
  BOOT/RESET recovery guidance; Deployer::owns_post_flash_port_discovery
  marks the RP2040 port as authoritative
- daemon: recovery/monitor port resolution no longer substitutes the
  requested pre-flash name when the deployer owns port discovery
  (deploy_port_str.or(result.port) inverted the trust order); flash
  success with an unrecovered CDC now surfaces the deployer's recovery
  diagnostic instead of discarding it; choose_deploy_port partitions
  known-unhealthy RP2040 records out of auto-selection with a
  diagnostic warning

Closes #1147

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(ci): satisfy pinned formatter tree-wide and non-Windows dead-code gate

The Formatting and Check workflows have been red on main since #1145:
- four files (two from this branch, two pre-existing: board/loaders.rs,
  ch32v_core.rs) carry diffs under the pinned 1.94.1 formatter. Local
  cargo-fmt --all silently skips the daemon handler files on this
  checkout, so they were formatted with direct invocations.
- the #1150 health-classification helpers in fbuild-serial have no
  non-Windows, non-test caller, which RUSTFLAGS=-D warnings promotes to
  a hard error on the ubuntu/macos Check jobs. Annotate them
  cfg_attr(not(windows), allow(dead_code)) — they are fed by the
  Windows PnP enumeration and unit-tested cross-platform.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(ci): mark registry-backed boards doctests no_run

board_hint, vcom_for_env, and family_for_vid_pid resolve through the
FastLED/boards registry at runtime; on doctest hosts (local and CI)
that cache is absent, so the three examples panicked on every Check
job since the vendored-table migration. no_run keeps them
compile-checked without requiring a populated registry.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(ci): annotate riscv multilib GCC probe for the direct-spawn lint

ci/find_direct_subprocess.py --fail has flagged the ch32v multilib
probe since it landed, keeping the Check jobs red. The spawn is a
short synchronous -print-multi-directory capability probe; mark it
allow-direct-spawn rather than routing a one-flag probe through the
subprocess facade.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(ci): drop embedded WCH ISP VID/PID from the wchisp failure message

The USB VID/PID catalogue-literal guard has flagged this user-facing
string since the wchisp deployer landed, keeping Check (ubuntu) red.
Identity data belongs to the FastLED/boards registry; point the user
at 'fbuild port scan' instead of embedding 4348/1a86 literals.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(ci): apply nightly import ordering in ban_raw_subprocess dylint

The Dylint crate-formatting gate (nightly style edition) has wanted
these two use-statement reorders since the lint landed; apply them so
the gate goes green.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(ci): apply nightly import ordering in ban_raw_path_prefix_compare dylint

Same nightly-2026-03-26 style-edition reorders as ban_raw_subprocess;
verified with the crate-pinned formatter that both format-checked
dylint crates are now clean.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(ci): allowlist riscv toolchain GCC probe in ban_raw_subprocess dylint

Companion to the Python-lint marker: the workspace dylint pass wants
its own allowlist.txt entry for the same short synchronous
-print-multi-directory capability probe. ci/check_dylint_allowlists.py
passes.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(ci): allowlist esp32 framework-library cache in ban_raw_path_prefix_compare

WalkDir-derived paths are stripped against the exact root spelling they
were enumerated from (same-normalized by construction) and feed a
content hash, not a path-keyed cache lookup — the lint's own stated
exemption. Pre-existing debt unmasked once the earlier Dylint gate
failures were fixed.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(ci): force rebuild of ban_raw_path_prefix_compare after allowlist edit

The previous round still flagged the freshly-allowlisted esp32
framework-library cache: the workspace dylint step served a compiled
lint from the restored build cache, ignoring the allowlist-only edit.
Touch the lint source (documenting the trap) so the fingerprint moves.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(ci): refresh stale dylint @toolchain aliases after rebuilds

cargo-dylint loads the <name>@<toolchain> library copy, but the alias
loop only created it when absent. With target caching, a restored stale
alias therefore pinned every lint to its first-ever compiled allowlist:
rebuilt bare libraries (allowlist/source edits) were never loaded, which
is why ban_raw_path_prefix_compare kept flagging the freshly-allowlisted
esp32 framework-library cache. Overwrite the alias when the bare library
is newer.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Triage

Development

Successfully merging this pull request may close these issues.

fix(serial): preserve Windows PnP health through port probing

1 participant