Skip to content

Commit 88ca895

Browse files
RealWhyKnotRealWhyKnot
committed
fix(diag): report fallback and USB mount states (2026.6.16.0-C8ED)
1 parent aa7b2d9 commit 88ca895

5 files changed

Lines changed: 268 additions & 27 deletions

File tree

‎bridge/src/cmd_auto.rs‎

Lines changed: 25 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,11 @@ pub(crate) const PLAYSTATION_FAMILY: &[Persona] = &[Persona::Ps3, Persona::Ps4];
1616
pub(crate) enum AutoScore {
1717
NoUsbTraffic = 0,
1818
EnumerationStarted = 1,
19-
Configured = 2,
20-
Polling = 3,
21-
PollingWithOut = 4,
19+
ConfiguredThenUnmounted = 2,
20+
Suspended = 3,
21+
Configured = 4,
22+
Polling = 5,
23+
PollingWithOut = 6,
2224
}
2325

2426
#[derive(Clone, Debug)]
@@ -304,10 +306,19 @@ pub(crate) fn score_usb_diag(diag: &UsbDiag) -> AutoScore {
304306
AutoScore::Polling
305307
} else if diag.mounted() {
306308
AutoScore::Configured
307-
} else if diag.device_desc_count > 0 || diag.config_desc_count > 0 {
308-
AutoScore::EnumerationStarted
309309
} else {
310-
AutoScore::NoUsbTraffic
310+
match diag.configuration_state() {
311+
crate::protocol::UsbConfigurationState::NoHostTraffic => AutoScore::NoUsbTraffic,
312+
crate::protocol::UsbConfigurationState::EnumerationStarted => {
313+
AutoScore::EnumerationStarted
314+
}
315+
crate::protocol::UsbConfigurationState::ConfiguredThenUnmounted
316+
| crate::protocol::UsbConfigurationState::ConfiguredThenUnmountedWithoutCallback => {
317+
AutoScore::ConfiguredThenUnmounted
318+
}
319+
crate::protocol::UsbConfigurationState::Suspended => AutoScore::Suspended,
320+
crate::protocol::UsbConfigurationState::Configured => AutoScore::Configured,
321+
}
311322
}
312323
}
313324

@@ -319,6 +330,8 @@ fn score_label(score: AutoScore) -> &'static str {
319330
match score {
320331
AutoScore::NoUsbTraffic => "no USB host enumeration traffic",
321332
AutoScore::EnumerationStarted => "USB host started enumeration but did not configure",
333+
AutoScore::ConfiguredThenUnmounted => "USB configured once, then was not mounted",
334+
AutoScore::Suspended => "USB suspended",
322335
AutoScore::Configured => "USB configured but no input report accepted yet",
323336
AutoScore::Polling => "USB host accepted input reports",
324337
AutoScore::PollingWithOut => "USB host accepted input reports and sent OUT traffic",
@@ -435,6 +448,12 @@ mod tests {
435448
score_usb_diag(&diag(false, false, false, true)),
436449
AutoScore::EnumerationStarted
437450
);
451+
let mut unmounted = diag(false, false, false, true);
452+
unmounted.mount_count = 1;
453+
assert_eq!(
454+
score_usb_diag(&unmounted),
455+
AutoScore::ConfiguredThenUnmounted
456+
);
438457
assert_eq!(
439458
score_usb_diag(&diag(true, false, false, true)),
440459
AutoScore::Configured

‎bridge/src/cmd_bundle/pico_diag.rs‎

Lines changed: 79 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -380,16 +380,41 @@ pub(super) async fn capture_pico_diag() -> DiagOutcome {
380380
return udp_result;
381381
}
382382

383-
// All three paths failed. Prefer the CDC outcome for diagnostic
384-
// specificity (it has step / elapsed / rx_bytes detail); the vendor
385-
// and UDP outcomes get logged but not surfaced in the manifest.
386383
tracing::warn!(
387384
"bundle: all three diag paths failed (cdc={}, vendor={}, udp={})",
388385
cdc_result.discriminant_str(),
389386
vendor_result.discriminant_str(),
390387
udp_result.discriminant_str()
391388
);
392-
cdc_result
389+
choose_failed_diag_outcome(cdc_result, vendor_result, udp_result)
390+
}
391+
392+
fn choose_failed_diag_outcome(
393+
cdc_result: DiagOutcome,
394+
vendor_result: DiagOutcome,
395+
udp_result: DiagOutcome,
396+
) -> DiagOutcome {
397+
let mut best = cdc_result;
398+
for candidate in [vendor_result, udp_result] {
399+
if failure_rank(&candidate) > failure_rank(&best) {
400+
best = candidate;
401+
}
402+
}
403+
best
404+
}
405+
406+
fn failure_rank(outcome: &DiagOutcome) -> u8 {
407+
match outcome {
408+
DiagOutcome::Captured { .. } | DiagOutcome::Empty { .. } => 100,
409+
DiagOutcome::SetupProbeFailed { .. }
410+
| DiagOutcome::VendorTransferFailed { .. }
411+
| DiagOutcome::UdpProbeFailed { .. } => 60,
412+
DiagOutcome::SetupOpenFailed { .. } | DiagOutcome::VendorOpenFailed { .. } => 50,
413+
DiagOutcome::UdpUnsupported { .. } => 45,
414+
DiagOutcome::UdpDiscoveryFailed { .. } => 40,
415+
DiagOutcome::NoLastPicoInConfig => 20,
416+
DiagOutcome::NoSetupPort | DiagOutcome::VendorNotFound => 10,
417+
}
393418
}
394419

395420
pub(super) async fn capture_run_udp_for_target(pico: &cmd_run::PicoTarget) -> DiagOutcome {
@@ -973,4 +998,54 @@ mod tests {
973998
.source_str()
974999
.is_none());
9751000
}
1001+
1002+
#[test]
1003+
fn failed_diag_selection_surfaces_udp_last_known_failure() {
1004+
let selected = choose_failed_diag_outcome(
1005+
DiagOutcome::NoSetupPort,
1006+
DiagOutcome::VendorNotFound,
1007+
DiagOutcome::UdpDiscoveryFailed {
1008+
reason: "broadcast: no ack; unicast: no ack".into(),
1009+
},
1010+
);
1011+
1012+
assert_eq!(selected.discriminant_str(), "udp_discovery_failed");
1013+
let stub = selected.stub_text();
1014+
assert!(
1015+
stub.contains("run-mode UDP probe"),
1016+
"wrong stub selected: {stub}"
1017+
);
1018+
}
1019+
1020+
#[test]
1021+
fn failed_diag_selection_keeps_setup_probe_detail() {
1022+
let selected = choose_failed_diag_outcome(
1023+
DiagOutcome::SetupProbeFailed {
1024+
port: "COM3".into(),
1025+
step: "read",
1026+
elapsed_ms: 3012,
1027+
bytes_received: 0,
1028+
rx_first_32_hex: "none".into(),
1029+
error: "timed out".into(),
1030+
},
1031+
DiagOutcome::VendorNotFound,
1032+
DiagOutcome::UdpDiscoveryFailed {
1033+
reason: "no ack".into(),
1034+
},
1035+
);
1036+
1037+
assert_eq!(selected.discriminant_str(), "setup_probe_failed");
1038+
assert!(selected.stub_text().contains("port: COM3"));
1039+
}
1040+
1041+
#[test]
1042+
fn failed_diag_selection_distinguishes_no_known_run_mode_pico() {
1043+
let selected = choose_failed_diag_outcome(
1044+
DiagOutcome::NoSetupPort,
1045+
DiagOutcome::VendorNotFound,
1046+
DiagOutcome::NoLastPicoInConfig,
1047+
);
1048+
1049+
assert_eq!(selected.discriminant_str(), "no_last_pico_in_config");
1050+
}
9761051
}

‎bridge/src/cmd_usb_diag.rs‎

Lines changed: 61 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -194,11 +194,7 @@ pub fn format_usb_diag(diag: &protocol::UsbDiag, persona: protocol::Persona) ->
194194
let _ = writeln!(
195195
out,
196196
" USB: {}{} mounts={} unmounts={} suspends={} resumes={}",
197-
if diag.mounted() {
198-
"configured"
199-
} else {
200-
"not configured"
201-
},
197+
usb_state_label(diag),
202198
if diag.suspended() { " / suspended" } else { "" },
203199
diag.mount_count,
204200
diag.umount_count,
@@ -252,13 +248,28 @@ pub fn format_usb_diag(diag: &protocol::UsbDiag, persona: protocol::Persona) ->
252248
}
253249

254250
fn usb_verdict(diag: &protocol::UsbDiag, device_label: &str) -> String {
255-
if !diag.mounted() {
256-
if diag.device_desc_count > 0 || diag.config_desc_count > 0 {
257-
format!("FAIL USB host started enumeration but did not configure the {device_label} device.")
258-
} else {
259-
"FAIL Pico sees no USB host enumeration traffic.".to_string()
251+
match diag.configuration_state() {
252+
protocol::UsbConfigurationState::NoHostTraffic => {
253+
return "FAIL Pico sees no USB host enumeration traffic.".to_string();
254+
}
255+
protocol::UsbConfigurationState::EnumerationStarted => {
256+
return format!("FAIL USB host started enumeration but did not configure the {device_label} device.");
257+
}
258+
protocol::UsbConfigurationState::ConfiguredThenUnmountedWithoutCallback => {
259+
return format!("FAIL USB configured the {device_label} device once, then current state is not mounted and no unmount callback was recorded.");
260+
}
261+
protocol::UsbConfigurationState::ConfiguredThenUnmounted => {
262+
return format!("FAIL USB configured the {device_label} device once, then the host disconnected or reset it.");
263+
}
264+
protocol::UsbConfigurationState::Suspended => {
265+
return format!(
266+
"WARN USB is suspended; the host has not resumed the {device_label} device."
267+
);
260268
}
261-
} else if !diag.xinput_report_sent() {
269+
protocol::UsbConfigurationState::Configured => {}
270+
}
271+
272+
if !diag.xinput_report_sent() {
262273
if diag.in_blocked_total() > 0 && diag.last_in_blocked_reason != 0 {
263274
format!(
264275
"WARN USB is configured, but the latest {device_label} report was blocked: {} (want={} got={}).",
@@ -278,6 +289,19 @@ fn usb_verdict(diag: &protocol::UsbDiag, device_label: &str) -> String {
278289
}
279290
}
280291

292+
fn usb_state_label(diag: &protocol::UsbDiag) -> &'static str {
293+
match diag.configuration_state() {
294+
protocol::UsbConfigurationState::Configured => "configured",
295+
protocol::UsbConfigurationState::Suspended => "suspended",
296+
protocol::UsbConfigurationState::ConfiguredThenUnmounted
297+
| protocol::UsbConfigurationState::ConfiguredThenUnmountedWithoutCallback => {
298+
"not mounted now (configured earlier)"
299+
}
300+
protocol::UsbConfigurationState::EnumerationStarted
301+
| protocol::UsbConfigurationState::NoHostTraffic => "not configured",
302+
}
303+
}
304+
281305
fn age_label(diag: &protocol::UsbDiag, timestamp_ms: u32) -> String {
282306
match diag.age_ms(timestamp_ms) {
283307
Some(ms) if ms < 1000 => format!("{ms} ms ago"),
@@ -358,6 +382,32 @@ mod tests {
358382
assert!(usb_verdict(&diag(true, true, true, true), "XInput").contains("OUT traffic"));
359383
}
360384

385+
#[test]
386+
fn verdict_distinguishes_configured_then_unmounted() {
387+
let mut d = diag(false, false, false, true);
388+
d.mount_count = 1;
389+
d.umount_count = 0;
390+
391+
let verdict = usb_verdict(&d, "PS3 HID gamepad");
392+
assert!(
393+
verdict.contains("configured the PS3 HID gamepad device once"),
394+
"wrong verdict: {verdict}"
395+
);
396+
assert!(
397+
verdict.contains("no unmount callback"),
398+
"missing callback clue: {verdict}"
399+
);
400+
assert!(
401+
!verdict.contains("did not configure"),
402+
"kept old verdict: {verdict}"
403+
);
404+
let text = format_usb_diag(&d, protocol::Persona::Ps3);
405+
assert!(
406+
text.contains("USB: not mounted now (configured earlier)"),
407+
"wrong USB state: {text}"
408+
);
409+
}
410+
361411
#[test]
362412
fn verdict_uses_persona_label() {
363413
let warn = usb_verdict(&diag(true, false, false, true), "HID keyboard");

‎bridge/src/pico_cache.rs‎

Lines changed: 73 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -282,13 +282,22 @@ fn warn_once(message: String) {
282282
}
283283

284284
fn usb_verdict_label(diag: &protocol::UsbDiag, persona: protocol::Persona) -> &'static str {
285-
if !diag.mounted() {
286-
if diag.device_desc_count > 0 || diag.config_desc_count > 0 {
287-
"enumeration_started_not_configured"
288-
} else {
289-
"no_usb_host_traffic"
285+
match diag.configuration_state() {
286+
protocol::UsbConfigurationState::NoHostTraffic => return "no_usb_host_traffic",
287+
protocol::UsbConfigurationState::EnumerationStarted => {
288+
return "enumeration_started_not_configured"
290289
}
291-
} else if !diag.xinput_report_sent() {
290+
protocol::UsbConfigurationState::ConfiguredThenUnmounted => {
291+
return "configured_then_unmounted"
292+
}
293+
protocol::UsbConfigurationState::ConfiguredThenUnmountedWithoutCallback => {
294+
return "configured_then_unmounted_without_callback"
295+
}
296+
protocol::UsbConfigurationState::Suspended => return "configured_suspended",
297+
protocol::UsbConfigurationState::Configured => {}
298+
}
299+
300+
if !diag.xinput_report_sent() {
292301
match diag.last_in_blocked_reason {
293302
protocol::USB_DIAG_IN_BLOCKED_NOT_MOUNTED => "configured_report_blocked_not_mounted",
294303
protocol::USB_DIAG_IN_BLOCKED_NOT_READY => "configured_report_blocked_not_ready",
@@ -329,4 +338,62 @@ mod tests {
329338
assert!(!json.to_ascii_lowercase().contains("password"));
330339
assert!(!json.to_ascii_lowercase().contains("ssid"));
331340
}
341+
342+
fn usb_diag(mounted: bool, desc: bool) -> protocol::UsbDiag {
343+
protocol::UsbDiag {
344+
seq: 1,
345+
flags: 0,
346+
version: protocol::USB_DIAG_VERSION,
347+
usb_flags: if mounted {
348+
protocol::USB_DIAG_FLAG_MOUNTED
349+
} else {
350+
0
351+
},
352+
activity_flags: 0,
353+
last_out_len: 0,
354+
now_ms: 10_000,
355+
last_bridge_packet_ms: 0,
356+
mount_count: if mounted { 1 } else { 0 },
357+
umount_count: 0,
358+
suspend_count: 0,
359+
resume_count: 0,
360+
device_desc_count: if desc { 1 } else { 0 },
361+
config_desc_count: if desc { 1 } else { 0 },
362+
xinput_in_queued_count: 0,
363+
xinput_in_sent_count: 0,
364+
xinput_out_count: 0,
365+
xinput_in_blocked_not_mounted_count: 0,
366+
xinput_in_blocked_not_ready_count: 0,
367+
xinput_in_blocked_short_write_count: 0,
368+
xinput_in_idle_suppressed_count: 0,
369+
last_mount_ms: 9000,
370+
last_umount_ms: 0,
371+
last_in_queued_ms: 0,
372+
last_in_sent_ms: 0,
373+
last_out_ms: 0,
374+
last_in_blocked_ms: 0,
375+
last_in_blocked_reason: 0,
376+
last_in_blocked_want: 0,
377+
last_in_blocked_got: 0,
378+
last_out_byte0: 0,
379+
last_out_byte1: 0,
380+
}
381+
}
382+
383+
#[test]
384+
fn usb_verdict_label_keeps_dropped_after_mount_distinct() {
385+
let mut diag = usb_diag(false, true);
386+
diag.mount_count = 1;
387+
388+
assert_eq!(
389+
usb_verdict_label(&diag, protocol::Persona::Ps3),
390+
"configured_then_unmounted_without_callback"
391+
);
392+
393+
diag.umount_count = 1;
394+
assert_eq!(
395+
usb_verdict_label(&diag, protocol::Persona::Ps3),
396+
"configured_then_unmounted"
397+
);
398+
}
332399
}

‎bridge/src/protocol.rs‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,16 @@ pub const PICO_STATE_WIRE_SIZE: usize = 128;
112112
pub const PICO_STATE_V1_VERSION: u8 = 1;
113113
pub const PICO_STATE_VERSION: u8 = 2;
114114

115+
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
116+
pub enum UsbConfigurationState {
117+
NoHostTraffic,
118+
EnumerationStarted,
119+
ConfiguredThenUnmounted,
120+
ConfiguredThenUnmountedWithoutCallback,
121+
Suspended,
122+
Configured,
123+
}
124+
115125
/// Set in a `TYPE_LOG_CHUNK` datagram's `flags` byte to mark the final
116126
/// chunk in the reply sequence.
117127
pub const LOG_CHUNK_FLAG_LAST: u8 = 1 << 0;
@@ -705,6 +715,26 @@ impl UsbDiag {
705715
.saturating_add(self.xinput_in_blocked_short_write_count)
706716
}
707717

718+
pub fn configuration_state(&self) -> UsbConfigurationState {
719+
if self.mounted() {
720+
return UsbConfigurationState::Configured;
721+
}
722+
if self.suspended() {
723+
return UsbConfigurationState::Suspended;
724+
}
725+
if self.mount_count > 0 {
726+
if self.umount_count == 0 {
727+
return UsbConfigurationState::ConfiguredThenUnmountedWithoutCallback;
728+
}
729+
return UsbConfigurationState::ConfiguredThenUnmounted;
730+
}
731+
if self.device_desc_count > 0 || self.config_desc_count > 0 {
732+
UsbConfigurationState::EnumerationStarted
733+
} else {
734+
UsbConfigurationState::NoHostTraffic
735+
}
736+
}
737+
708738
pub fn age_ms(&self, timestamp_ms: u32) -> Option<u32> {
709739
if timestamp_ms == 0 {
710740
None

0 commit comments

Comments
 (0)