diff --git a/hid-service/src/i2c/device.rs b/hid-service/src/i2c/device.rs index 9d8940e2..b64e9203 100644 --- a/hid-service/src/i2c/device.rs +++ b/hid-service/src/i2c/device.rs @@ -1,6 +1,7 @@ use core::borrow::BorrowMut; use embassy_sync::mutex::Mutex; +use embassy_time::{Duration, with_timeout}; use embedded_hal_async::i2c::{AddressMode, I2c}; use embedded_services::hid::{DeviceContainer, InvalidSizeError, Opcode, Response}; use embedded_services::{GlobalRawMutex, buffer::*}; @@ -8,22 +9,48 @@ use embedded_services::{error, hid, info, trace}; use crate::Error; +/// Timeout configuration for I2C HID device operations. +pub struct Config { + /// Timeout for descriptor reads and commands. + pub device_response_timeout: Duration, + /// Timeout for input reports and feature data reads. + pub data_read_timeout: Duration, +} + +impl Default for Config { + fn default() -> Self { + Self { + device_response_timeout: Duration::from_millis(200), + data_read_timeout: Duration::from_millis(50), + } + } +} + pub struct Device> { device: hid::Device, buffer: OwnedRef<'static, u8>, address: A, descriptor: Mutex>, bus: Mutex, + timeout_config: Config, } impl> Device { - pub fn new(id: hid::DeviceId, address: A, bus: B, regs: hid::RegisterFile, buffer: OwnedRef<'static, u8>) -> Self { + pub fn new( + id: hid::DeviceId, + address: A, + bus: B, + regs: hid::RegisterFile, + buffer: OwnedRef<'static, u8>, + timeout_config: Config, + ) -> Self { Self { device: hid::Device::new(id, regs), buffer, address, descriptor: Mutex::new(None), bus: Mutex::new(bus), + timeout_config, } } @@ -47,10 +74,19 @@ impl> Device { })))?; reg.copy_from_slice(&self.device.regs.hid_desc_reg.to_le_bytes()); - if let Err(e) = bus.write_read(self.address, ®, buf).await { + with_timeout( + self.timeout_config.device_response_timeout, + bus.write_read(self.address, ®, buf), + ) + .await + .map_err(|_| { + error!("Read HID descriptor timeout"); + Error::Hid(hid::Error::Timeout) + })? + .map_err(|e| { error!("Failed to read HID descriptor"); - return Err(Error::Bus(e)); - } + Error::Bus(e) + })?; let res = hid::Descriptor::decode_from_slice(buf); match res { @@ -89,8 +125,9 @@ impl> Device { let len = desc.w_report_desc_length as usize; let mut bus = self.bus.lock().await; - if let Err(e) = bus - .write_read( + with_timeout( + self.timeout_config.device_response_timeout, + bus.write_read( self.address, ®, buf.get_mut(0..len) @@ -98,12 +135,17 @@ impl> Device { expected: len, actual: buffer_len, })))?, - ) - .await - { + ), + ) + .await + .map_err(|_| { + error!("Read report descriptor timeout"); + Error::Hid(hid::Error::Timeout) + })? + .map_err(|e| { error!("Failed to read report descriptor"); - return Err(Error::Bus(e)); - } + Error::Bus(e) + })?; self.buffer.reference().slice(0..len).map_err(Error::Buffer) } @@ -123,10 +165,16 @@ impl> Device { })))?; let mut bus = self.bus.lock().await; - if let Err(e) = bus.read(self.address, buf).await { - error!("Failed to read input report"); - return Err(Error::Bus(e)); - } + with_timeout(self.timeout_config.data_read_timeout, bus.read(self.address, buf)) + .await + .map_err(|_| { + error!("Read input report timeout"); + Error::Hid(hid::Error::Timeout) + })? + .map_err(|e| { + error!("Failed to read input report"); + Error::Bus(e) + })?; self.buffer .reference() @@ -164,27 +212,39 @@ impl> Device { })?; let mut bus = self.bus.lock().await; - if let Err(e) = bus - .write( + with_timeout( + self.timeout_config.device_response_timeout, + bus.write( self.address, buf.get(..len) .ok_or(Error::Hid(hid::Error::InvalidSize(InvalidSizeError { expected: len, actual: buffer_len, })))?, - ) - .await - { + ), + ) + .await + .map_err(|_| { + error!("Write command timeout"); + Error::Hid(hid::Error::Timeout) + })? + .map_err(|e| { error!("Failed to write command"); - return Err(Error::Bus(e)); - } + Error::Bus(e) + })?; if opcode.has_response() { trace!("Reading host data"); - if let Err(e) = bus.read(self.address, buf).await { - error!("Failed to read host data"); - return Err(Error::Bus(e)); - } + with_timeout(self.timeout_config.data_read_timeout, bus.read(self.address, buf)) + .await + .map_err(|_| { + error!("Read host data timeout"); + Error::Hid(hid::Error::Timeout) + })? + .map_err(|e| { + error!("Failed to read host data"); + Error::Bus(e) + })?; return Ok(Some(Response::FeatureReport(self.buffer.reference()))); } diff --git a/hid-service/src/i2c/host.rs b/hid-service/src/i2c/host.rs index 9de78e26..97380043 100644 --- a/hid-service/src/i2c/host.rs +++ b/hid-service/src/i2c/host.rs @@ -13,8 +13,22 @@ use embedded_services::{error, trace}; use super::{Command as I2cCommand, I2cSlaveAsync}; use crate::Error; -const DEVICE_RESPONSE_TIMEOUT_MS: u64 = 200; -const DATA_READ_TIMEOUT_MS: u64 = 50; +/// Timeout configuration for I2C HID host operations. +pub struct Config { + /// Timeout for device response reads. + pub device_response_timeout: Duration, + /// Timeout for data reads from the host. + pub data_read_timeout: Duration, +} + +impl Default for Config { + fn default() -> Self { + Self { + device_response_timeout: Duration::from_millis(200), + data_read_timeout: Duration::from_millis(50), + } + } +} #[derive(Copy, Clone, PartialEq, Eq)] pub enum Access { @@ -28,22 +42,24 @@ pub struct Host { response: Signal>>, buffer: OwnedRef<'static, u8>, bus: Mutex, + timeout_config: Config, } impl Host { - pub fn new(id: DeviceId, bus: B, buffer: OwnedRef<'static, u8>) -> Self { + pub fn new(id: DeviceId, bus: B, buffer: OwnedRef<'static, u8>, timeout_config: Config) -> Self { Host { id, tp: Endpoint::uninit(EndpointID::External(External::Host)), response: Signal::new(), buffer, bus: Mutex::new(bus), + timeout_config, } } - async fn read_bus(&self, timeout_ms: u64, buffer: &mut [u8]) -> Result<(), Error> { + async fn read_bus(&self, timeout: Duration, buffer: &mut [u8]) -> Result<(), Error> { let mut bus = self.bus.lock().await; - with_timeout(Duration::from_millis(timeout_ms), bus.respond_to_write(buffer)) + with_timeout(timeout, bus.respond_to_write(buffer)) .await .map_err(|_| { error!("Response timeout"); @@ -55,11 +71,11 @@ impl Host { }) } - async fn write_bus(&self, timeout_ms: u64, buffer: &[u8]) -> Result<(), Error> { + async fn write_bus(&self, timeout: Duration, buffer: &[u8]) -> Result<(), Error> { let mut bus = self.bus.lock().await; // Send response, timeout if the host doesn't read so we don't get stuck here trace!("Sending {} bytes", buffer.len()); - with_timeout(Duration::from_millis(timeout_ms), bus.respond_to_read(buffer)) + with_timeout(timeout, bus.respond_to_read(buffer)) .await .map_err(|_| { error!("Response timeout"); @@ -77,7 +93,7 @@ impl Host { let buffer_len = buffer.len(); self.read_bus( - DATA_READ_TIMEOUT_MS, + self.timeout_config.data_read_timeout, buffer .get_mut(..2) .ok_or(Error::Hid(hid::Error::InvalidSize(InvalidSizeError { @@ -95,7 +111,7 @@ impl Host { )?); trace!("Reading {} bytes", length); self.read_bus( - DATA_READ_TIMEOUT_MS, + self.timeout_config.data_read_timeout, buffer .get_mut(2..length as usize) .ok_or(Error::Hid(hid::Error::InvalidSize(InvalidSizeError { @@ -121,7 +137,7 @@ impl Host { async fn process_command(&self, device: &hid::Device) -> Result, Error> { trace!("Waiting for command"); let mut cmd = [0u8; 2]; - self.read_bus(DATA_READ_TIMEOUT_MS, &mut cmd).await?; + self.read_bus(self.timeout_config.data_read_timeout, &mut cmd).await?; let cmd = u16::from_le_bytes(cmd); let opcode = Opcode::try_from(cmd).map_err(|e| { @@ -137,7 +153,8 @@ impl Host { if hid::ReportId::has_extended_report_id(cmd) { trace!("Reading extended report ID"); let mut report_id = [0u8; 1]; - self.read_bus(DATA_READ_TIMEOUT_MS, &mut report_id).await?; + self.read_bus(self.timeout_config.data_read_timeout, &mut report_id) + .await?; Some(hid::ReportId(report_id[0])) } else { @@ -152,7 +169,7 @@ impl Host { let mut addr = [0u8; 2]; // If the command has a response then we only needed to consume the data register address trace!("Waiting for host data access"); - self.read_bus(DATA_READ_TIMEOUT_MS, &mut addr).await?; + self.read_bus(self.timeout_config.data_read_timeout, &mut addr).await?; let reg = u16::from_le_bytes(addr); if reg != device.regs.data_reg { @@ -167,7 +184,7 @@ impl Host { let buffer_len = buffer.len(); self.read_bus( - DATA_READ_TIMEOUT_MS, + self.timeout_config.data_read_timeout, buffer .get_mut(0..2) .ok_or(Error::Hid(hid::Error::InvalidSize(InvalidSizeError { @@ -186,7 +203,7 @@ impl Host { trace!("Reading {} bytes", length); self.read_bus( - DATA_READ_TIMEOUT_MS, + self.timeout_config.data_read_timeout, buffer .get_mut(2..length as usize) .ok_or(Error::Hid(hid::Error::InvalidSize(InvalidSizeError { @@ -224,7 +241,7 @@ impl Host { async fn process_register_access(&self) -> Result<(), Error> { let mut reg = [0u8; 2]; trace!("Waiting for register address"); - self.read_bus(DATA_READ_TIMEOUT_MS, &mut reg).await?; + self.read_bus(self.timeout_config.data_read_timeout, &mut reg).await?; let reg = u16::from_le_bytes(reg); trace!("Register address {:#x}", reg); @@ -324,20 +341,23 @@ impl Host { | hid::Response::InputReport(data) | hid::Response::FeatureReport(data) => { let bytes = data.borrow().map_err(Error::Buffer)?; - self.write_bus(DEVICE_RESPONSE_TIMEOUT_MS, bytes.borrow()).await + self.write_bus(self.timeout_config.device_response_timeout, bytes.borrow()) + .await } hid::Response::Command(cmd) => match cmd { hid::CommandResponse::GetIdle(freq) => { let freq: u16 = freq.into(); let mut buffer = [0u8; 2]; buffer.copy_from_slice(freq.to_le_bytes().as_slice()); - self.write_bus(DEVICE_RESPONSE_TIMEOUT_MS, &buffer).await + self.write_bus(self.timeout_config.device_response_timeout, &buffer) + .await } hid::CommandResponse::GetProtocol(protocol) => { let protocol: u16 = protocol.into(); let mut buffer = [0u8; 2]; buffer.copy_from_slice(protocol.to_le_bytes().as_slice()); - self.write_bus(DEVICE_RESPONSE_TIMEOUT_MS, &buffer).await + self.write_bus(self.timeout_config.device_response_timeout, &buffer) + .await } hid::CommandResponse::Vendor => Ok(()), }, diff --git a/hid-service/src/i2c/mod.rs b/hid-service/src/i2c/mod.rs index 49889796..747dffa0 100644 --- a/hid-service/src/i2c/mod.rs +++ b/hid-service/src/i2c/mod.rs @@ -2,8 +2,8 @@ mod device; mod host; pub mod passthrough; -pub use device::*; -pub use host::*; +pub use device::{Config as DeviceConfig, Device}; +pub use host::{Access, Config as HostConfig, Host}; #[derive(Clone, Copy, Debug, PartialEq, Eq)] #[cfg_attr(feature = "defmt", derive(defmt::Format))] diff --git a/hid-service/src/i2c/passthrough/mod.rs b/hid-service/src/i2c/passthrough/mod.rs index 0a2a7009..c6895bcb 100644 --- a/hid-service/src/i2c/passthrough/mod.rs +++ b/hid-service/src/i2c/passthrough/mod.rs @@ -8,13 +8,22 @@ macro_rules! define_i2c_passthrough_device_task { pub async fn device_task(bus: $bus, id: ::embedded_services::hid::DeviceId, addr: u8) { use ::embassy_sync::once_lock::OnceLock; use ::embedded_services::{define_static_buffer, error, hid, info}; - use $crate::i2c::Device; + use $crate::i2c::{Device, DeviceConfig}; define_static_buffer!(gen_buffer, u8, [0; 512]); let gen_buffer = gen_buffer::get_mut().unwrap(); info!("Create HID passthrough device {}", id.0); static DEVICE: OnceLock> = OnceLock::new(); - let device = DEVICE.get_or_init(|| Device::new(id, addr, bus, Default::default(), gen_buffer)); + let device = DEVICE.get_or_init(|| { + Device::new( + id, + addr, + bus, + Default::default(), + gen_buffer, + DeviceConfig::default(), + ) + }); hid::register_device(device).await.unwrap(); info!("Starting device task"); @@ -37,12 +46,19 @@ macro_rules! define_i2c_passthrough_host_task { ) { use ::embassy_sync::once_lock::OnceLock; use ::embedded_services::{comms, define_static_buffer, error, info}; - use $crate::i2c::Host; + use $crate::i2c::{Host, HostConfig}; info!("Creating HIDI2C Host"); define_static_buffer!(host_buffer, u8, [0; 128]); static HOST: OnceLock> = OnceLock::new(); - let host = HOST.get_or_init(|| Host::new(HID_ID0, bus, host_buffer::get_mut().unwrap())); + let host = HOST.get_or_init(|| { + Host::new( + HID_ID0, + bus, + host_buffer::get_mut().unwrap(), + HostConfig::default(), + ) + }); comms::register_endpoint(host, &host.tp).await.unwrap(); loop { diff --git a/keyboard-service/src/task.rs b/keyboard-service/src/task.rs index 658e78c3..6d73fd86 100644 --- a/keyboard-service/src/task.rs +++ b/keyboard-service/src/task.rs @@ -29,7 +29,12 @@ macro_rules! impl_host_request_task { // In this macro since static items cannot be generic either static HOST: ::static_cell::StaticCell> = ::static_cell::StaticCell::new(); - let host = hid_service::i2c::Host::new(keyboard_service::HID_KB_ID, kb_i2c, buf); + let host = hid_service::i2c::Host::new( + keyboard_service::HID_KB_ID, + kb_i2c, + buf, + hid_service::i2c::HostConfig::default(), + ); let host = HOST.init(host); keyboard_service::hid_kb::handle_host_requests(host).await;