Skip to content

Commit 7b6b416

Browse files
committed
fix(cli): harden routing recovery
Rediscover stale Pico routes during streaming, reject duplicate Pico targets, and improve no-button flash recovery guidance. Keep the guided flash menu clippy-clean while preserving the same choices.
1 parent e1f72d8 commit 7b6b416

3 files changed

Lines changed: 145 additions & 35 deletions

File tree

bridge/src/cmd_flash.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -331,7 +331,10 @@ async fn reboot_setup_picos_to_bootsel(all: bool) -> Result<usize> {
331331
let mut ports = cdc::find_setup_ports()?;
332332
if ports.is_empty() {
333333
bail!(
334-
"no setup-mode Pico found (looking for VID 0x{:04X} PID 0x{:04X})",
334+
"no setup-mode Pico found (looking for VID 0x{:04X} PID 0x{:04X}). \
335+
No-button flashing only works when CouchLink setup-mode firmware is visible over USB. \
336+
If the Pico is already in run mode, use `couchlink flash --all` and put it in BOOTSEL, \
337+
or run `couchlink` and choose Flash or update Pico firmware for the guided path.",
335338
cdc::SETUP_VID,
336339
cdc::SETUP_PID,
337340
);

bridge/src/cmd_home.rs

Lines changed: 24 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -209,39 +209,33 @@ async fn stream(routes: Vec<cmd_run::StreamRoute>, save: bool) -> Result<()> {
209209
}
210210

211211
async fn flash_menu() -> Result<()> {
212-
loop {
213-
println!();
214-
println!("Firmware update");
215-
println!("Use setup-mode USB for no-button reflashing when the Pico firmware is already running setup mode.");
216-
let choices = vec![
217-
"Update setup-mode Pico(s) without BOOTSEL",
218-
"Flash Pico(s) already in BOOTSEL",
219-
"First-time setup wizard",
220-
"Back",
221-
];
222-
match select("Flash path", &choices, 0).await? {
223-
0 => {
224-
let result = cmd_flash::run(None, true, true).await;
225-
if let Err(e) = result {
226-
println!();
227-
println!("No-button flash did not complete:");
228-
println!(" {e:#}");
229-
if confirm("Try BOOTSEL-drive flashing instead?", true).await? {
230-
cmd_flash::run(None, true, false).await?;
231-
}
212+
println!();
213+
println!("Firmware update");
214+
println!(
215+
"Use setup-mode USB for no-button reflashing when the Pico firmware is already running setup mode."
216+
);
217+
let choices = vec![
218+
"Update setup-mode Pico(s) without BOOTSEL",
219+
"Flash Pico(s) already in BOOTSEL",
220+
"First-time setup wizard",
221+
"Back",
222+
];
223+
match select("Flash path", &choices, 0).await? {
224+
0 => {
225+
let result = cmd_flash::run(None, true, true).await;
226+
if let Err(e) = result {
227+
println!();
228+
println!("No-button flash did not complete:");
229+
println!(" {e:#}");
230+
if confirm("Try BOOTSEL-drive flashing instead?", true).await? {
231+
cmd_flash::run(None, true, false).await?;
232232
}
233-
return Ok(());
234-
}
235-
1 => {
236-
cmd_flash::run(None, true, false).await?;
237-
return Ok(());
238-
}
239-
2 => {
240-
cmd_setup::run(None).await?;
241-
return Ok(());
242233
}
243-
_ => return Ok(()),
234+
Ok(())
244235
}
236+
1 => cmd_flash::run(None, true, false).await,
237+
2 => cmd_setup::run(None).await,
238+
_ => Ok(()),
245239
}
246240
}
247241

bridge/src/cmd_run.rs

Lines changed: 117 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,13 @@
33
//! module keeps the scriptable route syntax for startup shortcuts and
44
//! third-party launchers.
55
6+
use std::collections::HashSet;
67
use std::net::{IpAddr, SocketAddr};
78
use std::time::{Duration, Instant};
89

910
use anyhow::{anyhow, bail, Context, Result};
1011
use tokio::net::UdpSocket;
12+
use tokio::sync::mpsc;
1113
use tokio::time::{interval, MissedTickBehavior};
1214

1315
use crate::protocol::{self, GamepadState, Packet, PacketKind, FLAG_PARSEC_CONNECTED};
@@ -17,6 +19,8 @@ const DEFAULT_DISCOVER_SECONDS: u64 = 5;
1719
const DEFAULT_STATUS_SECONDS: u64 = 2;
1820
const STREAM_TICK: Duration = Duration::from_millis(16);
1921
const PEER_STALE_AFTER: Duration = Duration::from_secs(5);
22+
const PEER_RECOVER_EVERY: Duration = Duration::from_secs(10);
23+
const PEER_RECOVERY_DISCOVER: Duration = Duration::from_secs(2);
2024

2125
#[derive(Clone, Debug)]
2226
pub struct RunOptions {
@@ -238,8 +242,8 @@ pub fn parse_route_specs(specs: &[String], picos: &[PicoTarget]) -> Result<Vec<S
238242
pub fn parse_user_slot(input: &str) -> Result<u32> {
239243
let s = input
240244
.trim()
241-
.trim_start_matches(|c: char| c == 'p' || c == 'P')
242-
.trim_start_matches(|c: char| c == 'x' || c == 'X');
245+
.trim_start_matches(['p', 'P'])
246+
.trim_start_matches(['x', 'X']);
243247
let user_slot: u32 = s
244248
.parse()
245249
.with_context(|| format!("invalid controller slot `{input}`"))?;
@@ -360,6 +364,8 @@ pub async fn stream_routes(routes: Vec<StreamRoute>, options: StreamOptions) ->
360364
let mut status = interval(Duration::from_secs(options.status_seconds.max(1)));
361365
status.set_missed_tick_behavior(MissedTickBehavior::Skip);
362366
let mut buf = [0u8; 64];
367+
let (recovery_tx, mut recovery_rx) = mpsc::channel::<Result<Vec<PicoTarget>>>(1);
368+
let mut recovery_in_flight = false;
363369

364370
loop {
365371
tokio::select! {
@@ -376,8 +382,30 @@ pub async fn stream_routes(routes: Vec<StreamRoute>, options: StreamOptions) ->
376382
Err(e) => return Err(e).context("receiving stream reply"),
377383
}
378384
}
379-
_ = status.tick(), if !options.quiet => {
380-
print_status(&mut runtime);
385+
_ = status.tick() => {
386+
if !options.quiet {
387+
print_status(&mut runtime);
388+
}
389+
if !recovery_in_flight && schedule_recovery_if_needed(&mut runtime) {
390+
recovery_in_flight = true;
391+
let tx = recovery_tx.clone();
392+
tokio::spawn(async move {
393+
let result = discover_picos(PEER_RECOVERY_DISCOVER).await;
394+
let _ = tx.send(result).await;
395+
});
396+
}
397+
}
398+
recovery = recovery_rx.recv() => {
399+
recovery_in_flight = false;
400+
match recovery {
401+
Some(Ok(picos)) => apply_recovery_results(&mut runtime, &picos, options.quiet),
402+
Some(Err(e)) if !options.quiet => {
403+
println!("Recovery discovery failed: {e:#}");
404+
println!(" Check Wi-Fi, firewall, and router client isolation, then run `couchlink test discover --all`.");
405+
}
406+
Some(Err(_)) => {}
407+
None => {}
408+
}
381409
}
382410
_ = tokio::signal::ctrl_c() => {
383411
if !options.quiet {
@@ -391,10 +419,17 @@ pub async fn stream_routes(routes: Vec<StreamRoute>, options: StreamOptions) ->
391419
}
392420

393421
fn validate_routes(routes: &[StreamRoute]) -> Result<()> {
422+
let mut pico_uids = HashSet::new();
394423
for route in routes {
395424
if route.source_slot >= 4 {
396425
bail!("controller slot must be 1, 2, 3, or 4");
397426
}
427+
if !pico_uids.insert(route.pico.info.unique_id_short) {
428+
bail!(
429+
"the same Pico ({}) is routed more than once. Pick one source controller per Pico.",
430+
route.pico.uid_hex()
431+
);
432+
}
398433
}
399434
Ok(())
400435
}
@@ -434,6 +469,8 @@ struct RouteRuntime {
434469
last_packet_number: Option<u32>,
435470
source_connected: bool,
436471
last_send_type: &'static str,
472+
last_recovery_attempt: Option<Instant>,
473+
recovery_hint_printed: bool,
437474
}
438475

439476
impl RouteRuntime {
@@ -449,6 +486,8 @@ impl RouteRuntime {
449486
last_packet_number: None,
450487
source_connected: false,
451488
last_send_type: "heartbeat",
489+
last_recovery_attempt: None,
490+
recovery_hint_printed: false,
452491
}
453492
}
454493

@@ -557,6 +596,64 @@ fn print_status(routes: &mut [RouteRuntime]) {
557596
route.last_state.right_x,
558597
route.last_state.right_y,
559598
);
599+
if route.inbound_total == 0 && route.sent_total > 180 && !route.recovery_hint_printed {
600+
println!(
601+
" hint: no Pico reply yet. Confirm this Pico is powered, on the same Wi-Fi, and visible in `couchlink test discover --all`."
602+
);
603+
route.recovery_hint_printed = true;
604+
} else if route.inbound_total > 0
605+
&& route.last_inbound.elapsed() > PEER_STALE_AFTER
606+
&& !route.recovery_hint_printed
607+
{
608+
println!(
609+
" hint: this Pico stopped replying. CouchLink will try to rediscover it; check power and Wi-Fi if it stays stale."
610+
);
611+
route.recovery_hint_printed = true;
612+
}
613+
}
614+
}
615+
616+
fn schedule_recovery_if_needed(routes: &mut [RouteRuntime]) -> bool {
617+
let now = Instant::now();
618+
let mut needed = false;
619+
for route in routes {
620+
if route.last_inbound.elapsed() <= PEER_STALE_AFTER {
621+
continue;
622+
}
623+
if route
624+
.last_recovery_attempt
625+
.map(|last| now.duration_since(last) < PEER_RECOVER_EVERY)
626+
.unwrap_or(false)
627+
{
628+
continue;
629+
}
630+
route.last_recovery_attempt = Some(now);
631+
needed = true;
632+
}
633+
needed
634+
}
635+
636+
fn apply_recovery_results(routes: &mut [RouteRuntime], picos: &[PicoTarget], quiet: bool) {
637+
for route in routes {
638+
let Some(found) = picos
639+
.iter()
640+
.find(|p| p.info.unique_id_short == route.route.pico.info.unique_id_short)
641+
else {
642+
continue;
643+
};
644+
if found.peer != route.route.pico.peer {
645+
if !quiet {
646+
println!(
647+
"Recovered {}: {} -> {}",
648+
route.route.pico.uid_hex(),
649+
route.route.pico.peer,
650+
found.peer
651+
);
652+
}
653+
route.route.pico = found.clone();
654+
route.last_inbound = Instant::now();
655+
route.recovery_hint_printed = false;
656+
}
560657
}
561658
}
562659

@@ -671,4 +768,20 @@ mod tests {
671768
assert_eq!(routes[1].source_slot, 1);
672769
assert_eq!(routes[1].pico.info.unique_id_short, 0x523861E6);
673770
}
771+
772+
#[test]
773+
fn validate_routes_rejects_same_pico_twice() {
774+
let target = pico(0x07D37EB6, "192.168.50.226", protocol::BOARD_PICO_2_W);
775+
let routes = vec![
776+
StreamRoute {
777+
source_slot: 0,
778+
pico: target.clone(),
779+
},
780+
StreamRoute {
781+
source_slot: 1,
782+
pico: target,
783+
},
784+
];
785+
assert!(validate_routes(&routes).is_err());
786+
}
674787
}

0 commit comments

Comments
 (0)