From 6c73670f4e925c824e9c2c8a6631e65a436022df Mon Sep 17 00:00:00 2001 From: Artem Egorkine Date: Thu, 21 Nov 2024 18:35:41 +0200 Subject: [PATCH] USB: remove hotplug usage Since opening devices has been decoupled from hotplug "device added" events in 37ace4dd432ff91ff41c40aa0298077b329be691, hotplug in itself does not benefit pod-ui. Instead, as libusb doesn't support hotplug on Windows, we'd need to make some ad-hoc hotplug implementataion to work around it. Intead, we remove hotplug altogether, replying purely on `ctx.devices()` listing during USB enumeration. --- gui/src/usb.rs | 4 - usb/src/event.rs | 22 ----- usb/src/lib.rs | 227 +++++++++++------------------------------------ usb/src/usb.rs | 46 +++++++--- 4 files changed, 86 insertions(+), 213 deletions(-) delete mode 100644 usb/src/event.rs diff --git a/gui/src/usb.rs b/gui/src/usb.rs index 2842d7f..c059d69 100644 --- a/gui/src/usb.rs +++ b/gui/src/usb.rs @@ -9,7 +9,6 @@ pub use nop::*; mod imp { use anyhow::*; use core::result::Result::Ok; - use futures::executor; use log::warn; use pod_core::midi::Channel; use pod_core::midi_io; @@ -17,9 +16,6 @@ mod imp { pub fn start_usb() { pod_usb::usb_start().unwrap(); - executor::block_on( - pod_usb::usb_init_wait() - ); } pub fn usb_list_devices() -> Vec<(String, bool)> { diff --git a/usb/src/event.rs b/usb/src/event.rs deleted file mode 100644 index 41ae680..0000000 --- a/usb/src/event.rs +++ /dev/null @@ -1,22 +0,0 @@ -#[derive(Clone, Debug)] -pub struct DeviceAddedEvent { - pub vid: u16, - pub pid: u16, - pub bus: u8, - pub address: u8, -} - -#[derive(Clone, Debug)] -pub struct DeviceRemovedEvent { - pub vid: u16, - pub pid: u16, - pub bus: u8, - pub address: u8, -} - -#[derive(Clone, Debug)] -pub enum UsbEvent { - DeviceAdded(DeviceAddedEvent), - DeviceRemoved(DeviceRemovedEvent), - InitDone -} \ No newline at end of file diff --git a/usb/src/lib.rs b/usb/src/lib.rs index 920948c..14006c6 100644 --- a/usb/src/lib.rs +++ b/usb/src/lib.rs @@ -1,4 +1,3 @@ -mod event; mod devices; mod line6; mod dev_handler; @@ -9,98 +8,29 @@ mod midi_framer; mod podxt_framer; mod framer; -use log::{debug, error, info, trace}; +use log::{error, info}; use anyhow::*; -use anyhow::Context as _; use core::result::Result::Ok; -use std::collections::HashMap; +use std::collections::{HashMap, HashSet}; use std::str::FromStr; use std::sync::{Arc, Mutex, OnceLock}; -use std::sync::atomic::{AtomicBool, Ordering}; use once_cell::sync::Lazy; -use rusb::{Context, Device as UsbDevice, Hotplug, UsbContext}; -use tokio::sync::{broadcast, Notify}; -use tokio::sync::broadcast::error::RecvError; use pod_core::midi_io::{MidiIn, MidiOut}; use regex::Regex; use crate::dev_handler::Device; use crate::devices::find_device; -use crate::event::*; use crate::usb::Usb; use crate::util::usb_address_string; -struct HotplugHandler { - event_tx: broadcast::Sender, - init_devices: Option -} - -impl HotplugHandler { - /// Notify the hotplug handler that `num` devices have been initialized. - /// This is used for `UsbEvent::InitDone` event tracking. - fn device_init_notify(&mut self, added: isize) { - if let Some(mut num) = self.init_devices.take() { - num -= added; - self.init_devices = if num > 1 { - Some(num) - } else { - self.event_tx.send(UsbEvent::InitDone).unwrap(); - None - }; - } - } -} - -impl Hotplug for HotplugHandler { - fn device_arrived(&mut self, device: UsbDevice) { - let Ok(desc) = device.device_descriptor() else { return }; - - trace!("device arrived: {:?}", device); - if find_device(desc.vendor_id(), desc.product_id()).is_some() { - trace!("device added: {:?}", device); - let e = DeviceAddedEvent { - vid: desc.vendor_id(), - pid: desc.product_id(), - bus: device.bus_number(), - address: device.address(), - }; - self.event_tx.send(UsbEvent::DeviceAdded(e)).unwrap(); - } - - self.device_init_notify(1); - } - - fn device_left(&mut self, device: UsbDevice) { - let Ok(desc) = device.device_descriptor() else { return }; - - trace!("device left: {:?}", device); - if find_device(desc.vendor_id(), desc.product_id()).is_some() { - trace!("device removed: {:?}", device); - let e = DeviceRemovedEvent { - vid: desc.vendor_id(), - pid: desc.product_id(), - bus: device.bus_number(), - address: device.address(), - }; - self.event_tx.send(UsbEvent::DeviceRemoved(e)).unwrap(); - } - } -} - -enum UsbFoundDevice { - Found(DeviceAddedEvent), - Open(Device) -} enum UsbEnumeratedDevice { Device(Device), Error(String) } -static mut INIT_DONE: AtomicBool = AtomicBool::new(false); -static INIT_DONE_NOTIFY: Lazy> = Lazy::new(|| { - Arc::new(Notify::new()) -}); -static DEVICES: Lazy>>> = Lazy::new(|| { +type DeviceMap = HashMap; + +static DEVICES: Lazy>> = Lazy::new(|| { Arc::new(Mutex::new(HashMap::new())) }); @@ -112,81 +42,13 @@ pub fn usb_start() -> Result<()> { v.major(), v.minor(), v.micro(), v.nano(), v.rc().unwrap_or("") ); - if !rusb::has_hotplug() { - bail!("Libusb hotplug API not supported"); - } - - let (event_tx, mut event_rx) = broadcast::channel::(512); - - let ctx = Context::new()?; - let num_devices = ctx.devices()?.len() as isize; - let mut hh = HotplugHandler { - event_tx: event_tx.clone(), - init_devices: Some(num_devices) - }; - hh.device_init_notify(0); - - let usb = Usb::new(Box::new(hh))?; + let usb = Usb::new()?; USB.set(Arc::new(Mutex::new(usb))).map_err(|_| anyhow!("Failed to set global USB var"))?; - tokio::spawn(async move { - info!("USB event RX thread start"); - loop { - let msg = match event_rx.recv().await { - Ok(msg) => { msg } - Err(RecvError::Closed) => { - info!("Event bus closed"); - break; - } - Err(RecvError::Lagged(n)) => { - error!("Event bus lagged: {}", n); - continue; - } - }; - - match msg { - UsbEvent::DeviceAdded(e @ DeviceAddedEvent{ bus, address, .. }) => { - let address = usb_address_string(bus, address); - usb_add_device(address, e); - } - UsbEvent::DeviceRemoved(DeviceRemovedEvent{ bus, address, .. }) => { - let address = usb_address_string(bus, address); - usb_remove_device(address); - - } - UsbEvent::InitDone => { - usb_init_set_done(); - } - } - } - info!("USB event RX thread finish"); - }); - Ok(()) } -fn usb_init_set_done() { - unsafe { INIT_DONE.store(true, Ordering::Relaxed) } - - debug!("USB init done"); - INIT_DONE_NOTIFY.notify_waiters() -} - -fn usb_init_done() -> bool { - unsafe { INIT_DONE.load(Ordering::Relaxed) } -} - -pub async fn usb_init_wait() { - if usb_init_done() { - return; - } - - debug!("Waiting for USB init..."); - INIT_DONE_NOTIFY.notified().await; - debug!("Waiting for USB init over"); -} - -fn usb_enumerate_devices(devices: &mut HashMap) -> Vec { +fn usb_enumerate_devices(devices: &mut DeviceMap) -> Vec { let Some(usb) = USB.get() else { error!("Cannot enumerate USB: usb not ready!"); return vec![]; @@ -194,13 +56,29 @@ fn usb_enumerate_devices(devices: &mut HashMap) -> Vec { - let usb_dev = find_device(vid, pid).unwrap(); - let h = match usb.open(vid, pid, bus, address) { + + let listed_devices = match usb.list_devices() { + Ok(devices) => { devices } + Err(err) => { + error!("Failed to list USB devices: {err}"); + return vec![]; + } + }; + + let mut add = HashMap::new(); + let mut enumerated = Vec::with_capacity(listed_devices.len()); + let mut listed_keys = HashSet::new(); + for dev in listed_devices { + let key = usb_address_string(dev.bus, dev.address); + match devices.get(&key) { + Some(dev) => { + // device already open + enumerated.push(UsbEnumeratedDevice::Device(dev.clone())) + } + None => { + // attempt to open a new device + let usb_dev = find_device(dev.vid, dev.pid).unwrap(); + let h = match usb.open(dev.vid, dev.pid, dev.bus, dev.address) { Ok(h) => { h } Err(e) => { error!("Failed to open device: {}", e); @@ -217,19 +95,28 @@ fn usb_enumerate_devices(devices: &mut HashMap) -> Vec { - enumerated.push(UsbEnumeratedDevice::Device(dev.clone())) - } } + listed_keys.insert(key); + } - let updated = update.len(); - for (key, value) in update { + + let added = add.len(); + for (key, value) in add { devices.insert(key, value); } - info!("Enumerating USB devices finished: {updated} entries updated"); + let keys_to_remove = devices.keys() + .filter(|key| !listed_keys.contains(*key)) + .cloned() + .collect::>(); + let removed = keys_to_remove.len(); + for key in keys_to_remove { + devices.remove(&key); + } + + info!("Enumerating USB devices finished: +{added}/-{removed} entries"); enumerated } @@ -243,11 +130,6 @@ pub fn usb_list_devices() -> Vec<(String, bool)> { }).collect() } -fn usb_add_device(key: String, event: DeviceAddedEvent) { - let mut devices = DEVICES.lock().unwrap(); - devices.insert(key, UsbFoundDevice::Found(event)); -} - fn usb_remove_device(key: String) { let mut devices = DEVICES.lock().unwrap(); devices.remove(&key); @@ -272,10 +154,7 @@ pub fn usb_device_for_address(dev_addr: &str) -> Result<(impl MidiIn, impl MidiO } match found { - Some(UsbFoundDevice::Open(dev)) => { dev.open() } - Some(_) => { - bail!("USB device for address {:?} found, but couldn't be opened", dev_addr); - } + Some(dev) => { dev.open() } None => { bail!("USB device for address {:?} not found!", dev_addr); } @@ -286,16 +165,10 @@ pub fn usb_device_for_name(dev_name: &str) -> Result<(impl MidiIn, impl MidiOut) let mut devices = DEVICES.lock().unwrap(); let _ = usb_enumerate_devices(&mut devices); - let found = devices.values_mut().find(|dev| - match dev { - UsbFoundDevice::Open(dev) if dev.name == dev_name => { true } - _ => { false } - } - ); - + let found = devices.values_mut().find(|dev| dev.name == dev_name); match found { - Some(UsbFoundDevice::Open(dev)) => { dev.open() } - _ => { + Some(dev) => { dev.open() } + None => { bail!("USB device for name {:?} not found!", dev_name); } } diff --git a/usb/src/usb.rs b/usb/src/usb.rs index a498c0a..98ba736 100644 --- a/usb/src/usb.rs +++ b/usb/src/usb.rs @@ -8,15 +8,24 @@ use std::sync::atomic::{AtomicBool, Ordering}; use std::thread; use std::time::Duration; use log::{debug, error, info}; -use rusb::{Context, Hotplug, HotplugBuilder, Registration, UsbContext}; +use rusb::{Context, Hotplug, UsbContext}; use rusb::constants::{LIBUSB_ENDPOINT_DIR_MASK, LIBUSB_ENDPOINT_IN, LIBUSB_ENDPOINT_OUT, LIBUSB_TRANSFER_TYPE_BULK}; use rusb::ffi::{libusb_alloc_transfer, libusb_cancel_transfer, libusb_free_transfer, libusb_submit_transfer, libusb_transfer}; use crate::check; +use crate::devices::find_device; use crate::util::usb_address_string; pub type Device = rusb::Device; pub type DeviceHandle = rusb::DeviceHandle; +#[derive(Clone, Debug)] +pub struct ListedDevice { + pub vid: u16, + pub pid: u16, + pub bus: u8, + pub address: u8, +} + pub struct Usb { ctx: Context, running: Arc, @@ -24,21 +33,40 @@ pub struct Usb { } impl Usb { - pub fn new(hotplug: Box>) -> Result { + pub fn new() -> Result { let ctx = libusb::new_ctx(rusb::constants::LIBUSB_LOG_LEVEL_INFO)?; let running = Arc::new(AtomicBool::new(true)); - let hotplug= HotplugBuilder::new() - .enumerate(true) - .register(&ctx, hotplug)?; - let thread = { let (ctx, run) = (ctx.clone(), Arc::clone(&running)); - Some(thread::spawn(move || Self::event_thread(ctx, run, hotplug))) + Some(thread::spawn(move || Self::event_thread(ctx, run))) }; Ok(Self { ctx, running, thread }) } + pub fn list_devices(&self) -> Result> { + let devices = self.ctx.devices()?; + let devices = devices.iter() + .flat_map(|dev| { + let Ok(desc) = dev.device_descriptor() else { + return None; + }; + let vid = desc.vendor_id(); + let pid = desc.product_id(); + if find_device(vid, pid).is_some() { + let bus = dev.bus_number(); + let address = dev.address(); + + Some(ListedDevice { vid, pid, bus, address }) + } else { + None + } + }) + .collect::>(); + + Ok(devices) + } + pub fn close(&mut self) { self.running.store(false, Ordering::Release); self.ctx.interrupt_handle_events(); @@ -60,7 +88,7 @@ impl Usb { } /// Dedicated thread for async transfer and hotplug events. - fn event_thread(ctx: Context, run: Arc, hotplug: Registration) { + fn event_thread(ctx: Context, run: Arc) { debug!("USB event thread start"); while run.load(Ordering::Acquire) { if let Err(e) = ctx.handle_events(None) { @@ -69,8 +97,6 @@ impl Usb { break; } } - - ctx.unregister_callback(hotplug); debug!("USB event thread finish"); } }