mirror of
https://github.com/horizon-rust/horizon.git
synced 2026-11-07 11:06:11 -10:00
DualSense: drop the output handle on any hidapi error
hidapi's bundled C can crash inside hid_write on Windows once the pad has gone (Bluetooth sleep or unplug), and flush() only handled a returned error, not a crash inside the call. Treat the output as lossy: - probe the pad with a non-blocking read before every write, so a gone device is noticed before hid_write is entered; - drop the handle on any hidapi failure (read or write), never writing through it again; - schedule a rescan in lost() so the writer goes back through the normal scan/reopen path instead of keeping a stale handle; - watch_animation() now drops the pad on a failed read too. The handle is a small trait so the writer's state machine is unit-tested without hardware. There is no vanilla reference: this is Horizon-only hardware output. Co-Authored-By: opencode (DeepSeek V4.1 Flash) <[email protected]>
This commit is contained in:
1 parent
1989b1b595
commit
38487aed11
1 file changed
+183
-24
@@ -224,7 +224,7 @@ const SCAN_TRIES: u32 = 5;
|
||||
#[cfg(not(target_os = "ios"))]
|
||||
struct Writer {
|
||||
api: Option<HidApi>,
|
||||
pad: Option<DualSense>,
|
||||
pad: Option<Box<dyn Handle>>,
|
||||
open: Arc<AtomicU8>,
|
||||
state: State,
|
||||
/// What the pad was last sent; `None` after (re)opening.
|
||||
@@ -268,6 +268,26 @@ fn bluetooth_clock(report: &[u8]) -> Option<u32> {
|
||||
}
|
||||
}
|
||||
|
||||
/// What the writer needs from an open pad. It is a trait so the state machine can be tested
|
||||
/// without hardware: hidapi's Windows back end can crash inside `hid_write` once the device is
|
||||
/// gone, so every failure has to drop the handle.
|
||||
#[cfg(not(target_os = "ios"))]
|
||||
trait Handle: Send {
|
||||
fn write(&mut self, out: &Output) -> HidResult<()>;
|
||||
fn read(&self, buf: &mut [u8], millis: i32) -> HidResult<usize>;
|
||||
}
|
||||
|
||||
#[cfg(not(target_os = "ios"))]
|
||||
impl Handle for DualSense {
|
||||
fn write(&mut self, out: &Output) -> HidResult<()> {
|
||||
DualSense::write(self, out)
|
||||
}
|
||||
|
||||
fn read(&self, buf: &mut [u8], millis: i32) -> HidResult<usize> {
|
||||
DualSense::read(self, buf, millis)
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(not(target_os = "ios"))]
|
||||
impl Writer {
|
||||
fn new(open: Arc<AtomicU8>) -> Self {
|
||||
@@ -313,13 +333,8 @@ impl Writer {
|
||||
}
|
||||
Ok(Message::Scan) => {
|
||||
// A pad that was unplugged while idle fails to read.
|
||||
let mut buf = [0; 80];
|
||||
if self
|
||||
.pad
|
||||
.as_ref()
|
||||
.is_some_and(|pad| pad.read(&mut buf, 0).is_err())
|
||||
{
|
||||
self.lost("unplugged");
|
||||
if self.pad.is_some() {
|
||||
self.probe();
|
||||
}
|
||||
if self.pad.is_none() {
|
||||
self.scans_left = SCAN_TRIES;
|
||||
@@ -334,14 +349,16 @@ impl Writer {
|
||||
self.advance_rumble();
|
||||
self.flush();
|
||||
}
|
||||
if let Some(pad) = self.pad.as_mut() {
|
||||
if self.pad.is_some() && self.probe() {
|
||||
// The lights go back to a host's defaults, if they were changed.
|
||||
let release = Output {
|
||||
lightbar: self.state.lightbar.map(|_| DEFAULT_LIGHTBAR),
|
||||
player_leds: self.state.player_leds.map(|_| PLAYER_1),
|
||||
..Output::release()
|
||||
};
|
||||
let _ = pad.write(&release);
|
||||
if let Some(pad) = self.pad.as_mut() {
|
||||
let _ = pad.write(&release);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -370,7 +387,7 @@ impl Writer {
|
||||
Lights::Waiting(Instant::now() + Duration::from_secs(1))
|
||||
}
|
||||
};
|
||||
self.pad = Some(pad);
|
||||
self.pad = Some(Box::new(pad));
|
||||
self.sent = None;
|
||||
self.scans_left = 0;
|
||||
self.flush();
|
||||
@@ -391,10 +408,20 @@ impl Writer {
|
||||
};
|
||||
let mut buf = [0; 80];
|
||||
let mut clock = None;
|
||||
while let Ok(n) = pad.read(&mut buf, 0)
|
||||
&& n > 0
|
||||
{
|
||||
clock = bluetooth_clock(&buf[..n]).or(clock);
|
||||
let mut failed = None;
|
||||
loop {
|
||||
match pad.read(&mut buf, 0) {
|
||||
Ok(0) => break,
|
||||
Ok(n) => clock = bluetooth_clock(&buf[..n]).or(clock),
|
||||
Err(e) => {
|
||||
failed = Some(e.to_string());
|
||||
break;
|
||||
}
|
||||
}
|
||||
}
|
||||
if let Some(why) = failed {
|
||||
self.lost(&why);
|
||||
return;
|
||||
}
|
||||
let over = match clock {
|
||||
Some(clock) => clock >= ANIMATION_OVER,
|
||||
@@ -421,9 +448,9 @@ impl Writer {
|
||||
|
||||
/// Send what changed; a failed write means the pad is gone.
|
||||
fn flush(&mut self) {
|
||||
let Some(pad) = self.pad.as_mut() else {
|
||||
if self.pad.is_none() {
|
||||
return;
|
||||
};
|
||||
}
|
||||
let motors = self.rumble.last().map_or([0, 0], |(m, _)| *m);
|
||||
let mut state = self.state;
|
||||
if matches!(self.lights, Lights::Waiting(_)) {
|
||||
@@ -448,10 +475,8 @@ impl Writer {
|
||||
self.lights = Lights::Reset;
|
||||
}
|
||||
// The reset goes alone, before any colour (hid-playstation.c).
|
||||
if first.full_power || first.release_leds {
|
||||
if let Err(e) = pad.write(&first) {
|
||||
return self.lost(&e.to_string());
|
||||
}
|
||||
if (first.full_power || first.release_leds) && !self.write(&first) {
|
||||
return;
|
||||
}
|
||||
let mut out = Output {
|
||||
rumble: motors,
|
||||
@@ -468,22 +493,156 @@ impl Writer {
|
||||
if old.is_none_or(|s| s.player_leds != state.player_leds) {
|
||||
out.player_leds = state.player_leds;
|
||||
}
|
||||
match pad.write(&out) {
|
||||
Ok(()) => self.sent = Some((state, motors)),
|
||||
Err(e) => self.lost(&e.to_string()),
|
||||
if self.write(&out) {
|
||||
self.sent = Some((state, motors));
|
||||
}
|
||||
}
|
||||
|
||||
/// Whether the pad still answers. hidapi's Windows back end can crash inside `hid_write`
|
||||
/// once the device has gone (Bluetooth sleep, unplug), so a write reads first; a failed
|
||||
/// read drops the handle and the thread looks for the pad again.
|
||||
fn probe(&mut self) -> bool {
|
||||
let Some(pad) = self.pad.as_ref() else {
|
||||
return false;
|
||||
};
|
||||
let mut buf = [0; 80];
|
||||
let failed = match pad.read(&mut buf, 0) {
|
||||
Ok(_) => None,
|
||||
Err(e) => Some(e.to_string()),
|
||||
};
|
||||
match failed {
|
||||
Some(why) => {
|
||||
self.lost(&why);
|
||||
false
|
||||
}
|
||||
None => true,
|
||||
}
|
||||
}
|
||||
|
||||
/// Write a report, dropping the pad on any hidapi error: a handle that failed is never
|
||||
/// written through again.
|
||||
fn write(&mut self, out: &Output) -> bool {
|
||||
if !self.probe() {
|
||||
return false;
|
||||
}
|
||||
let result = match self.pad.as_mut() {
|
||||
Some(pad) => pad.write(out),
|
||||
None => return false,
|
||||
};
|
||||
match result {
|
||||
Ok(()) => true,
|
||||
Err(e) => {
|
||||
self.lost(&e.to_string());
|
||||
false
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Drop a handle that hidapi failed on, and look for the pad again: one that slept or was
|
||||
/// unplugged may come back. Nothing is written through the old handle.
|
||||
fn lost(&mut self, why: &str) {
|
||||
log::info!("DualSense output closed: {why}");
|
||||
self.pad = None;
|
||||
self.sent = None;
|
||||
self.lights = Lights::Ready;
|
||||
self.open.store(0, Ordering::Relaxed);
|
||||
self.scans_left = SCAN_TRIES;
|
||||
self.next_scan = Instant::now();
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(all(test, not(target_os = "ios")))]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use std::sync::atomic::AtomicUsize;
|
||||
|
||||
/// A pad whose reads and writes can fail, so the writer's state machine runs without
|
||||
/// hardware (the crash itself is inside hidapi's C on Windows).
|
||||
struct FakePad {
|
||||
writes: Arc<AtomicUsize>,
|
||||
reads: Arc<AtomicUsize>,
|
||||
read_fails: bool,
|
||||
write_fails: bool,
|
||||
}
|
||||
|
||||
impl Handle for FakePad {
|
||||
fn write(&mut self, _out: &Output) -> HidResult<()> {
|
||||
self.writes.fetch_add(1, Ordering::Relaxed);
|
||||
if self.write_fails {
|
||||
Err(hidapi::HidError::HidApiError {
|
||||
message: "gone".into(),
|
||||
})
|
||||
} else {
|
||||
Ok(())
|
||||
}
|
||||
}
|
||||
|
||||
fn read(&self, _buf: &mut [u8], _millis: i32) -> HidResult<usize> {
|
||||
self.reads.fetch_add(1, Ordering::Relaxed);
|
||||
if self.read_fails {
|
||||
Err(hidapi::HidError::HidApiError {
|
||||
message: "gone".into(),
|
||||
})
|
||||
} else {
|
||||
Ok(0)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn writer(
|
||||
open: &Arc<AtomicU8>,
|
||||
read_fails: bool,
|
||||
write_fails: bool,
|
||||
) -> (Writer, Arc<AtomicUsize>, Arc<AtomicUsize>) {
|
||||
let writes = Arc::new(AtomicUsize::new(0));
|
||||
let reads = Arc::new(AtomicUsize::new(0));
|
||||
let mut writer = Writer::new(open.clone());
|
||||
writer.pad = Some(Box::new(FakePad {
|
||||
writes: writes.clone(),
|
||||
reads: reads.clone(),
|
||||
read_fails,
|
||||
write_fails,
|
||||
}));
|
||||
(writer, writes, reads)
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_failed_read_drops_the_pad_without_writing() {
|
||||
let open = Arc::new(AtomicU8::new(1));
|
||||
let (mut writer, writes, reads) = writer(&open, true, false);
|
||||
writer.flush();
|
||||
assert_eq!(reads.load(Ordering::Relaxed), 1);
|
||||
assert_eq!(writes.load(Ordering::Relaxed), 0);
|
||||
assert!(writer.pad.is_none());
|
||||
assert_eq!(open.load(Ordering::Relaxed), 0);
|
||||
assert_eq!(writer.scans_left, SCAN_TRIES);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_failed_write_drops_the_pad_and_schedules_a_rescan() {
|
||||
let open = Arc::new(AtomicU8::new(1));
|
||||
let (mut writer, writes, _) = writer(&open, false, true);
|
||||
writer.flush();
|
||||
assert_eq!(writes.load(Ordering::Relaxed), 1);
|
||||
assert!(writer.pad.is_none());
|
||||
assert_eq!(open.load(Ordering::Relaxed), 0);
|
||||
assert!(writer.sent.is_none());
|
||||
assert_eq!(writer.scans_left, SCAN_TRIES);
|
||||
// The handle that failed is not written through again.
|
||||
writer.flush();
|
||||
assert_eq!(writes.load(Ordering::Relaxed), 1);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_live_pad_stays_open() {
|
||||
let open = Arc::new(AtomicU8::new(1));
|
||||
let (mut writer, writes, _) = writer(&open, false, false);
|
||||
writer.flush();
|
||||
assert_eq!(writes.load(Ordering::Relaxed), 2);
|
||||
assert!(writer.pad.is_some());
|
||||
assert!(writer.sent.is_some());
|
||||
assert_eq!(open.load(Ordering::Relaxed), 1);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn bluetooth_clock_offset() {
|
||||
|
||||
Reference in new issue
Block a user