From 68e477a27861341fb65f97a2e2e02bede3ef7cce Mon Sep 17 00:00:00 2001 From: Billy Price Date: Fri, 4 Sep 2026 11:45:38 -0700 Subject: [PATCH 1/4] hidi2c: fix command protocol This change fixes two bugs in the HID-I2C command protocol: 1. In the GET_REPORT path, we weren't listening for a read request before responding with the report payload. On some HALs, this causes the bytes you write to be dropped and never get transmitted over the bus. 2. In the SET_REPORT path, when not using implicit report IDs, the host actually writes the report ID twice; once in the command register and again in the data register. We were not accounting for the second write as the first byte in the data register. --- hidi2c-target-service/src/service.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/hidi2c-target-service/src/service.rs b/hidi2c-target-service/src/service.rs index 770da99f..ebe59a05 100644 --- a/hidi2c-target-service/src/service.rs +++ b/hidi2c-target-service/src/service.rs @@ -612,6 +612,8 @@ impl< // but as soon as the aggregation / HID library goes in, look into leveraging it for filtering out invalid report // IDs here. + bus.listen_for_response().await?; + hid_device .process_get_report(report_type.try_into()?, report_id, async |report| { // Note: per HID spec, the length field needs to include its own length (2 bytes) @@ -639,8 +641,9 @@ impl< .checked_sub(device_descriptor::HID_REPORT_HEADER_SIZE_BYTES)) .ok_or(Error::Protocol(ProtocolError::InvalidSize))? as usize; + let data_start_index = if hid_device.report_descriptor().report_ids_implicit() { 0 } else { 1 }; let report_data = data - .get(..report_size) + .get(data_start_index..report_size) .ok_or(Error::Protocol(ProtocolError::InvalidSize))?; let set_report = match report_type { From 2fe923626506a7c3bfd6fba9c9d2c8cb61d30d70 Mon Sep 17 00:00:00 2001 From: Billy Price Date: Fri, 4 Sep 2026 11:58:23 -0700 Subject: [PATCH 2/4] clippy --- hidi2c-target-service/src/service.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/hidi2c-target-service/src/service.rs b/hidi2c-target-service/src/service.rs index ebe59a05..3794e642 100644 --- a/hidi2c-target-service/src/service.rs +++ b/hidi2c-target-service/src/service.rs @@ -641,7 +641,11 @@ impl< .checked_sub(device_descriptor::HID_REPORT_HEADER_SIZE_BYTES)) .ok_or(Error::Protocol(ProtocolError::InvalidSize))? as usize; - let data_start_index = if hid_device.report_descriptor().report_ids_implicit() { 0 } else { 1 }; + let data_start_index = if hid_device.report_descriptor().report_ids_implicit() { + 0 + } else { + 1 + }; let report_data = data .get(data_start_index..report_size) .ok_or(Error::Protocol(ProtocolError::InvalidSize))?; From aac3605ebfa281f815ff517e06c6f0f3e104c52d Mon Sep 17 00:00:00 2001 From: Billy Price Date: Fri, 4 Sep 2026 13:22:14 -0700 Subject: [PATCH 3/4] offset --- hidi2c-target-service/src/service.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hidi2c-target-service/src/service.rs b/hidi2c-target-service/src/service.rs index 3794e642..ac6c1b62 100644 --- a/hidi2c-target-service/src/service.rs +++ b/hidi2c-target-service/src/service.rs @@ -647,7 +647,7 @@ impl< 1 }; let report_data = data - .get(data_start_index..report_size) + .get(data_start_index..data_start_index + report_size) .ok_or(Error::Protocol(ProtocolError::InvalidSize))?; let set_report = match report_type { From 7163e139bdcc8b1939f1f4f9cb5d12e11d318584 Mon Sep 17 00:00:00 2001 From: Billy Price Date: Fri, 4 Sep 2026 13:28:52 -0700 Subject: [PATCH 4/4] pr feedback --- hidi2c-target-service/src/service.rs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/hidi2c-target-service/src/service.rs b/hidi2c-target-service/src/service.rs index ac6c1b62..7d79249b 100644 --- a/hidi2c-target-service/src/service.rs +++ b/hidi2c-target-service/src/service.rs @@ -612,7 +612,13 @@ impl< // but as soon as the aggregation / HID library goes in, look into leveraging it for filtering out invalid report // IDs here. - bus.listen_for_response().await?; + match bus.listen_for_response().await? { + Request::Read(_address) => {} + other => { + error!("Expected read request after get report command, got {:?}", other); + return Err(Error::Protocol(ProtocolError::InvalidCommand)); + } + } hid_device .process_get_report(report_type.try_into()?, report_id, async |report| {