Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 21 additions & 21 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

30 changes: 25 additions & 5 deletions agents/docs/deploy-architecture.md
Original file line number Diff line number Diff line change
Expand Up @@ -92,17 +92,29 @@ The follow-up issues track the full polymorphic version:
`crates/fbuild-deploy/src/rp2040.rs` (`--transport picotool|uf2`,
FastLED/fbuild#1162) selects which stock transport is tried first:

- **`picotool` (default).** After the unchanged pre-touch/1200-bps-touch/
UF2-preparation steps and a best-effort bounded BOOTSEL volume wait,
- **Shared reset ladder.** After the pre-touch scan and 1200-bps CDC touch,
fbuild waits for BOOTSEL. If none appears and the selected runtime endpoint
supplied an exact VID, PID, and non-empty USB serial, it asks the Pico SDK
application reset interface to enter BOOTSEL and waits again. On Windows,
fbuild maps the selected CDC identity to one exact healthy WinUSB reset
interface and sends the Pico class control request directly. Other hosts use
managed `picotool reboot -u --vid ... --pid ... -f`; picotool derives the
runtime serial from the opened application device because that interface
cannot be selected reliably with `--ser`. The fallback is attempted only
after fbuild resolves one exact runtime identity, and picotool refuses a
forced command when the VID/PID is ambiguous. A missing or ambiguous identity
skips this layer, so an unscoped forced command is impossible. Deployment
results name the application reset-interface reboot when it ran successfully.
- **`picotool` (default).** After the shared reset ladder and UF2 preparation,
fbuild derives one exact BOOTSEL VID:PID from the verified FastLED/boards
profile for the selected RP family and binds each picotool operation to
that identity plus the selected runtime USB serial:
`--vid 0x<registry-vid> --pid 0x<registry-pid> --ser <serial>`. It then
runs a Windows-only PICOBOOT driver preflight for that same composite
interface, a bounded `picotool info` probe, and `picotool load <uf2> -x`.
It intentionally does **not** use `-f`: a failed 1200-bps transition must
not let picotool reset some other compatible RP board. A missing runtime
serial or ambiguous/missing registry BOOTSEL identity disables picotool;
The ROM load does not use `-f`; forced application reset is the separately
scoped ladder step above. A missing runtime serial or ambiguous/missing
registry BOOTSEL identity disables picotool;
fbuild uses only an explicitly identified BOOTSEL mass-storage volume. A
Windows driver problem, including Code 43, also skips picotool rather than
spending its timeout. Any remaining picotool failure falls back to the
Expand All @@ -129,6 +141,14 @@ image, a quiet runtime-CDC window no longer fails the deploy — it reports
re-flash a healthy board whose first-plug driver install outlived the
window. A genuine port-enumeration error still fails.

On Windows, the daemon retains the last non-empty `LocationPaths` observed
for a runtime endpoint. An unidentified `VID_0000&PID_0002` Code 43 node is
eligible for the explicit/admin-gated exact-child restart only when one and
only one matching RP history has the same normalized physical USB path. The
elevated helper re-queries that path before acting. A different location or
multiple matches fail closed; fbuild never cycles a hub or changes the
host-wide selective-suspend policy.

## Worked example — "agent ports a new RP2350 board"

The right sequence today:
Expand Down
18 changes: 17 additions & 1 deletion ci/check_usb_vidpid_literals.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@
r"(?i)[\[(]\s*0x([0-9a-f]{4})\s*,\s*0x([0-9a-f]{4})\s*[\])]"
)
NAMED_LITERAL_RE = re.compile(
r"(?i)\b(?:vid|pid|[a-z0-9]+_(?:vid|pid))\b\s*"
r"(?i)\b(?:vid|pid|[a-z0-9_]+_(?:vid|pid))\b\s*"
r"(?::\s*[a-z_][a-z0-9_:<>]*)?\s*(?:==|!=|=|:)\s*"
r"(?:Some\(\s*)?\b(0x[0-9a-f]{4}|[1-9][0-9]{0,4})\b"
)
Expand All @@ -65,6 +65,16 @@
)
CFG_TEST_RE = re.compile(r"#\s*\[\s*cfg\s*\(\s*test\s*\)\s*\]")

# This is a Windows PnP protocol sentinel, not a board identity. Keep the
# exception exact in path, spelling, type, and value so it cannot become a
# second production VID/PID catalogue.
IDENTITY_LITERAL_EXCEPTIONS = {
(
"crates/fbuild-core/src/usb/recovery.rs",
"pub const WINDOWS_DESCRIPTOR_FAILURE_PID: u16 = 2;",
)
}


@dataclass(frozen=True)
class Finding:
Expand Down Expand Up @@ -210,6 +220,12 @@ def scan_text(path: str, source: str) -> list[Finding]:
for pattern, reason in checks:
if not pattern.search(line):
continue
if (
reason == "named VID/PID literal"
and (path.replace("\\", "/"), stripped)
in IDENTITY_LITERAL_EXCEPTIONS
):
continue
key = (line_number, reason)
if key not in seen:
findings.append(Finding(path, line_number, reason, stripped[:160]))
Expand Down
17 changes: 17 additions & 0 deletions ci/test_check_usb_vidpid_literals.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,23 @@ def test_named_decimal_literal_is_rejected(self):
reasons = self.reasons("crates/demo/src/device.rs", "const DEVICE_VID: u16 = 4660;")
self.assertIn("named VID/PID literal", reasons)

def test_exact_windows_descriptor_failure_sentinel_is_allowed_only_in_core_contract(self):
declaration = "pub const WINDOWS_DESCRIPTOR_FAILURE_PID: u16 = 2;"
self.assertEqual(
self.reasons("crates/fbuild-core/src/usb/recovery.rs", declaration), []
)
self.assertIn(
"named VID/PID literal",
self.reasons("crates/demo/src/device.rs", declaration),
)
self.assertIn(
"named VID/PID literal",
self.reasons(
"crates/fbuild-core/src/usb/recovery.rs",
"pub const WINDOWS_DESCRIPTOR_FAILURE_PID: u16 = 3;",
),
)

def test_board_json_separate_fields_are_rejected(self):
findings = guard.scan_text(
"crates/fbuild-config/assets/boards/json/demo.json",
Expand Down
1 change: 1 addition & 0 deletions crates/fbuild-cli/src/cli/deploy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -732,6 +732,7 @@ mod tests {
instance_id: None,
parent_instance_id: None,
ancestor_instance_ids: Vec::new(),
location_paths: Vec::new(),
}
}

Expand Down
2 changes: 2 additions & 0 deletions crates/fbuild-cli/src/cli/port_doctor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -665,6 +665,7 @@ mod tests {
location: Some("Port_#0014.Hub_#0001".into()),
behind_external_hub: Some(false),
device_class: None,
location_paths: Vec::new(),
parent_instance_id: None,
}];
let out = render_report(&[diag(Some(false), None)], &problems);
Expand Down Expand Up @@ -735,6 +736,7 @@ Power Scheme GUID: 381b4222-f694-41f0-9685-ff5bb260df2e (Balanced)
location: Some("Port_#0014.Hub_#0001".into()),
behind_external_hub: Some(false),
device_class: None,
location_paths: Vec::new(),
parent_instance_id: None,
}];
let report = build_json_report(&[diag(Some(false), None)], &problems, Some(true));
Expand Down
2 changes: 2 additions & 0 deletions crates/fbuild-cli/src/cli/port_scan.rs
Original file line number Diff line number Diff line change
Expand Up @@ -833,6 +833,7 @@ mod tests {
behind_external_hub: Some(true),
parent_instance_id: None,
device_class: None,
location_paths: Vec::new(),
}];
let warning = format_usb_problem_warning(&devices);
assert!(warning.contains("problem code 43"));
Expand All @@ -852,6 +853,7 @@ mod tests {
behind_external_hub: Some(false),
parent_instance_id: None,
device_class: None,
location_paths: Vec::new(),
}];
let warning = format_usb_problem_warning(&devices);
assert!(warning.contains("Unknown USB device"));
Expand Down
2 changes: 2 additions & 0 deletions crates/fbuild-cli/src/cli/usb_recovery.rs
Original file line number Diff line number Diff line change
Expand Up @@ -473,6 +473,8 @@ mod tests {
expected_vid: 0x2e8a,
expected_pid: 0x000a,
expected_serial: Some("serial".to_string()),
descriptor_failure_at_location: false,
expected_location_path: None,
problem_code: Some(43),
flash_completed: true,
},
Expand Down
3 changes: 2 additions & 1 deletion crates/fbuild-core/src/usb/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@ pub use data::{
pub use embedded::vendor_name as embedded_vendor_name;
pub use recovery::{
UNCLASSED_DEVICE_CLASS, UsbRecoveryHealth, UsbRecoveryOperation, UsbRecoveryPolicy,
UsbRecoveryRequest, UsbRecoveryResult,
UsbRecoveryRequest, UsbRecoveryResult, is_windows_descriptor_failure_identity,
normalize_physical_location,
};
#[cfg(test)]
pub use resolver::resolve_bundled;
Expand Down
Loading
Loading