diff --git a/src/hid/ch9329.rs b/src/hid/ch9329.rs index 4a1a2e18..49fe730e 100644 --- a/src/hid/ch9329.rs +++ b/src/hid/ch9329.rs @@ -44,7 +44,10 @@ const PARAM_CFG_VID_PID_OFFSET: usize = 11; const PARAM_CFG_STRING_FLAGS_OFFSET: usize = 36; const DESCRIPTOR_READ_RETRIES: usize = 3; const DESCRIPTOR_RETRY_DELAY_MS: u64 = 80; -const DESCRIPTOR_APPLY_RESET_WAIT_MS: u64 = 3000; + +// CH9329/CH9329F can take several seconds to restart after a descriptor update. +const DESCRIPTOR_APPLY_RESET_WAIT_MS: u64 = 5000; + const USB_STRING_MAX_LEN: usize = 23; const USB_STRING_FLAG_ENABLE: u8 = 0x80; const USB_STRING_FLAG_MANUFACTURER: u8 = 0x04; @@ -368,33 +371,41 @@ impl Ch9329Backend { Self::write_packet(port, address, cmd, data)?; - let mut pending = Vec::with_capacity(128); + // Keep enough room for a full parameter response and adjacent packets. + let mut pending = Vec::with_capacity(256); let deadline = Instant::now() + Duration::from_millis(RESPONSE_TIMEOUT_MS); let expected_ok = expected_response_cmd(cmd, false); let expected_err = expected_response_cmd(cmd, true); loop { - let mut chunk = [0u8; 128]; + let mut chunk = [0u8; 256]; match port.read(&mut chunk) { Ok(n) if n > 0 => { pending.extend_from_slice(&chunk[..n]); + // Drain every complete frame so adjacent/out-of-order responses + // cannot block the response for the current command. while let Some((response, consumed)) = try_extract_response(&pending) { + let current_response_cmd = response.cmd; pending.drain(..consumed); - if response.cmd == expected_ok || response.cmd == expected_err { + + if current_response_cmd == expected_ok + || current_response_cmd == expected_err + { return Ok(response); } trace!( - "CH9329 ignored out-of-order response: expected 0x{:02X}/0x{:02X}, got 0x{:02X}", + "CH9329 filtered an overlapping packet: expected 0x{:02X}/0x{:02X}, bypass 0x{:02X}", expected_ok, expected_err, - response.cmd + current_response_cmd ); } + // Bound memory use if a noisy or disconnected port keeps delivering bytes. if pending.len() > MAX_PACKET_SIZE * 4 { - let keep = MAX_PACKET_SIZE; + let keep = MAX_PACKET_SIZE * 2; pending.drain(..pending.len().saturating_sub(keep)); } } @@ -410,15 +421,19 @@ impl Ch9329Backend { if Instant::now() >= deadline { return Err(Self::backend_error( - format!("No matching response from CH9329 for cmd 0x{:02X}", cmd), + format!( + "No matching response from CH9329 for cmd 0x{:02X}. Remaining buffer: {}", + cmd, + Self::hex_bytes(&pending) + ), "no_response", )); } - thread::sleep(Duration::from_millis(1)); + // Give the serial driver a short opportunity to deliver the next chunk. + thread::sleep(Duration::from_micros(200)); } } - fn try_best_effort_reset(port: &mut dyn serialport::SerialPort, address: u8) { if let Err(err) = Self::write_packet(port, address, cmd::RESET, &[]) { trace!("CH9329 best-effort reset failed: {}", err); @@ -698,7 +713,6 @@ impl Ch9329Backend { let mut port = Self::open_port(port_path, baud_rate)?; Self::read_device_descriptor_on_port(port.as_mut(), DEFAULT_ADDR) } - fn open_ready_port( port_path: &str, baud_rate: u32, @@ -869,7 +883,7 @@ impl Ch9329Backend { match Self::open_ready_port(port_path, baud_rate, address) { Ok((port, info)) => { info!( - "CH9329 reconnected: {}, USB: {}", + "CH9329-compatible chip reconnected: {}, USB: {}", info.version, if info.usb_connected { "connected" @@ -892,7 +906,6 @@ impl Ch9329Backend { } } } - fn recover_worker_port( mut port: Box, rx: &mpsc::Receiver, @@ -1170,7 +1183,7 @@ impl HidBackend for Ch9329Backend { match init_rx.recv_timeout(Duration::from_millis(INIT_WAIT_MS)) { Ok(Ok(info)) => { info!( - "CH9329 chip detected: {}, USB: {}, LEDs: NumLock={}, CapsLock={}, ScrollLock={}", + "CH9329-compatible chip detected: {}, USB: {}, LEDs: NumLock={}, CapsLock={}, ScrollLock={}", info.version, if info.usb_connected { "connected" @@ -1189,13 +1202,13 @@ impl HidBackend for Ch9329Backend { Ok(Err(err)) => { self.record_error( format!( - "CH9329 not responding on {} @ {} baud: {}", + "CH9329-compatible chip not responding on {} @ {} baud: {}", self.port_path, self.baud_rate, err ), "init_failed", ); warn!( - "CH9329 not responding on {} @ {} baud, retrying in background: {}", + "CH9329-compatible chip not responding on {} @ {} baud, retrying in background: {}", self.port_path, self.baud_rate, err ); *self.worker_tx.lock() = Some(tx); @@ -1205,9 +1218,12 @@ impl HidBackend for Ch9329Backend { Err(_) => { let _ = tx.send(WorkerCommand::Shutdown); let _ = handle.join(); - self.record_error("Timed out waiting for CH9329 worker init", "init_timeout"); + self.record_error( + "Timed out waiting for CH9329-compatible worker init", + "init_timeout", + ); Err(AppError::Internal( - "Timed out waiting for CH9329 initialization".to_string(), + "Timed out waiting for CH9329-compatible initialization".to_string(), )) } } diff --git a/src/hid/ch9329_proto.rs b/src/hid/ch9329_proto.rs index c0594f30..28eb9202 100644 --- a/src/hid/ch9329_proto.rs +++ b/src/hid/ch9329_proto.rs @@ -10,6 +10,7 @@ pub const DEFAULT_ADDR: u8 = 0x00; pub const DEFAULT_BAUD_RATE: u32 = 9600; pub const MAX_DATA_LEN: usize = 64; pub const MAX_PACKET_SIZE: usize = 70; +const EXTENDED_PARAMETER_RESPONSE_SIZES: [usize; 2] = [72, 88]; pub mod cmd { pub const GET_INFO: u8 = 0x01; @@ -130,7 +131,8 @@ impl Response { let cmd = bytes[3]; let len = bytes[4] as usize; - if bytes.len() < 5 + len + 1 { + let expected_frame_len = 6 + len; + if bytes.len() < expected_frame_len { return None; } @@ -139,7 +141,7 @@ impl Response { .iter() .fold(0u8, |acc, &x| acc.wrapping_add(x)); if expected_checksum != calculated_checksum { - tracing::warn!( + tracing::debug!( "CH9329 checksum mismatch: expected {:02X}, got {:02X}", expected_checksum, calculated_checksum @@ -215,6 +217,11 @@ pub fn try_extract_response(buffer: &[u8]) -> Option<(Response, usize)> { } let len = buffer[offset + 4] as usize; + if len > MAX_DATA_LEN { + offset += 1; + continue; + } + let frame_len = 6 + len; if offset + frame_len > buffer.len() { return None; @@ -225,8 +232,103 @@ pub fn try_extract_response(buffer: &[u8]) -> Option<(Response, usize)> { return Some((response, offset + frame_len)); } + // Some CH9329F firmware appends reserved bytes to GET_PARA_CFG while + // retaining the protocol LEN value of 50. Locate and validate the real + // checksum, return the documented 50-byte payload, and consume the + // complete extended frame. Other commands keep strict framing. + let cmd = buffer[offset + 3]; + let data_start = offset + 5; + let parameter_payload_is_plausible = cmd == expected_response_cmd(cmd::GET_PARA_CFG, false) + && len == 50 + && matches!(buffer[data_start], 0x00..=0x03 | 0x80..=0x83) + && matches!(buffer[data_start + 1], 0x00..=0x02 | 0x80..=0x82); + if parameter_payload_is_plausible { + for extended_size in EXTENDED_PARAMETER_RESPONSE_SIZES { + let extended_end = offset + extended_size; + if buffer.len() >= extended_end { + let checksum_index = extended_end - 1; + if calculate_checksum(&buffer[offset..checksum_index]) != buffer[checksum_index] + { + continue; + } + return Some(( + Response { + cmd, + data: buffer[data_start..data_start + len].to_vec(), + is_error: false, + error_code: None, + }, + extended_end, + )); + } + } + + if buffer.len() < offset + EXTENDED_PARAMETER_RESPONSE_SIZES[1] { + return None; + } + } + offset += 1; } None } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn parses_standard_response_and_checksum() { + let frame = build_packet(DEFAULT_ADDR, 0x81, &[0x30, 0x01, 0x00, 0, 0, 0, 0, 0]); + let response = Response::parse(&frame).expect("valid response"); + assert_eq!(response.cmd, 0x81); + assert_eq!(response.data, vec![0x30, 0x01, 0x00, 0, 0, 0, 0, 0]); + assert!(!response.is_error); + } + + #[test] + fn extracts_extended_parameter_response_with_valid_trailing_checksum() { + let payload = [ + 0x80, 0x80, 0x00, 0x00, 0x00, 0x25, 0x80, 0x08, 0x00, 0x00, 0x03, 0x86, 0x1A, 0x2A, + 0xE1, 0x00, 0x00, 0x00, 0x01, 0x00, 0x0D, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + ]; + for reserved_len in [16, 32] { + let mut frame = vec![0x57, 0xAB, DEFAULT_ADDR, 0x88, 50]; + frame.extend_from_slice(&payload); + frame.extend_from_slice(&vec![0; reserved_len]); + frame.push(calculate_checksum(&frame)); + + assert!(Response::parse(&frame[..56]).is_none()); + let (response, consumed) = try_extract_response(&frame).expect("extended response"); + assert_eq!(response.cmd, 0x88); + assert_eq!(response.data, payload); + assert!(!response.is_error); + assert_eq!(consumed, frame.len()); + } + } + + #[test] + fn rejects_bad_checksum_for_other_commands() { + let mut frame = build_packet(DEFAULT_ADDR, 0x89, &[0x00; 50]); + *frame.last_mut().unwrap() ^= 0xFF; + assert!(Response::parse(&frame).is_none()); + assert!(try_extract_response(&frame).is_none()); + } + + #[test] + fn extracts_noise_and_adjacent_packets() { + let first = build_packet(DEFAULT_ADDR, 0x81, &[0x30, 0x01, 0, 0, 0, 0, 0, 0]); + let second = build_packet(DEFAULT_ADDR, 0x82, &[0x00]); + let mut buffer = vec![0x00, 0xFF]; + buffer.extend_from_slice(&first); + buffer.extend_from_slice(&second); + + let (_, consumed) = try_extract_response(&buffer).expect("first response"); + assert_eq!(consumed, 2 + first.len()); + let (response, _) = try_extract_response(&buffer[consumed..]).expect("second response"); + assert_eq!(response.cmd, 0x82); + } +}