Skip to content

Commit e968767

Browse files
authored
Fix I2C HID command to use write_read for response commands (#867)
Per the HID over I2C Protocol Spec v1.0, commands that require a response (GetReport, GetIdle, GetProtocol) must be performed as a single I2C transaction with a repeated START condition (no STOP between the write and read phases). The previous implementation used separate write and read operations, which inserted a STOP condition between them, violating the spec. Use write_read with a separate stack-allocated command buffer (max 7 bytes for these commands) so the command is sent and the response is read in a single I2C transaction. Also simplify the data_reg conditional logic in both branches to remove dead-code conditions. Assisted-by: GitHub Copilot:claude-opus-4.6
1 parent 053388e commit e968767

1 file changed

Lines changed: 71 additions & 46 deletions

File tree

hid-service/src/i2c/device.rs

Lines changed: 71 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -196,60 +196,85 @@ impl<A: AddressMode + Copy, B: I2c<A>> Device<A, B> {
196196
let buffer_len = buf.len();
197197

198198
let opcode: Opcode = cmd.into();
199-
let len = cmd
200-
.encode_into_slice(
201-
buf,
202-
Some(command_reg),
203-
if opcode.has_response() || opcode.requires_host_data() {
204-
Some(data_reg)
205-
} else {
206-
None
207-
},
199+
200+
if opcode.has_response() {
201+
// Commands that require a response (GetReport, GetIdle, GetProtocol)
202+
// have an upper limit of 7 bytes for the command
203+
let mut temp_w_buf = [0u8; 7];
204+
205+
let len = cmd
206+
.encode_into_slice(&mut temp_w_buf, Some(command_reg), Some(data_reg))
207+
.map_err(|_| {
208+
error!("Failed to serialize command");
209+
Error::Hid(hid::Error::Serialize)
210+
})?;
211+
212+
let mut bus = self.bus.lock().await;
213+
214+
with_timeout(
215+
self.timeout_config.device_response_timeout,
216+
bus.write_read(
217+
self.address,
218+
temp_w_buf
219+
.get(..len)
220+
.ok_or(Error::Hid(hid::Error::InvalidSize(InvalidSizeError {
221+
expected: len,
222+
actual: temp_w_buf.len(),
223+
})))?,
224+
buf,
225+
),
208226
)
227+
.await
209228
.map_err(|_| {
210-
error!("Failed to serialize command");
211-
Error::Hid(hid::Error::Serialize)
229+
error!("Command write_read timeout");
230+
Error::Hid(hid::Error::Timeout)
231+
})?
232+
.map_err(|e| {
233+
error!("Failed to execute command write_read");
234+
Error::Bus(e)
212235
})?;
213236

214-
let mut bus = self.bus.lock().await;
215-
with_timeout(
216-
self.timeout_config.device_response_timeout,
217-
bus.write(
218-
self.address,
219-
buf.get(..len)
220-
.ok_or(Error::Hid(hid::Error::InvalidSize(InvalidSizeError {
221-
expected: len,
222-
actual: buffer_len,
223-
})))?,
224-
),
225-
)
226-
.await
227-
.map_err(|_| {
228-
error!("Write command timeout");
229-
Error::Hid(hid::Error::Timeout)
230-
})?
231-
.map_err(|e| {
232-
error!("Failed to write command");
233-
Error::Bus(e)
234-
})?;
235-
236-
if opcode.has_response() {
237-
trace!("Reading host data");
238-
with_timeout(self.timeout_config.data_read_timeout, bus.read(self.address, buf))
239-
.await
237+
Ok(Some(Response::FeatureReport(self.buffer.reference())))
238+
} else {
239+
let len = cmd
240+
.encode_into_slice(
241+
buf,
242+
Some(command_reg),
243+
if opcode.requires_host_data() {
244+
Some(data_reg)
245+
} else {
246+
None
247+
},
248+
)
240249
.map_err(|_| {
241-
error!("Read host data timeout");
242-
Error::Hid(hid::Error::Timeout)
243-
})?
244-
.map_err(|e| {
245-
error!("Failed to read host data");
246-
Error::Bus(e)
250+
error!("Failed to serialize command");
251+
Error::Hid(hid::Error::Serialize)
247252
})?;
248253

249-
return Ok(Some(Response::FeatureReport(self.buffer.reference())));
250-
}
254+
let mut bus = self.bus.lock().await;
255+
with_timeout(
256+
self.timeout_config.device_response_timeout,
257+
bus.write(
258+
self.address,
259+
buf.get(..len)
260+
.ok_or(Error::Hid(hid::Error::InvalidSize(InvalidSizeError {
261+
expected: len,
262+
actual: buffer_len,
263+
})))?,
264+
),
265+
)
266+
.await
267+
.map_err(|_| {
268+
error!("Write command timeout");
269+
Error::Hid(hid::Error::Timeout)
270+
})?
271+
.map_err(|e| {
272+
error!("Failed to write command");
273+
Error::Bus(e)
274+
})?;
251275

252-
Ok(None)
276+
Ok(None)
277+
}
253278
}
254279

255280
pub async fn process_request(&self) -> Result<(), Error<B::Error>> {

0 commit comments

Comments
 (0)