diff --git a/crates/fbuild-cli/src/cli/deploy.rs b/crates/fbuild-cli/src/cli/deploy.rs index d3f7507b..c9320a4a 100644 --- a/crates/fbuild-cli/src/cli/deploy.rs +++ b/crates/fbuild-cli/src/cli/deploy.rs @@ -337,6 +337,59 @@ fn operation_streams_include_message(resp: &OperationResponse) -> bool { .any(|stream| stream.trim() == message) } +fn select_monitor_port( + requested: Option, + ports: &[fbuild_serial::ports::DetectedPort], +) -> fbuild_core::Result { + match requested { + Some(port) => { + if let Some(detected) = ports + .iter() + .find(|detected| detected.info.port_name == port) + .filter(|detected| detected.health.is_known_unhealthy()) + { + return Err(fbuild_core::FbuildError::SerialError(format!( + "refusing to monitor {port}: endpoint health is {}; run `fbuild port scan` for recovery details", + detected.health.label() + ))); + } + Ok(port) + } + None => { + let selectable = ports + .iter() + .filter(|port| !port.health.is_known_unhealthy()) + .collect::>(); + match selectable.as_slice() { + [port] => Ok(port.info.port_name.clone()), + [] if ports.is_empty() => Err(fbuild_core::FbuildError::SerialError( + "no serial ports detected; specify one with --port".to_string(), + )), + [] => { + let unhealthy = ports + .iter() + .map(|port| format!("{} ({})", port.info.port_name, port.health.label())) + .collect::>() + .join(", "); + Err(fbuild_core::FbuildError::SerialError(format!( + "no selectable serial ports detected ({unhealthy}); run `fbuild port scan` for recovery details" + ))) + } + selectable => { + let names = selectable + .iter() + .map(|port| port.info.port_name.as_str()) + .collect::>() + .join(", "); + Err(fbuild_core::FbuildError::SerialError(format!( + "multiple serial ports detected ({names}); specify one with --port" + ))) + } + } + } + } +} + #[allow(clippy::too_many_arguments)] pub async fn run_monitor( project_dir: String, @@ -354,34 +407,10 @@ pub async fn run_monitor( daemon_client::warn_if_daemon_identity_mismatch(&client, &project_dir).await; let _ = (project_dir, environment); - let port = match port { - Some(port) => port, - None => { - let ports = fbuild_serial::ports::available_ports().map_err(|e| { - fbuild_core::FbuildError::SerialError(format!( - "failed to enumerate serial ports: {e}" - )) - })?; - match ports.as_slice() { - [port] => port.port_name.clone(), - [] => { - return Err(fbuild_core::FbuildError::SerialError( - "no serial ports detected; specify one with --port".to_string(), - )); - } - ports => { - let names = ports - .iter() - .map(|p| p.port_name.as_str()) - .collect::>() - .join(", "); - return Err(fbuild_core::FbuildError::SerialError(format!( - "multiple serial ports detected ({names}); specify one with --port" - ))); - } - } - } - }; + let ports = fbuild_serial::ports::available_ports().map_err(|e| { + fbuild_core::FbuildError::SerialError(format!("failed to enumerate serial ports: {e}")) + })?; + let port = select_monitor_port(port, &ports)?; let baud_rate = baud_rate.unwrap_or(115200); let client_id = format!("fbuild-monitor-{}", std::process::id()); let (mut socket, _) = connect_async(client.websocket_url("/ws/serial-monitor")) @@ -493,6 +522,21 @@ pub async fn run_monitor( mod tests { use super::*; + fn detected_port( + name: &str, + health: fbuild_serial::ports::PortHealth, + ) -> fbuild_serial::ports::DetectedPort { + fbuild_serial::ports::DetectedPort { + info: serialport::SerialPortInfo { + port_name: name.to_string(), + port_type: serialport::SerialPortType::Unknown, + }, + health, + instance_id: None, + parent_instance_id: None, + } + } + fn response(message: &str, stdout: Option<&str>, stderr: Option<&str>) -> OperationResponse { OperationResponse { success: false, @@ -522,4 +566,22 @@ mod tests { let resp = response("deploy failed", None, Some("transport detail")); assert!(!operation_streams_include_message(&resp)); } + + #[test] + fn monitor_selection_never_chooses_a_known_unhealthy_endpoint() { + let ports = vec![detected_port( + "COM12", + fbuild_serial::ports::PortHealth::Phantom { + problem_code: Some(45), + status: Some(0), + }, + )]; + + let auto_error = select_monitor_port(None, &ports).unwrap_err().to_string(); + assert!(auto_error.contains("no selectable serial ports")); + let explicit_error = select_monitor_port(Some("COM12".to_string()), &ports) + .unwrap_err() + .to_string(); + assert!(explicit_error.contains("endpoint health is phantom")); + } } diff --git a/crates/fbuild-cli/src/cli/port_scan.rs b/crates/fbuild-cli/src/cli/port_scan.rs index 41527d3a..2b7e4ed8 100644 --- a/crates/fbuild-cli/src/cli/port_scan.rs +++ b/crates/fbuild-cli/src/cli/port_scan.rs @@ -159,7 +159,7 @@ fn overlay_json_cache_path_in(root: &std::path::Path) -> Option String { +pub fn render_scan(ports: &[fbuild_serial::ports::DetectedPort]) -> String { if ports.is_empty() { return "no serial ports visible\n".to_string(); } @@ -168,7 +168,8 @@ pub fn render_scan(ports: &[serialport::SerialPortInfo]) -> String { let mut usb_count = 0usize; let mut other_count = 0usize; - for port in ports { + for detected in ports { + let port = &detected.info; match &port.port_type { serialport::SerialPortType::UsbPort(info) => { usb_count += 1; @@ -195,6 +196,7 @@ pub fn render_scan(ports: &[serialport::SerialPortInfo]) -> String { render_non_usb(&mut out, &port.port_name, "Unknown"); } } + append_health_annotation(&mut out, detected); out.push('\n'); } @@ -209,6 +211,22 @@ pub fn render_scan(ports: &[serialport::SerialPortInfo]) -> String { out } +fn append_health_annotation(out: &mut String, port: &fbuild_serial::ports::DetectedPort) { + use std::fmt::Write as _; + let _ = out.pop(); + let _ = write!(out, " health={}", port.health.label()); + if port.health.is_known_unhealthy() { + out.push_str(" selectable=no"); + } + if let Some(problem_code) = port.health.problem_code() { + let _ = write!(out, " problem={problem_code}"); + } + if let Some(instance_id) = port.instance_id.as_deref() { + let _ = write!(out, " instance={instance_id}"); + } + out.push('\n'); +} + fn render_usb_port( out: &mut String, name: &str, @@ -362,8 +380,8 @@ mod tests { pid: u16, product: Option<&str>, serial: Option<&str>, - ) -> SerialPortInfo { - SerialPortInfo { + ) -> fbuild_serial::ports::DetectedPort { + fbuild_serial::ports::DetectedPort::unknown(SerialPortInfo { port_name: name.to_string(), port_type: SerialPortType::UsbPort(UsbPortInfo { vid, @@ -373,7 +391,14 @@ mod tests { product: product.map(String::from), interface: None, }), - } + }) + } + + fn non_usb_port(name: &str, port_type: SerialPortType) -> fbuild_serial::ports::DetectedPort { + fbuild_serial::ports::DetectedPort::unknown(SerialPortInfo { + port_name: name.to_string(), + port_type, + }) } #[test] @@ -543,18 +568,9 @@ mod tests { #[test] fn non_usb_ports_get_kind_label_and_uniform_two_rows() { let ports = vec![ - SerialPortInfo { - port_name: "BT0".to_string(), - port_type: SerialPortType::BluetoothPort, - }, - SerialPortInfo { - port_name: "PCI3".to_string(), - port_type: SerialPortType::PciPort, - }, - SerialPortInfo { - port_name: "X1".to_string(), - port_type: SerialPortType::Unknown, - }, + non_usb_port("BT0", SerialPortType::BluetoothPort), + non_usb_port("PCI3", SerialPortType::PciPort), + non_usb_port("X1", SerialPortType::Unknown), ]; let out = render_scan(&ports); assert!(out.contains("[Bluetooth]")); @@ -620,6 +636,22 @@ mod tests { assert!(unknown.contains("cdc=unknown"), "got: {unknown}"); } + #[test] + fn scan_marks_phantom_ports_as_not_selectable_diagnostics() { + let mut port = usb_port("COM12", 0x2E8A, 0x000A, None, Some("PICO-1")); + port.health = fbuild_serial::ports::PortHealth::Phantom { + problem_code: Some(45), + status: Some(0), + }; + port.instance_id = Some(r"USB\VID_2E8A&PID_000A\PICO-1".to_string()); + + let out = render_scan(&[port]); + assert!(out.contains("health=phantom"), "got: {out}"); + assert!(out.contains("selectable=no"), "got: {out}"); + assert!(out.contains("problem=45"), "got: {out}"); + assert!(out.contains("instance=USB\\VID_2E8A&PID_000A\\PICO-1")); + } + #[test] fn usb_problem_warning_recommends_root_port_for_hub_node() { let devices = vec![fbuild_serial::ports::UsbProblemDevice { @@ -765,10 +797,7 @@ mod tests { fn mixed_port_list_summary_counts_correctly() { let ports = vec![ usb_port("COM1", 0x303A, 0x1001, None, None), - SerialPortInfo { - port_name: "BT0".to_string(), - port_type: SerialPortType::BluetoothPort, - }, + non_usb_port("BT0", SerialPortType::BluetoothPort), usb_port("COM2", 0x16C0, 0x0483, None, None), ]; let out = render_scan(&ports); diff --git a/crates/fbuild-cli/src/cli/serial_probe.rs b/crates/fbuild-cli/src/cli/serial_probe.rs index 847e6803..a107050a 100644 --- a/crates/fbuild-cli/src/cli/serial_probe.rs +++ b/crates/fbuild-cli/src/cli/serial_probe.rs @@ -112,7 +112,7 @@ fn run_probe(action: ProbeAction) -> Result<()> { /// `fbuild serial probe list` — enumerate every port with annotation. fn list_ports() -> Result<()> { - let ports = serialport::available_ports() + let ports = fbuild_serial::ports::available_ports() .map_err(|e| FbuildError::SerialError(format!("serial port enumeration failed: {e}")))?; if ports.is_empty() { @@ -126,9 +126,10 @@ fn list_ports() -> Result<()> { Ok(()) } -fn print_port_summary(port: &serialport::SerialPortInfo) { - let name = &port.port_name; - match &port.port_type { +fn print_port_summary(port: &fbuild_serial::ports::DetectedPort) { + let name = &port.info.port_name; + let health = port.health.label(); + match &port.info.port_type { serialport::SerialPortType::UsbPort(info) => { let hint = board_hint(info.vid, info.pid) .map(|h| format!("[{h}]")) @@ -141,16 +142,20 @@ fn print_port_summary(port: &serialport::SerialPortInfo) { format!("ser={serial} ") }; output::result(format!( - "{name:<10} {vid:04X}:{pid:04X} {serial_field}{product} {hint}", + "{name:<10} {vid:04X}:{pid:04X} {serial_field}{product} {hint} health={health}", vid = info.vid, pid = info.pid, )); } - serialport::SerialPortType::PciPort => output::result(format!("{name:<10} [PCI]")), + serialport::SerialPortType::PciPort => { + output::result(format!("{name:<10} [PCI] health={health}")) + } serialport::SerialPortType::BluetoothPort => { - output::result(format!("{name:<10} [Bluetooth]")) + output::result(format!("{name:<10} [Bluetooth] health={health}")) + } + serialport::SerialPortType::Unknown => { + output::result(format!("{name:<10} [Unknown] health={health}")) } - serialport::SerialPortType::Unknown => output::result(format!("{name:<10} [Unknown]")), } } @@ -178,13 +183,13 @@ fn find_port(vid_pid: Option<&str>, env: Option<&str>) -> Result<()> { } }; - let ports = serialport::available_ports() + let ports = fbuild_serial::ports::available_ports() .map_err(|e| FbuildError::SerialError(format!("serial port enumeration failed: {e}")))?; for port in ports { - if let serialport::SerialPortType::UsbPort(info) = &port.port_type { - if (info.vid, info.pid) == target { - output::result(&port.port_name); + if let serialport::SerialPortType::UsbPort(info) = &port.info.port_type { + if (info.vid, info.pid) == target && !port.health.is_known_unhealthy() { + output::result(&port.info.port_name); return Ok(()); } } diff --git a/crates/fbuild-daemon/src/device_manager.rs b/crates/fbuild-daemon/src/device_manager.rs index f4d46ccb..2ce4291f 100644 --- a/crates/fbuild-daemon/src/device_manager.rs +++ b/crates/fbuild-daemon/src/device_manager.rs @@ -113,6 +113,11 @@ pub struct DeviceState { /// non-USB or unknown on this platform. pub is_cdc: Option, pub serial_number: Option, + /// Host health retained from the blessed serial enumeration. + pub port_health: fbuild_serial::ports::PortHealth, + /// Canonical Plug and Play identity when the host exposes one. + pub instance_id: Option, + pub parent_instance_id: Option, pub previous_port: Option, pub exclusive_lease: Option, pub monitor_leases: HashMap, @@ -165,6 +170,9 @@ struct DiscoveredDevice { product_name: Option, is_cdc: Option, serial_number: Option, + port_health: fbuild_serial::ports::PortHealth, + instance_id: Option, + parent_instance_id: Option, } /// Thread-safe device manager. @@ -228,7 +236,7 @@ impl DeviceManager { /// Refresh the device inventory from serial port enumeration. /// Preserves existing leases for devices that are still present. pub fn refresh_devices(&self) { - let ports = match serialport::available_ports() { + let ports = match fbuild_serial::ports::available_ports() { Ok(p) => p, Err(e) => { tracing::warn!("failed to enumerate serial ports: {}", e); @@ -238,7 +246,11 @@ impl DeviceManager { let discovered: Vec = ports .into_iter() - .map(|port_info| { + .map(|detected| { + let port_health = detected.health.clone(); + let instance_id = detected.instance_id.clone(); + let parent_instance_id = detected.parent_instance_id.clone(); + let port_info = detected.info; let (vid, pid, fallback_desc) = match &port_info.port_type { serialport::SerialPortType::UsbPort(usb) => ( Some(usb.vid), @@ -288,6 +300,9 @@ impl DeviceManager { product_name, is_cdc, serial_number, + port_health, + instance_id, + parent_instance_id, } }) .collect(); @@ -341,6 +356,9 @@ impl DeviceManager { state.product_name = device.product_name; state.is_cdc = device.is_cdc; state.serial_number = device.serial_number; + state.port_health = device.port_health; + state.instance_id = device.instance_id; + state.parent_instance_id = device.parent_instance_id; if let Some(previous_port) = state.previous_port.clone() { self.recent_port_moves .lock() @@ -368,6 +386,9 @@ impl DeviceManager { product_name: device.product_name.clone(), is_cdc: device.is_cdc, serial_number: device.serial_number.clone(), + port_health: device.port_health.clone(), + instance_id: device.instance_id.clone(), + parent_instance_id: device.parent_instance_id.clone(), previous_port: None, exclusive_lease: None, monitor_leases: HashMap::new(), @@ -387,6 +408,9 @@ impl DeviceManager { entry.product_name = device.product_name; entry.is_cdc = device.is_cdc; entry.serial_number = device.serial_number; + entry.port_health = device.port_health; + entry.instance_id = device.instance_id; + entry.parent_instance_id = device.parent_instance_id; } // Stamp `last_disconnect_at` for every device that went from @@ -703,6 +727,9 @@ impl DeviceManager { product_name: Some("Test Device".to_string()), is_cdc: None, serial_number: Some("TEST-SERIAL".to_string()), + port_health: fbuild_serial::ports::PortHealth::Unknown, + instance_id: None, + parent_instance_id: None, previous_port: None, exclusive_lease: None, monitor_leases: HashMap::new(), diff --git a/crates/fbuild-daemon/src/device_manager/tests.rs b/crates/fbuild-daemon/src/device_manager/tests.rs index 9f45bfa9..4df29e6e 100644 --- a/crates/fbuild-daemon/src/device_manager/tests.rs +++ b/crates/fbuild-daemon/src/device_manager/tests.rs @@ -190,12 +190,23 @@ fn tracked_serial_lease_moves_to_new_port_on_refresh() { product_name: Some("Test Device".to_string()), is_cdc: Some(true), serial_number: Some("TEST-SERIAL".to_string()), + port_health: fbuild_serial::ports::PortHealth::HealthyPresent, + instance_id: Some(r"USB\VID_1234&PID_5678\TEST-SERIAL".to_string()), + parent_instance_id: Some(r"USB\VID_1234&PID_5678\PARENT".to_string()), }]); assert!(mgr.get_device_status("COM3").is_none()); let moved = mgr.get_device_status("COM4").unwrap(); assert_eq!(moved.previous_port.as_deref(), Some("COM3")); assert_eq!(moved.is_cdc, Some(true)); + assert_eq!( + moved.port_health, + fbuild_serial::ports::PortHealth::HealthyPresent + ); + assert_eq!( + moved.instance_id.as_deref(), + Some(r"USB\VID_1234&PID_5678\TEST-SERIAL") + ); assert_eq!( moved.exclusive_lease.as_ref().map(|l| l.client_id.as_str()), Some("c1") @@ -234,6 +245,12 @@ fn untracked_serial_lease_stays_on_old_disconnected_port() { product_name: Some("Test Device".to_string()), is_cdc: Some(false), serial_number: Some("TEST-SERIAL".to_string()), + port_health: fbuild_serial::ports::PortHealth::PresentProblem { + problem_code: 31, + status: Some(0), + }, + instance_id: Some(r"USB\VID_1234&PID_5678\TEST-SERIAL".to_string()), + parent_instance_id: Some(r"USB\VID_1234&PID_5678\PARENT".to_string()), }]); let old = mgr.get_device_status("COM3").unwrap(); diff --git a/crates/fbuild-daemon/src/handlers/operations/deploy_port.rs b/crates/fbuild-daemon/src/handlers/operations/deploy_port.rs index 73495ba1..210d6649 100644 --- a/crates/fbuild-daemon/src/handlers/operations/deploy_port.rs +++ b/crates/fbuild-daemon/src/handlers/operations/deploy_port.rs @@ -309,6 +309,9 @@ mod tests { product_name: None, is_cdc: None, serial_number: None, + port_health: fbuild_serial::ports::PortHealth::Unknown, + instance_id: None, + parent_instance_id: None, previous_port: None, exclusive_lease: None, monitor_leases: HashMap::new(), diff --git a/crates/fbuild-deploy/src/rp2040.rs b/crates/fbuild-deploy/src/rp2040.rs index 8b107f8f..bd2d5e8d 100644 --- a/crates/fbuild-deploy/src/rp2040.rs +++ b/crates/fbuild-deploy/src/rp2040.rs @@ -981,6 +981,9 @@ impl Rp2040Deployer { struct PicoCdcPort { name: String, serial_number: Option, + health: fbuild_serial::ports::PortHealth, + instance_id: Option, + parent_instance_id: Option, } fn catalogue_pico_cdc_ports(expected_family: u32) -> Result> { @@ -992,15 +995,18 @@ fn catalogue_pico_cdc_ports(expected_family: u32) -> Result> { let mut matches: Vec = ports .into_iter() .filter_map(|port| { - let serialport::SerialPortType::UsbPort(usb) = &port.port_type else { + let serialport::SerialPortType::UsbPort(usb) = &port.info.port_type else { return None; }; let matches_family = fbuild_core::usb::profiles::profiles_for(usb.vid, usb.pid) .iter() .any(|profile| profile_matches_family(profile, expected_family)); matches_family.then(|| PicoCdcPort { - name: port.port_name, + name: port.info.port_name, serial_number: usb.serial_number.clone(), + health: port.health, + instance_id: port.instance_id, + parent_instance_id: port.parent_instance_id, }) }) .collect(); @@ -1034,9 +1040,27 @@ impl CdcWaitTimeout { } else { self.candidates .iter() - .map(|candidate| match candidate.serial_number.as_deref() { - Some(serial) => format!("{} (serial {serial})", candidate.name), - None => format!("{} (serial unavailable)", candidate.name), + .map(|candidate| { + let serial = candidate + .serial_number + .as_deref() + .map(|value| format!("serial {value}")) + .unwrap_or_else(|| "serial unavailable".to_string()); + let instance = candidate + .instance_id + .as_deref() + .map(|value| format!("; instance {value}")) + .unwrap_or_default(); + let parent = candidate + .parent_instance_id + .as_deref() + .map(|value| format!("; parent {value}")) + .unwrap_or_default(); + format!( + "{} ({serial}; health {}{instance}{parent})", + candidate.name, + candidate.health.label() + ) }) .collect::>() .join(", ") @@ -2097,6 +2121,9 @@ mod tests { candidates: vec![PicoCdcPort { name: "COM27".to_string(), serial_number: Some("5303284720C4641C".to_string()), + health: fbuild_serial::ports::PortHealth::Unknown, + instance_id: None, + parent_instance_id: None, }], } .diagnostics(); @@ -2104,7 +2131,7 @@ mod tests { assert!(diagnostics.contains("elapsed 30000ms")); assert!(diagnostics.contains("prior port COM12")); assert!(diagnostics.contains("requested serial 5303284720C4641C")); - assert!(diagnostics.contains("COM27 (serial 5303284720C4641C)")); + assert!(diagnostics.contains("COM27 (serial 5303284720C4641C; health unknown)")); } #[test] @@ -2152,6 +2179,9 @@ mod tests { vec![PicoCdcPort { name: "COM27".to_string(), serial_number: Some("5303284720C4641C".to_string()), + health: fbuild_serial::ports::PortHealth::Unknown, + instance_id: None, + parent_instance_id: None, }] }) }, @@ -2178,6 +2208,9 @@ mod tests { Ok(vec![PicoCdcPort { name: "COM27".to_string(), serial_number: Some("5303284720C4641C".to_string()), + health: fbuild_serial::ports::PortHealth::Unknown, + instance_id: None, + parent_instance_id: None, }]) }, || elapsed.next().unwrap_or(Duration::from_secs(30)), @@ -2206,10 +2239,16 @@ mod tests { PicoCdcPort { name: "COM12".to_string(), serial_number: None, + health: fbuild_serial::ports::PortHealth::Unknown, + instance_id: None, + parent_instance_id: None, }, PicoCdcPort { name: "COM13".to_string(), serial_number: None, + health: fbuild_serial::ports::PortHealth::Unknown, + instance_id: None, + parent_instance_id: None, }, ]) }, diff --git a/crates/fbuild-deploy/src/rp2040_target.rs b/crates/fbuild-deploy/src/rp2040_target.rs index f5e59317..1ca076e8 100644 --- a/crates/fbuild-deploy/src/rp2040_target.rs +++ b/crates/fbuild-deploy/src/rp2040_target.rs @@ -104,6 +104,9 @@ mod tests { PicoCdcPort { name: name.to_string(), serial_number: serial_number.map(str::to_string), + health: fbuild_serial::ports::PortHealth::Unknown, + instance_id: None, + parent_instance_id: None, } } diff --git a/crates/fbuild-deploy/src/teensy/halfkay_probe.rs b/crates/fbuild-deploy/src/teensy/halfkay_probe.rs index 14dbd246..8dc50af3 100644 --- a/crates/fbuild-deploy/src/teensy/halfkay_probe.rs +++ b/crates/fbuild-deploy/src/teensy/halfkay_probe.rs @@ -43,7 +43,9 @@ pub fn wait_for_disappearance(port: &str, timeout: Duration) -> DisappearOutcome let deadline = Instant::now() + timeout; let poll = Duration::from_millis(75); loop { - let present = list_ports().iter().any(|info| info.port_name == port); + let present = list_ports().iter().any(|detected| { + detected.info.port_name == port && !detected.health.is_known_unhealthy() + }); if !present { return DisappearOutcome::Gone; } diff --git a/crates/fbuild-deploy/src/teensy/port_discovery.rs b/crates/fbuild-deploy/src/teensy/port_discovery.rs index caa843a9..a8d23d34 100644 --- a/crates/fbuild-deploy/src/teensy/port_discovery.rs +++ b/crates/fbuild-deploy/src/teensy/port_discovery.rs @@ -15,7 +15,8 @@ use std::collections::HashSet; use std::time::{Duration, Instant}; -use serialport::{SerialPortInfo, SerialPortType}; +use fbuild_serial::ports::DetectedPort; +use serialport::SerialPortType; /// Test-only identity fixture. Production Teensy classification is supplied by /// the verified FastLED/boards USB transport profiles. @@ -55,7 +56,7 @@ fn is_teensy_runtime_identity(vid: u16, pid: u16) -> bool { /// `serialport::available_ports()`) lists Windows ports whose PnP devnode /// reports a non-OK status — every Teensy composite serial port. Without this /// the pre/post-flash port diff never sees the Teensy. FastLED/fbuild#962. -pub fn list_ports() -> Vec { +pub fn list_ports() -> Vec { match fbuild_serial::ports::available_ports() { Ok(ports) => ports, Err(e) => { @@ -71,7 +72,7 @@ pub fn list_ports() -> Vec { /// `wait_for_new_cdc_port` needs to compute the diff, and serialising /// `SerialPortInfo` across thread boundaries can be awkward. pub fn snapshot_port_names() -> HashSet { - list_ports().into_iter().map(|p| p.port_name).collect() + list_ports().into_iter().map(|p| p.info.port_name).collect() } /// Return true if `port` is reported by `available_ports()` as a PJRC USB @@ -79,8 +80,8 @@ pub fn snapshot_port_names() -> HashSet { /// device is already HalfKay" from "the user gave us the wrong port entirely". pub fn is_pjrc_cdc(port: &str) -> bool { for info in list_ports() { - if info.port_name == port { - if let SerialPortType::UsbPort(usb) = &info.port_type { + if info.info.port_name == port { + if let SerialPortType::UsbPort(usb) = &info.info.port_type { return is_teensy_runtime_identity(usb.vid, usb.pid); } } @@ -94,9 +95,12 @@ pub fn is_pjrc_cdc(port: &str) -> bool { /// the baud-134 trigger. pub fn first_pjrc_cdc_port() -> Option { for info in list_ports() { - if let SerialPortType::UsbPort(usb) = &info.port_type { + if info.health.is_known_unhealthy() { + continue; + } + if let SerialPortType::UsbPort(usb) = &info.info.port_type { if is_teensy_runtime_identity(usb.vid, usb.pid) { - return Some(info.port_name); + return Some(info.info.port_name); } } } @@ -129,12 +133,12 @@ pub fn wait_for_new_cdc_port(pre_snapshot: &HashSet, timeout: Duration) while Instant::now() < deadline { let current = list_ports(); - let mut pjrc_new: Vec<&SerialPortInfo> = Vec::new(); - let mut any_new: Vec<&SerialPortInfo> = Vec::new(); + let mut pjrc_new: Vec<&DetectedPort> = Vec::new(); + let mut any_new: Vec<&DetectedPort> = Vec::new(); for info in ¤t { - if !pre_snapshot.contains(&info.port_name) { + if !pre_snapshot.contains(&info.info.port_name) && !info.health.is_known_unhealthy() { any_new.push(info); - if let SerialPortType::UsbPort(usb) = &info.port_type { + if let SerialPortType::UsbPort(usb) = &info.info.port_type { if is_teensy_runtime_identity(usb.vid, usb.pid) { pjrc_new.push(info); } @@ -142,10 +146,10 @@ pub fn wait_for_new_cdc_port(pre_snapshot: &HashSet, timeout: Duration) } } if let Some(info) = pjrc_new.first() { - return NewPortOutcome::Found(info.port_name.clone()); + return NewPortOutcome::Found(info.info.port_name.clone()); } if let Some(info) = any_new.first() { - return NewPortOutcome::Found(info.port_name.clone()); + return NewPortOutcome::Found(info.info.port_name.clone()); } std::thread::sleep(poll); } diff --git a/crates/fbuild-serial/src/boards.rs b/crates/fbuild-serial/src/boards.rs index 8a658a83..59966294 100644 --- a/crates/fbuild-serial/src/boards.rs +++ b/crates/fbuild-serial/src/boards.rs @@ -820,18 +820,19 @@ pub fn family_for_port(name: &str) -> Option { family_for_port_via_kernel_class_inner(kernel_class) } -/// Walk `serialport::available_ports()` for `name` and return the +/// Walk the blessed port enumerator for `name` and return the /// USB port's `(vid, pid, family_lookup_result)`. Returns `None` when /// the port is not present in the live enumeration or is not a USB /// port (real UART, Bluetooth virtual serial, etc.). When the port IS /// USB but its VID/PID isn't in the table, returns `Some((vid, pid, None))`. fn lookup_port_vid_pid(name: &str) -> Option<(u16, u16, Option)> { - let ports = serialport::available_ports().ok()?; + let ports = crate::ports::available_ports().ok()?; for port in ports { - if !serial_port_name_matches(&port.port_name, name) { + if port.health.is_known_unhealthy() || !serial_port_name_matches(&port.info.port_name, name) + { continue; } - if let serialport::SerialPortType::UsbPort(info) = port.port_type { + if let serialport::SerialPortType::UsbPort(info) = port.info.port_type { return Some((info.vid, info.pid, family_for_vid_pid(info.vid, info.pid))); } } diff --git a/crates/fbuild-serial/src/bootloader_watcher.rs b/crates/fbuild-serial/src/bootloader_watcher.rs index 8674c50a..2beb29df 100644 --- a/crates/fbuild-serial/src/bootloader_watcher.rs +++ b/crates/fbuild-serial/src/bootloader_watcher.rs @@ -161,15 +161,18 @@ pub struct SerialPortSource; impl PortSource for SerialPortSource { fn snapshot(&self) -> Vec { - match serialport::available_ports() { + match crate::ports::available_ports() { Ok(ports) => ports .into_iter() .filter_map(|port| { - if let serialport::SerialPortType::UsbPort(info) = port.port_type { + if port.health.is_known_unhealthy() { + return None; + } + if let serialport::SerialPortType::UsbPort(info) = port.info.port_type { Some(PortFingerprint { vid: info.vid, pid: info.pid, - name: port.port_name, + name: port.info.port_name, }) } else { None diff --git a/crates/fbuild-serial/src/ports.rs b/crates/fbuild-serial/src/ports.rs index b96022c5..52c63665 100644 --- a/crates/fbuild-serial/src/ports.rs +++ b/crates/fbuild-serial/src/ports.rs @@ -22,11 +22,125 @@ //! plus population of the composite-interface index (`MI_xx`) so callers can //! disambiguate a Teensy's Serial vs Serial+MIDI functions. +/// Current host health for an enumerated serial endpoint. +/// +/// Windows distinguishes a devnode that is present and healthy from a +/// present-but-problematic devnode and a historical phantom record. Those are +/// facts about the endpoint, not selection policy: callers may keep an +/// unhealthy record for diagnostics while rejecting it for deployment. +#[derive(Clone, Debug, Eq, PartialEq)] +pub enum PortHealth { + /// The devnode is in the live tree and Config Manager reports no problem. + HealthyPresent, + /// The devnode is present but Config Manager returned a non-zero problem. + PresentProblem { + problem_code: u32, + status: Option, + }, + /// The devnode is retained by Windows history but is not in the live tree. + Phantom { + problem_code: Option, + status: Option, + }, + /// The host cannot provide equivalent health data (normal off Windows). + Unknown, +} + +impl PortHealth { + /// True only for states that positively prove the endpoint is unhealthy. + pub fn is_known_unhealthy(&self) -> bool { + matches!(self, Self::PresentProblem { .. } | Self::Phantom { .. }) + } + + /// Stable lowercase label for diagnostics and machine-readable callers. + pub fn label(&self) -> &'static str { + match self { + Self::HealthyPresent => "healthy", + Self::PresentProblem { .. } => "present-problem", + Self::Phantom { .. } => "phantom", + Self::Unknown => "unknown", + } + } + + /// Config Manager problem code when the operating system supplied one. + pub fn problem_code(&self) -> Option { + match self { + Self::PresentProblem { problem_code, .. } => Some(*problem_code), + Self::Phantom { problem_code, .. } => *problem_code, + Self::HealthyPresent | Self::Unknown => None, + } + } +} + +/// A serial endpoint plus the host facts needed to decide whether it is safe +/// to select. [`Self::info`] retains the upstream `serialport` shape for +/// callers that only need USB identity or a port name. +#[derive(Clone, Debug)] +pub struct DetectedPort { + pub info: serialport::SerialPortInfo, + pub health: PortHealth, + /// Canonical Plug and Play device instance ID when the host exposes one. + pub instance_id: Option, + /// Immediate parent device instance ID when the host exposes one. + pub parent_instance_id: Option, +} + +impl DetectedPort { + pub fn unknown(info: serialport::SerialPortInfo) -> Self { + Self { + info, + health: PortHealth::Unknown, + instance_id: None, + parent_instance_id: None, + } + } +} + +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +enum PnpObservation { + Present { status: u32, problem_code: u32 }, + Phantom, + Unknown, +} + +fn classify_port_health(observation: PnpObservation) -> PortHealth { + match observation { + PnpObservation::Present { + status: _, + problem_code: 0, + } => PortHealth::HealthyPresent, + PnpObservation::Present { + status, + problem_code, + } => PortHealth::PresentProblem { + problem_code, + status: Some(status), + }, + PnpObservation::Phantom => PortHealth::Phantom { + problem_code: None, + status: None, + }, + PnpObservation::Unknown => PortHealth::Unknown, + } +} + +fn health_for_endpoint(observation: PnpObservation, is_usb: bool) -> PortHealth { + if is_usb { + classify_port_health(observation) + } else { + // A PnP status for a UART, Bluetooth, or other non-USB endpoint is + // not equivalent to the USB health contract consumers use for deploy + // selection. Preserve the cross-platform `Unknown` behavior instead. + PortHealth::Unknown + } +} + /// Enumerate every serial port currently visible to the OS. /// /// Unlike [`serialport::available_ports`], on Windows this includes ports -/// whose devnode status is not "OK" (the Teensy / composite-device case). -pub fn available_ports() -> serialport::Result> { +/// whose devnode status is not "OK" (the Teensy / composite-device case) and +/// preserves that health in the returned record. +pub fn available_ports() -> serialport::Result> { #[cfg(windows)] { imp::available_ports() @@ -34,6 +148,7 @@ pub fn available_ports() -> serialport::Result> #[cfg(not(windows))] { serialport::available_ports() + .map(|ports| ports.into_iter().map(DetectedPort::unknown).collect()) } } @@ -69,18 +184,85 @@ pub fn present_usb_problem_devices() -> Vec { } } +#[cfg(test)] +mod health_tests { + use super::*; + + #[test] + fn classifies_healthy_problem_phantom_and_unknown_endpoints() { + assert_eq!( + classify_port_health(PnpObservation::Present { + status: 0, + problem_code: 0, + }), + PortHealth::HealthyPresent + ); + assert_eq!( + classify_port_health(PnpObservation::Present { + status: 0x1234, + problem_code: 31, + }), + PortHealth::PresentProblem { + problem_code: 31, + status: Some(0x1234), + } + ); + assert_eq!( + classify_port_health(PnpObservation::Phantom), + PortHealth::Phantom { + problem_code: None, + status: None, + } + ); + assert_eq!( + classify_port_health(PnpObservation::Unknown), + PortHealth::Unknown + ); + assert_eq!( + health_for_endpoint( + PnpObservation::Present { + status: 0, + problem_code: 31, + }, + false, + ), + PortHealth::Unknown + ); + } + + #[test] + fn only_problem_and_phantom_states_are_known_unhealthy() { + assert!(!PortHealth::HealthyPresent.is_known_unhealthy()); + assert!(!PortHealth::Unknown.is_known_unhealthy()); + assert!( + PortHealth::PresentProblem { + problem_code: 43, + status: Some(0), + } + .is_known_unhealthy() + ); + assert!( + PortHealth::Phantom { + problem_code: None, + status: None, + } + .is_known_unhealthy() + ); + } +} + #[cfg(windows)] mod imp { - use super::UsbProblemDevice; + use super::{DetectedPort, PnpObservation, UsbProblemDevice, health_for_endpoint}; use std::collections::HashSet; use std::ptr; use serialport::{SerialPortInfo, SerialPortType, UsbPortInfo}; use windows_sys::Win32::Devices::DeviceAndDriverInstallation::{ - CM_Get_DevNode_Status, CM_Get_Device_IDW, CM_Get_Parent, CR_SUCCESS, DICS_FLAG_GLOBAL, - DIGCF_PRESENT, DIREG_DEV, GUID_DEVCLASS_USB, HDEVINFO, MAX_DEVICE_ID_LEN, SP_DEVINFO_DATA, - SPDRP_FRIENDLYNAME, SPDRP_HARDWAREID, SPDRP_LOCATION_INFORMATION, SPDRP_MFG, - SetupDiClassGuidsFromNameW, SetupDiDestroyDeviceInfoList, SetupDiEnumDeviceInfo, + CM_Get_DevNode_Status, CM_Get_Device_IDW, CM_Get_Parent, CR_NO_SUCH_DEVINST, CR_SUCCESS, + DICS_FLAG_GLOBAL, DIGCF_PRESENT, DIREG_DEV, GUID_DEVCLASS_USB, HDEVINFO, MAX_DEVICE_ID_LEN, + SP_DEVINFO_DATA, SPDRP_FRIENDLYNAME, SPDRP_HARDWAREID, SPDRP_LOCATION_INFORMATION, + SPDRP_MFG, SetupDiClassGuidsFromNameW, SetupDiDestroyDeviceInfoList, SetupDiEnumDeviceInfo, SetupDiGetClassDevsW, SetupDiGetDeviceInstanceIdW, SetupDiGetDeviceRegistryPropertyW, SetupDiOpenDevRegKey, }; @@ -381,23 +563,36 @@ mod imp { from_utf16_lossy_trimmed(port_name) } - /// True when the devnode is instantiated in the live device tree. - /// A phantom devnode (unplugged, or a Status=Unknown Teensy interface - /// whose function driver never started) returns `CR_NO_SUCH_DEVINST` - /// here, i.e. `false`. - fn present(&mut self) -> bool { + /// Read the Config Manager observation without flattening its three + /// important outcomes. A missing live devnode is a phantom; a query + /// failure that is not that explicit state remains unknown. + fn pnp_observation(&mut self) -> PnpObservation { let mut status = 0u32; let mut problem = 0u32; + // SAFETY: `DevInst` comes from the live SetupAPI record and both + // output pointers reference initialized writable local storage. let res = unsafe { CM_Get_DevNode_Status(&mut status, &mut problem, self.devinfo_data.DevInst, 0) }; - res == CR_SUCCESS + if res == CR_SUCCESS { + PnpObservation::Present { + status, + problem_code: problem, + } + } else if res == CR_NO_SUCH_DEVINST { + PnpObservation::Phantom + } else { + PnpObservation::Unknown + } } - fn port_type(&mut self) -> SerialPortType { - self.instance_id() - .map(|s| (s, self.parent_instance_id())) - .and_then(|(d, p)| parse_usb_port_info(&d, p.as_deref())) + fn port_type( + &mut self, + instance_id: Option<&str>, + parent_instance_id: Option<&str>, + ) -> SerialPortType { + instance_id + .and_then(|id| parse_usb_port_info(id, parent_instance_id)) .map(|mut info: UsbPortInfo| { info.manufacturer = self.property(SPDRP_MFG); info.product = self.property(SPDRP_FRIENDLYNAME); @@ -652,7 +847,7 @@ mod imp { ports_list } - pub(super) fn available_ports() -> serialport::Result> { + pub(super) fn available_ports() -> serialport::Result> { let mut ports = Vec::new(); let mut seen: HashSet = HashSet::new(); for guid in get_ports_guids()? { @@ -667,8 +862,11 @@ mod imp { if port_name.starts_with("LPT") { continue; } - let present = port_device.present(); - let port_type = port_device.port_type(); + let instance_id = port_device.instance_id(); + let parent_instance_id = port_device.parent_instance_id(); + let pnp_observation = port_device.pnp_observation(); + let port_type = + port_device.port_type(instance_id.as_deref(), parent_instance_id.as_deref()); let is_usb = matches!(port_type, SerialPortType::UsbPort(_)); // Include every present port (unchanged behaviour), PLUS // non-present USB serial ports — the Status=Unknown Teensy @@ -676,17 +874,23 @@ mod imp { // devnode is a stale phantom with no VID:PID to act on, so we // leave it out to avoid resurrecting ancient ACPI/BT junk. // FastLED/fbuild#962. - if !present && !is_usb { + if matches!(pnp_observation, super::PnpObservation::Phantom) && !is_usb { continue; } + let health = health_for_endpoint(pnp_observation, is_usb); // A phantom devnode can be enumerated once per matching class // GUID; de-dup on the COM name. if !seen.insert(port_name.clone()) { continue; } - ports.push(SerialPortInfo { - port_name, - port_type, + ports.push(DetectedPort { + info: SerialPortInfo { + port_name, + port_type, + }, + health, + instance_id, + parent_instance_id, }); } } @@ -694,10 +898,10 @@ mod imp { // Fold in any DEVICEMAP\SERIALCOMM ports not already found. for raw_port in get_registry_com_ports() { if seen.insert(raw_port.clone()) { - ports.push(SerialPortInfo { + ports.push(DetectedPort::unknown(SerialPortInfo { port_name: raw_port, port_type: SerialPortType::Unknown, - }); + })); } } Ok(ports)