From 45023276291cfd8d7cfadb08fa9061ec880fa256 Mon Sep 17 00:00:00 2001 From: Billy Price Date: Tue, 12 May 2026 15:16:47 -0700 Subject: [PATCH 01/17] Implement HID-I2C target service A new service that implements the HID-I2C spec, written against the I2C HAL trait and a new HidDevice trait that is transport-agnostic. This obsoletes the current hid-service, which is really a HID-I2C service that leverages the comms system / message passing and doesn't handle interrupt line management. A future change will introduce an analogue to the impl_odp_mctp_relay_handler macro to aggregate multiple logical HID devices into a single HID device. This should allow clients to write transport-agnostic HID device code that can be reused without modification on other, future physical transports (e.g. I3C), and removes our dependance on the comms system, which imposes lifetime requirements that make testing difficult. --- Cargo.lock | 34 +- Cargo.toml | 3 + battery-service-relay/src/serialization.rs | 2 +- debug-service-messages/src/lib.rs | 2 +- embedded-service/Cargo.toml | 2 + embedded-service/src/relay/hid.rs | 284 +++++++ embedded-service/src/relay/mctp.rs | 499 +++++++++++++ embedded-service/src/relay/mod.rs | 506 +------------ examples/rt685s-evk/Cargo.lock | 36 +- examples/rt685s-evk/Cargo.toml | 10 +- .../rt685s-evk/src/bin/mock_i2c_keyboard.rs | 100 +++ examples/rt685s-evk/src/bin/mock_i2c_mouse.rs | 95 +++ examples/rt685s-evk/src/lib.rs | 2 + examples/rt685s-evk/src/mocks/keyboard.rs | 269 +++++++ examples/rt685s-evk/src/mocks/mod.rs | 4 + examples/rt685s-evk/src/mocks/mouse.rs | 249 +++++++ examples/std/Cargo.lock | 19 + hidi2c-target-service/Cargo.toml | 28 + hidi2c-target-service/src/attn_pin_handler.rs | 45 ++ .../src/constrained_hid_device.rs | 43 ++ .../src/device_descriptor.rs | 121 +++ hidi2c-target-service/src/error.rs | 56 ++ hidi2c-target-service/src/lib.rs | 50 ++ hidi2c-target-service/src/service.rs | 699 ++++++++++++++++++ thermal-service-relay/src/serialization.rs | 2 +- time-alarm-service-relay/src/serialization.rs | 2 +- 26 files changed, 2651 insertions(+), 511 deletions(-) create mode 100644 embedded-service/src/relay/hid.rs create mode 100644 embedded-service/src/relay/mctp.rs create mode 100644 examples/rt685s-evk/src/bin/mock_i2c_keyboard.rs create mode 100644 examples/rt685s-evk/src/bin/mock_i2c_mouse.rs create mode 100644 examples/rt685s-evk/src/mocks/keyboard.rs create mode 100644 examples/rt685s-evk/src/mocks/mod.rs create mode 100644 examples/rt685s-evk/src/mocks/mouse.rs create mode 100644 hidi2c-target-service/Cargo.toml create mode 100644 hidi2c-target-service/src/attn_pin_handler.rs create mode 100644 hidi2c-target-service/src/constrained_hid_device.rs create mode 100644 hidi2c-target-service/src/device_descriptor.rs create mode 100644 hidi2c-target-service/src/error.rs create mode 100644 hidi2c-target-service/src/lib.rs create mode 100644 hidi2c-target-service/src/service.rs diff --git a/Cargo.lock b/Cargo.lock index 1eb354542..586ed7330 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -534,7 +534,7 @@ dependencies = [ [[package]] name = "embassy-imxrt" version = "0.1.0" -source = "git+https://github.com/OpenDevicePartnership/embassy-imxrt#8b9a6f0e11d9b65f1616a9e487af2cf04aef3584" +source = "git+https://github.com/OpenDevicePartnership/embassy-imxrt#0e68fe138967b9469ecd952a702fc7e42852fe47" dependencies = [ "cfg-if", "cortex-m", @@ -652,7 +652,7 @@ dependencies = [ [[package]] name = "embedded-cfu-protocol" version = "0.2.0" -source = "git+https://github.com/OpenDevicePartnership/embedded-cfu#523a83919c3535e8ccead031d80b715d77395f81" +source = "git+https://github.com/OpenDevicePartnership/embedded-cfu#300ff65bb03e0ff476708851c6b5d9b644292ec0" dependencies = [ "defmt 0.3.100", "embedded-io-async 0.6.1", @@ -797,8 +797,10 @@ dependencies = [ "defmt 0.3.100", "embassy-futures", "embassy-sync", + "generic-array", "log", "mctp-rs", + "num_enum", "paste", "portable-atomic", "serde", @@ -991,6 +993,16 @@ dependencies = [ "windows-result", ] +[[package]] +name = "generic-array" +version = "1.4.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "fb130435a959a8d525e6bca66ff6c40981a300ee96d70e3ef56f046556d614a3" +dependencies = [ + "rustversion", + "typenum", +] + [[package]] name = "glob" version = "0.3.3" @@ -1062,6 +1074,24 @@ dependencies = [ "log", ] +[[package]] +name = "hidi2c-target-service" +version = "0.1.0" +dependencies = [ + "defmt 0.3.100", + "embassy-futures", + "embassy-sync", + "embassy-time", + "embedded-hal 1.0.0", + "embedded-mcu-hal", + "embedded-services", + "generic-array", + "num_enum", + "odp-service-common", + "typenum", + "zerocopy", +] + [[package]] name = "include_dir" version = "0.7.4" diff --git a/Cargo.toml b/Cargo.toml index 37c6dab9d..7df8d3c1d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -12,6 +12,7 @@ members = [ "espi-service", "uart-service", "hid-service", + "hidi2c-target-service", "partition-manager/generation", "partition-manager/macros", "partition-manager/partition-manager", @@ -96,6 +97,7 @@ embedded-storage-async = "0.4.1" embedded-usb-pd = { git = "https://github.com/OpenDevicePartnership/embedded-usb-pd", tag = "v0.1.0", default-features = false } fw-update-interface = { path = "./fw-update-interface" } fw-update-interface-test-mocks = { path = "./fw-update-interface-test-mocks" } +generic-array = "1.4.1" mctp-rs = { path = "./mctp-rs" } num_enum = { version = "0.7.5", default-features = false } portable-atomic = { version = "1.11", default-features = false } @@ -118,6 +120,7 @@ time-alarm-service-interface = { path = "./time-alarm-service-interface" } time-alarm-service-relay = { path = "./time-alarm-service-relay" } type-c-interface = { path = "./type-c-interface" } type-c-interface-test-mocks = { path = "./type-c-interface-test-mocks" } +typenum = "1.20" syn = "2.0" tokio = { version = "1.42.0" } uuid = { version = "=1.17.0", default-features = false } diff --git a/battery-service-relay/src/serialization.rs b/battery-service-relay/src/serialization.rs index 39f411a44..3fdf94f44 100644 --- a/battery-service-relay/src/serialization.rs +++ b/battery-service-relay/src/serialization.rs @@ -1,5 +1,5 @@ use battery_service_interface::*; -use embedded_services::relay::{MessageSerializationError, SerializableMessage}; +use embedded_services::relay::mctp::{MessageSerializationError, SerializableMessage}; #[derive(num_enum::IntoPrimitive, num_enum::TryFromPrimitive, Copy, Clone, Debug, PartialEq)] #[repr(u16)] diff --git a/debug-service-messages/src/lib.rs b/debug-service-messages/src/lib.rs index a4dafc673..41ef0f004 100644 --- a/debug-service-messages/src/lib.rs +++ b/debug-service-messages/src/lib.rs @@ -1,5 +1,5 @@ #![no_std] -use embedded_services::relay::{MessageSerializationError, SerializableMessage}; +use embedded_services::relay::mctp::{MessageSerializationError, SerializableMessage}; /// Standard Debug Service Log Buffer Size pub const STD_DEBUG_BUF_SIZE: usize = 128; diff --git a/embedded-service/Cargo.toml b/embedded-service/Cargo.toml index 9774188c1..873fe610d 100644 --- a/embedded-service/Cargo.toml +++ b/embedded-service/Cargo.toml @@ -16,7 +16,9 @@ critical-section.workspace = true defmt = { workspace = true, optional = true } embassy-sync.workspace = true embassy-futures.workspace = true +generic-array.workspace = true log = { workspace = true, optional = true } +num_enum.workspace = true paste.workspace = true [dependencies.mctp-rs] diff --git a/embedded-service/src/relay/hid.rs b/embedded-service/src/relay/hid.rs new file mode 100644 index 000000000..9d06c602a --- /dev/null +++ b/embedded-service/src/relay/hid.rs @@ -0,0 +1,284 @@ +//! HID relay code + +use generic_array::ArrayLength; +use num_enum::TryFromPrimitive; + +/// Errors that a HID device operation can fail with. +/// +/// Reporting failure triggers a device-initiated reset, so callers must handle these errors explicitly +/// rather than swallowing them. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +#[cfg_attr(feature = "defmt", derive(defmt::Format))] +pub enum HidError { + /// The operation has failed and a device-initiated reset should be triggered. + TriggerReset, +} + +/// Power states that the host can command a HID device to be put into. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +#[cfg_attr(feature = "defmt", derive(defmt::Format))] +pub enum HidDevicePowerState { + /// Normal operation + On, + + /// Reduced power state, but a device that sends a report in this state can wake the host - quiesce messages if you don't want to do that + Sleep, + + /// The device is not allowed to wake the host. This is not supported on all transports - in particular, I2C will never command a device into the off state. + Off, +} + +/// A HID report of no more than X bytes +pub struct HidReport<'buf> { + id: ReportId, + + data: &'buf [u8], +} + +impl<'buf> HidReport<'buf> { + /// Create a new HID report from the provided data slice. + pub fn new(id: ReportId, data: &'buf [u8]) -> Self { + Self { id, data } + } + + /// The report ID for this report + pub fn id(&self) -> ReportId { + self.id + } + + /// The data for this report. + pub fn data(&self) -> &'buf [u8] { + self.data + } +} + +/// HID report types supported by the SetReport operation. +pub enum SetHidReport<'buf> { + /// An output report + Output(HidReport<'buf>), + + /// A feature report + Feature(HidReport<'buf>), +} + +impl<'buf> SetHidReport<'buf> { + /// The data for this report, whatever its type. + pub fn data(&self) -> &'buf [u8] { + match self { + SetHidReport::Output(report) => report.data(), + SetHidReport::Feature(report) => report.data(), + } + } +} + +/// A type of report that can be requested by the host +pub enum GetHidReportType { + /// The host has requested an input report + Input, + + /// The host has requested a feature report + Feature, +} + +/// HID report types supported by the GetReport operation. +pub enum GetHidReport<'buf> { + /// An input report + Input(HidReport<'buf>), + + /// A feature report + Feature(HidReport<'buf>), +} + +impl<'buf> GetHidReport<'buf> { + /// The data for this report, whatever its type. + pub fn data(&self) -> &'buf [u8] { + match self { + GetHidReport::Input(report) => report.data(), + GetHidReport::Feature(report) => report.data(), + } + } +} + +/// HID report ID +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +#[cfg_attr(feature = "defmt", derive(defmt::Format))] +pub struct ReportId(pub u8); + +/// A single HID device that we want to present to the host. +/// This is a transport-agnostic trait that abstracts over the details of how we get reports to/from the host, +/// so that we can implement it once and then use it for both HID-I2C and HID-I3C (and potentially HID-SPI in the +/// future if we want to add support for that). +/// +/// A note on error handling - the HID spec only really has one way for a device to communicate failure, and that's +/// by doing a device-initiated reset. If any of these functions fail, a device-initiated reset will be signalled. +/// +/// If you're part of an aggregate device created by impl_odp_hid_aggregate_device!, ***this will reset your peers too***. +/// Therefore, you should be very certain this is the behavior you want before you return an error from any of these functions. +/// +/// The normal pattern in HID seems to be to either embed an error code in an input report or to drop the message entirely. +/// +pub trait HidDevice { + /// The maximum size of an input report (device -> host) that this device can use, expressed in bytes. + /// This must agree with the descriptor returned by `report_descriptor()`. + type InputReportMaxSize: ArrayLength; + + /// The maximum size of an output report (host -> device) that this device can use, expressed in bytes. + /// This must agree with the descriptor returned by `report_descriptor()`. + type OutputReportMaxSize: ArrayLength; + + /// The maximum size of a feature report (bidirectional) that this device can use, expressed in bytes. + /// This must agree with the descriptor returned by `report_descriptor()`. + type FeatureReportMaxSize: ArrayLength; + + /// The maximum number of individual report IDs that the device will have. In most cases, this should be exactly + /// the number of individual report IDs that the device has, but in the passthrough case where that knowledge isn't + /// available at compile time, this will be an upper bound. This must agree with the descriptor returned by report_descriptor(). + /// + /// Note that this is the maximum number of unique report *IDs* - if you have a device that shares the same report ID for + /// an input and output report, that only counts as 1. + const MAX_REPORT_COUNT: u8; + + /// Returns the HID descriptor for this device. This isn't allowed to change, but the passthrough case means that + /// we can't require that it be known at compile time. + /// If the descriptor disagrees with the sizes implied by `InputReport` / `FeatureReport` / `OutputReport` / `MAX_REPORT_COUNT`, callers should not use the object. + fn report_descriptor(&self) -> &HidReportDescriptor<'_>; + + /// Respond to an explicit request for a particular report from the host. + /// + /// This invokes `process_report` with the requested [`GetHidReport`]. + /// + /// The value returned by `process_report` must be propagated back to the caller. Returning + /// `Err(HidError)` (before `process_report` is invoked) signals that the requested report could + /// not be produced. + fn process_get_report( + &mut self, + report_type: GetHidReportType, + report_id: ReportId, + process_report: impl AsyncFnOnce(GetHidReport<'_>) -> R, + ) -> impl core::future::Future>; + + /// Respond to a command from the host to handle a particular output/feature report. + fn set_report(&mut self, report: &SetHidReport<'_>) -> impl core::future::Future>; + + /// Blocks until the device is ready to yield an unsolicited input report. + /// When this returns, the next call to process_next_input_report should be able to run without blocking on I/O. + fn wait_for_input_report(&mut self) -> impl core::future::Future; + + /// Returns true if there is a pending input report that can be retrieved immediately with process_next_input_report(). + /// If this returns true, it implies that wait_for_input_report() and process_next_input_report() should return immediately. + fn has_pending_input_report(&mut self) -> bool; + + /// Process the next unsolicited input report to the transport. + /// + /// This blocks until an unsolicited report is available, then invokes `process_report` with a [`HidReport`]. + /// + /// The value returned by `process_report` must be propagated back to the caller. Returning `Err(HidError)` + /// (before `process_report` is invoked) signals an inability to retrieve a message. + /// + /// This is for 'unsolicited' reports that the device has decided to signal the host to retrieve. + /// + fn process_next_input_report( + &mut self, + process_report: impl AsyncFnOnce(HidReport<'_>) -> R, + ) -> impl core::future::Future>; + + /// Called when the host commands a particular power state. + fn set_power_state( + &mut self, + state: HidDevicePowerState, + ) -> impl core::future::Future>; + + /// Called when the device should reset its state. The semantics of reset are device-specific, but + /// should generally result in clearing any pending reports and returning to a known-good state. + /// This can be called under the following circumstances: + /// 1. The host commands a reset, which happens once at startup and can happen again at any time + /// 2. The implementor of this trait returned HidError::TriggerReset from one of its functions, thereby requesting a reset + /// 3. A peer HidDevice in an aggregate device triggers a device-initiated reset (see impl_odp_hid_aggregate_device! for details) + /// + fn reset(&mut self) -> impl core::future::Future; +} + +/// A HID report descriptor +pub struct HidReportDescriptor<'buf> { + bytes: &'buf [u8], + + report_ids_implicit: bool, +} + +struct HidReportDescriptorElementHeader(u8); + +#[derive(Clone, Copy, Debug, PartialEq, Eq, num_enum::IntoPrimitive, num_enum::TryFromPrimitive)] +#[repr(u8)] +enum HidItemType { + Main = 0, + Global = 1, + Local = 2, + Reserved = 3, +} + +impl HidReportDescriptorElementHeader { + /// The size of this item in bytes. + fn item_size(&self) -> usize { + #[allow(clippy::panic)] // This is temporary we get the HID support library implemented + if self.0 == 0b11111110 { + panic!("Long items are not yet supported"); // TODO implement this once we have the HID support library implemented - see 6.2.2.3 of https://www.usb.org/sites/default/files/hid1_11.pdf + } + + match self.0 & 0b11 { + 0 => 0, + 1 => 1, + 2 => 2, + _ => 4, // per hid spec, size=3 means 4 bytes, not 3 bytes. See section 6.2.2.2 of https://www.usb.org/sites/default/files/hid1_11.pdf + } + } + + /// The type of the item, which is one of Main, Global, Local, or Reserved. + fn item_type(&self) -> HidItemType { + // This can't actually panic because we mask to 2 bits, but there's no way to express that in the type system + #[allow(clippy::expect_used)] + HidItemType::try_from_primitive((self.0 >> 2) & 0b11) + .expect("HidItemType::try_from_primitive should never fail because we mask to 2 bits") + } + + /// The tag of this item, which is a 4-bit value that identifies the specific item within its type (e.g. start collection, end collection, input, output, etc) + fn item_tag(&self) -> u8 { + self.0 >> 4 + } +} + +impl<'buf> HidReportDescriptor<'buf> { + /// Constructs a HID descriptor from a byte slice. + pub fn new(bytes: &'buf [u8]) -> Result { + // TODO - validation is incomplete here. When we implement the HID support library / aggregation macro, we should have more tools to validate the descriptor here. + let mut iter = bytes.iter(); + let mut implicit = true; + while let Some(header_bytes) = iter.next() { + const REPORT_ID_ITEM_TAG: u8 = 0b1000; // per section 6.2.2.7 + let header = HidReportDescriptorElementHeader(*header_bytes); + if header.item_type() == HidItemType::Global && header.item_tag() == REPORT_ID_ITEM_TAG { + implicit = false; + break; + } + + if header.item_size() != 0 { + iter.nth(header.item_size() - 1); // skip over the data bytes for this item + } + } + + Ok(Self { + bytes, + report_ids_implicit: implicit, + }) + } + + /// Returns the raw bytes of the HID report descriptor. This is what will be sent to the host when it requests the HID descriptor. + pub fn as_bytes(&self) -> &'buf [u8] { + self.bytes + } + + /// Whether or not the report IDs are implicit in the report descriptor. If true, the report ID is not sent to the host as part of the report header. + /// This is only possible on devices that have no more than one report of each type (input, output, feature). + pub fn report_ids_implicit(&self) -> bool { + self.report_ids_implicit + } +} diff --git a/embedded-service/src/relay/mctp.rs b/embedded-service/src/relay/mctp.rs new file mode 100644 index 000000000..ded25f733 --- /dev/null +++ b/embedded-service/src/relay/mctp.rs @@ -0,0 +1,499 @@ +//! Contains helper functions for services that relay comms messages over MCTP + +/// Error type for serializing/deserializing messages +#[derive(Debug)] +#[cfg_attr(feature = "defmt", derive(defmt::Format))] +pub enum MessageSerializationError { + /// The message payload does not represent a valid message + InvalidPayload(&'static str), + + /// The message discriminant does not represent a known message type + UnknownMessageDiscriminant(u16), + + /// The provided buffer is too small to serialize the message + BufferTooSmall, + + /// Unspecified error + Other(&'static str), +} + +/// Trait for serializing and deserializing messages +pub trait SerializableMessage: Sized { + /// Serializes the message into the provided buffer. + /// On success, returns the number of bytes written + fn serialize(self, buffer: &mut [u8]) -> Result; + + /// Returns the discriminant needed to deserialize this type of message. + fn discriminant(&self) -> u16; + + /// Deserializes the message from the provided buffer. + fn deserialize(discriminant: u16, buffer: &[u8]) -> Result; +} + +// Prevent other types from implementing SerializableResult - they should instead use SerializableMessage on a Response type and an Error type +#[doc(hidden)] +mod private { + pub trait Sealed {} + + impl Sealed for Result {} +} + +/// Responses sent over MCTP are called "Results" and are of type Result where T and E both implement SerializableMessage +pub trait SerializableResult: private::Sealed + Sized { + /// The type of the result when the operation being responded to succeeded + type SuccessType: SerializableMessage; + + /// The type of the result when the operation being responded to failed + type ErrorType: SerializableMessage; + + /// Returns true if the result represents a successful operation, false otherwise + fn is_ok(&self) -> bool; + + /// Returns a unique discriminant that can be used to deserialize the specific type of result. + /// Discriminants can be reused for success and error messages. + fn discriminant(&self) -> u16; + + /// Writes the result into the provided buffer. + /// On success, returns the number of bytes written + fn serialize(self, buffer: &mut [u8]) -> Result; + + /// Attempts to deserialize the result from the provided buffer. + fn deserialize(is_error: bool, discriminant: u16, buffer: &[u8]) -> Result; +} + +impl SerializableResult for Result +where + T: SerializableMessage, + E: SerializableMessage, +{ + type SuccessType = T; + type ErrorType = E; + + fn is_ok(&self) -> bool { + Result::::is_ok(self) + } + + fn discriminant(&self) -> u16 { + match self { + Ok(success_value) => success_value.discriminant(), + Err(error_value) => error_value.discriminant(), + } + } + + fn serialize(self, buffer: &mut [u8]) -> Result { + match self { + Ok(success_value) => success_value.serialize(buffer), + Err(error_value) => error_value.serialize(buffer), + } + } + + fn deserialize(is_error: bool, discriminant: u16, buffer: &[u8]) -> Result { + if is_error { + Ok(Err(E::deserialize(discriminant, buffer)?)) + } else { + Ok(Ok(T::deserialize(discriminant, buffer)?)) + } + } +} + +/// Error type for MCTP relay operations +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +#[cfg_attr(feature = "defmt", derive(defmt::Format))] +pub enum MctpError { + /// The endpoint ID does not correspond to a known service + UnknownEndpointId, +} + +/// Trait for types that are used by a relay service to relay messages from your service over the wire. +/// If you are implementing this trait, you should also implement RelayServiceHandler. +/// +pub trait RelayServiceHandlerTypes { + /// The request type that this service handler processes + type RequestType: SerializableMessage; + + /// The result type that this service handler processes + type ResultType: SerializableResult; +} + +/// Trait for a service that can be relayed over an external bus (e.g. battery service, thermal service, time-alarm service) +/// +pub trait RelayServiceHandler: RelayServiceHandlerTypes { + /// Process the provided request and yield a result. + fn process_request<'a>( + &'a self, + request: Self::RequestType, + ) -> impl core::future::Future + 'a; +} + +// Traits below this point are intended for consumption by relay services (e.g. the eSPI service), not individual services that want their messages relayed. +// In general, you should not implement these yourself; rather, you should leverage the `impl_odp_mctp_relay_handler` macro to do that for you. + +/// Contains additional methods that must be implemented on the relay header type. +/// Do not implement this yourself - rather, rely on the `impl_odp_mctp_relay_handler` macro to implement this. +#[doc(hidden)] +pub trait RelayHeader { + /// Return the ID of the service associated with the request + fn get_service_id(&self) -> ServiceIdType; +} + +/// Contains additional methods that must be implemented on the relay response type. +/// Do not implement this yourself - rather, rely on the `impl_odp_mctp_relay_handler` macro to implement this. +#[doc(hidden)] +pub trait RelayResponse { + /// Construct an MCTP header suitable for representing the result based on the provided service handler ID and result + fn create_header(&self, service_id: &ServiceIdType) -> HeaderType; +} + +/// Trait for aggregating collections of services that can be relayed over an external bus. +/// Do not implement this yourself - rather, rely on the `impl_odp_mctp_relay_handler` macro to implement this. +/// +pub trait RelayHandler { + /// The type that uniquely identifies individual services. Generally expected to be a C-style enum. + type ServiceIdType: Into + TryFrom + Copy; + + /// The header type used by request and result enums + type HeaderType: mctp_rs::MctpMessageHeaderTrait + RelayHeader; + + /// An enum over all possible request types + type RequestEnumType: for<'buf> mctp_rs::MctpMessageTrait<'buf, Header = Self::HeaderType>; + + /// An enum over all possible result types + type ResultEnumType: for<'buf> mctp_rs::MctpMessageTrait<'buf, Header = Self::HeaderType> + + RelayResponse; + + /// Process the provided request and yield a result. + fn process_request<'a>( + &'a self, + message: Self::RequestEnumType, + ) -> impl core::future::Future + 'a; +} + +/// This macro generates a relay type over a collection of message types, which can be used by a relay service to +/// receive messages over the wire and translate them into calls to a particular service on the EC. +/// +/// This is the recommended way to implement a relay handler - you should not implement the RelayHandler trait yourself. +/// +/// This macro will emit a type with the name you specify that is generic over a lifetime for the hardware (probably 'static in production code), +/// implements the `RelayHandler` trait, and has a single constructor method `new` that takes as arguments references to the service handler +/// types that you specify that have the 'hardware lifetime'. +/// +/// The macro takes the following inputs once: +/// relay_type_name: The name of the relay type to generate. This is arbitrary. The macro will emit a type with this name. +/// +/// Followed by a list of any number of service entries, which are specified by the following inputs: +/// service_name: A name to assign to generated identifiers associated with the service, e.g. "Battery". +/// This can be arbitrary. +/// service_id: A unique u8 that addresses that service on the EC. +/// service_handler_type: A type that implements the RelayServiceHandler trait, which will be used to process messages +/// for this service. +/// +/// Example usage: +/// +/// ```ignore +/// +/// impl_odp_mctp_relay_handler!( +/// MyRelayHandlerType; +/// Battery, 0x9, battery_service_relay::RelayHandler>; +/// TimeAlarm, 0xB, time_alarm_service_relay::RelayHandler>; +/// ); +/// +/// let relay_handler = MyRelayHandlerType::new(battery_service_instance, time_alarm_service_instance); +/// +/// // Then, pass relay_handler to your relay service (e.g. eSPI service), which should be generic over an `impl RelayHandler`. +/// +/// ``` +/// +#[macro_export] +macro_rules! impl_odp_mctp_relay_handler { + ( + $relay_type_name:ident; + $( + $service_name:ident, + $service_id:expr, + $service_handler_type:ty; + )+ + ) => { + $crate::_macro_internal::paste::paste! { + mod [< _odp_impl_ $relay_type_name:snake >] { + use $crate::_macro_internal::bitfield::bitfield; + use core::convert::Infallible; + use $crate::_macro_internal::mctp_rs::smbus_espi::SmbusEspiMedium; + use $crate::_macro_internal::mctp_rs::{MctpMedium, MctpMessageHeaderTrait, MctpMessageTrait, MctpPacketError, MctpPacketResult}; + use $crate::relay::mctp::{SerializableMessage, SerializableResult}; + use $crate::relay::mctp::RelayServiceHandler; + + #[derive(Debug, PartialEq, Eq, Clone, Copy)] + #[repr(u8)] + pub enum OdpService { + $( + $service_name = $service_id, + )+ + } + + impl From for u8 { + fn from(val: OdpService) -> u8 { + val as u8 + } + } + + impl TryFrom for OdpService { + type Error = u8; + fn try_from(value: u8) -> Result { + match value { + $( + $service_id => Ok(OdpService::$service_name), + )+ + other => Err(other), + } + } + } + + pub enum HostRequest { + $( + $service_name(<$service_handler_type as $crate::relay::mctp::RelayServiceHandlerTypes>::RequestType), + )+ + } + + impl MctpMessageTrait<'_> for HostRequest { + type Header = OdpHeader; + const MESSAGE_TYPE: u8 = 0x7D; // ODP message type + + fn serialize(self, buffer: &mut [u8]) -> MctpPacketResult { + match self { + $( + HostRequest::$service_name(request) => SerializableMessage::serialize(request, buffer) + .map_err(|_| MctpPacketError::SerializeError(concat!("Failed to serialize ", stringify!($service_name), " request"))), + )+ + } + } + + fn deserialize(header: &Self::Header, buffer: &'_ [u8]) -> MctpPacketResult { + Ok(match header.service { + $( + OdpService::$service_name => Self::$service_name( + <$service_handler_type as $crate::relay::mctp::RelayServiceHandlerTypes>::RequestType::deserialize(header.message_id, buffer) + .map_err(|_| MctpPacketError::CommandParseError(concat!("Could not parse ", stringify!($service_name), " request")))?, + ), + )+ + }) + } + } + + bitfield! { + /// Wire format for ODP MCTP headers. Not user-facing - use OdpHeader instead. + #[derive(Copy, Clone, PartialEq, Eq)] + struct OdpHeaderWireFormat(u32); + impl Debug; + impl new; + /// If true, represents a request; otherwise, represents a result + is_request, set_is_request: 25; + + /// The service ID that this message is related to + /// Note: Error checking is done when you access the field, not when you construct the OdpHeader. Take care when constructing a header. + u8, service_id, set_service_id: 23, 16; + + /// On results, indicates if the result message is an error. Unused on requests. + is_error, set_is_error: 15; + + /// The message type/discriminant + u16, message_id, set_message_id: 14, 0; + } + + #[derive(Copy, Clone, PartialEq, Eq)] + pub enum OdpMessageType { + Request, + Result { is_error: bool }, + } + + #[derive(Copy, Clone, PartialEq, Eq)] + pub struct OdpHeader { + pub message_type: OdpMessageType, + pub service: OdpService, + pub message_id: u16, + } + + impl From for OdpHeaderWireFormat { + fn from(src: OdpHeader) -> Self { + Self::new( + matches!(src.message_type, OdpMessageType::Request), + src.service.into(), + match src.message_type { + OdpMessageType::Request => false, // unused on requests + OdpMessageType::Result { is_error } => is_error, + }, + src.message_id, + ) + } + } + + impl TryFrom for OdpHeader { + type Error = MctpPacketError; + + fn try_from(src: OdpHeaderWireFormat) -> Result { + let service = OdpService::try_from(src.service_id()) + .map_err(|_| MctpPacketError::HeaderParseError("invalid odp service in odp header"))?; + + let message_type = if src.is_request() { + OdpMessageType::Request + } else { + OdpMessageType::Result { + is_error: src.is_error(), + } + }; + + Ok(OdpHeader { + message_type, + service, + message_id: src.message_id(), + }) + } + } + + impl MctpMessageHeaderTrait for OdpHeader { + fn serialize(self, buffer: &mut [u8]) -> MctpPacketResult { + let wire_format = OdpHeaderWireFormat::from(self); + let bytes = wire_format.0.to_be_bytes(); + buffer + .get_mut(0..bytes.len()) + .ok_or(MctpPacketError::SerializeError("buffer too small for odp header"))? + .copy_from_slice(&bytes); + + Ok(bytes.len()) + } + + fn deserialize(buffer: &[u8]) -> MctpPacketResult<(Self, &[u8]), M> { + let bytes = buffer + .get(0..core::mem::size_of::()) + .ok_or(MctpPacketError::HeaderParseError("buffer too small for odp header"))?; + let raw = u32::from_be_bytes( + bytes + .try_into() + .map_err(|_| MctpPacketError::HeaderParseError("buffer too small for odp header"))?, + ); + + let parsed_wire_format = OdpHeaderWireFormat(raw); + let header = OdpHeader::try_from(parsed_wire_format) + .map_err(|_| MctpPacketError::HeaderParseError("invalid odp header received"))?; + + Ok(( + header, + buffer + .get(core::mem::size_of::()..) + .ok_or(MctpPacketError::HeaderParseError("buffer too small for odp header"))?, + )) + } + } + + impl $crate::relay::mctp::RelayHeader for OdpHeader { + fn get_service_id(&self) -> OdpService { + self.service + } + } + + #[derive(Clone)] + pub enum HostResult { + $( + $service_name(<$service_handler_type as $crate::relay::mctp::RelayServiceHandlerTypes>::ResultType), + )+ + } + + impl $crate::relay::mctp::RelayResponse for HostResult { + fn create_header(&self, service_id: &OdpService) -> OdpHeader { + match (self) { + $( + (HostResult::$service_name(result)) => OdpHeader { + message_type: OdpMessageType::Result { is_error: !result.is_ok() }, + service: *service_id, + message_id: result.discriminant(), + }, + )+ + } + } + } + + impl MctpMessageTrait<'_> for HostResult { + const MESSAGE_TYPE: u8 = 0x7D; // ODP message type + type Header = OdpHeader; + + fn serialize(self, buffer: &mut [u8]) -> MctpPacketResult { + match self { + $( + HostResult::$service_name(result) => result + .serialize(buffer) + .map_err(|_| MctpPacketError::SerializeError(concat!("Failed to serialize ", stringify!($service_name), " result"))), + )+ + } + } + + fn deserialize(header: &Self::Header, buffer: &'_ [u8]) -> MctpPacketResult { + match header.service { + $( + OdpService::$service_name => { + match header.message_type { + OdpMessageType::Request => { + Err(MctpPacketError::CommandParseError(concat!("Received ", stringify!($service_name), " request when expecting result"))) + } + OdpMessageType::Result { is_error } => { + Ok(HostResult::$service_name(<$service_handler_type as $crate::relay::mctp::RelayServiceHandlerTypes>::ResultType::deserialize(is_error, header.message_id, buffer) + .map_err(|_| MctpPacketError::CommandParseError(concat!("Could not parse ", stringify!($service_name), " result")))?)) + } + } + }, + )+ + } + } + } + + + pub struct $relay_type_name { + $( + [<$service_name:snake _handler>]: $service_handler_type, + )+ + } + + impl $relay_type_name { + pub fn new( + $( + [<$service_name:snake _handler>]: $service_handler_type, + )+ + ) -> Self { + Self { + $( + [<$service_name:snake _handler>], + )+ + } + } + } + + impl $crate::relay::mctp::RelayHandler for $relay_type_name { + type ServiceIdType = OdpService; + type HeaderType = OdpHeader; + type RequestEnumType = HostRequest; + type ResultEnumType = HostResult; + + fn process_request<'a>( + &'a self, + message: HostRequest, + ) -> impl core::future::Future + 'a { + async move { + match message { + $( + HostRequest::$service_name(request) => { + let result = self.[<$service_name:snake _handler>].process_request(request).await; + HostResult::$service_name(result) + } + )+ + } + } + } + } + } // end mod __odp_impl + + // Allows this generated relay type to be publicly re-exported + pub use [< _odp_impl_ $relay_type_name:snake >]::$relay_type_name; + + } // end paste! + }; // end macro arm + } // end macro + +pub use impl_odp_mctp_relay_handler; diff --git a/embedded-service/src/relay/mod.rs b/embedded-service/src/relay/mod.rs index d92cfb97f..9a0ce09d3 100644 --- a/embedded-service/src/relay/mod.rs +++ b/embedded-service/src/relay/mod.rs @@ -1,503 +1,5 @@ -//! Helper code for serialization/deserialization of arbitrary messages to/from the embedded controller via a relay service, e.g. the eSPI service. +//! Helper code for serialization/deserialization of arbitrary messages to/from the +//! embedded controller via a relay service, e.g. the eSPI service or hidi2c-target-service -/// Error type for serializing/deserializing messages -#[derive(Debug)] -#[cfg_attr(feature = "defmt", derive(defmt::Format))] -pub enum MessageSerializationError { - /// The message payload does not represent a valid message - InvalidPayload(&'static str), - - /// The message discriminant does not represent a known message type - UnknownMessageDiscriminant(u16), - - /// The provided buffer is too small to serialize the message - BufferTooSmall, - - /// Unspecified error - Other(&'static str), -} - -/// Trait for serializing and deserializing messages -pub trait SerializableMessage: Sized { - /// Serializes the message into the provided buffer. - /// On success, returns the number of bytes written - fn serialize(self, buffer: &mut [u8]) -> Result; - - /// Returns the discriminant needed to deserialize this type of message. - fn discriminant(&self) -> u16; - - /// Deserializes the message from the provided buffer. - fn deserialize(discriminant: u16, buffer: &[u8]) -> Result; -} - -// Prevent other types from implementing SerializableResult - they should instead use SerializableMessage on a Response type and an Error type -#[doc(hidden)] -mod private { - pub trait Sealed {} - - impl Sealed for Result {} -} - -/// Responses sent over MCTP are called "Results" and are of type Result where T and E both implement SerializableMessage -pub trait SerializableResult: private::Sealed + Sized { - /// The type of the result when the operation being responded to succeeded - type SuccessType: SerializableMessage; - - /// The type of the result when the operation being responded to failed - type ErrorType: SerializableMessage; - - /// Returns true if the result represents a successful operation, false otherwise - fn is_ok(&self) -> bool; - - /// Returns a unique discriminant that can be used to deserialize the specific type of result. - /// Discriminants can be reused for success and error messages. - fn discriminant(&self) -> u16; - - /// Writes the result into the provided buffer. - /// On success, returns the number of bytes written - fn serialize(self, buffer: &mut [u8]) -> Result; - - /// Attempts to deserialize the result from the provided buffer. - fn deserialize(is_error: bool, discriminant: u16, buffer: &[u8]) -> Result; -} - -impl SerializableResult for Result -where - T: SerializableMessage, - E: SerializableMessage, -{ - type SuccessType = T; - type ErrorType = E; - - fn is_ok(&self) -> bool { - Result::::is_ok(self) - } - - fn discriminant(&self) -> u16 { - match self { - Ok(success_value) => success_value.discriminant(), - Err(error_value) => error_value.discriminant(), - } - } - - fn serialize(self, buffer: &mut [u8]) -> Result { - match self { - Ok(success_value) => success_value.serialize(buffer), - Err(error_value) => error_value.serialize(buffer), - } - } - - fn deserialize(is_error: bool, discriminant: u16, buffer: &[u8]) -> Result { - if is_error { - Ok(Err(E::deserialize(discriminant, buffer)?)) - } else { - Ok(Ok(T::deserialize(discriminant, buffer)?)) - } - } -} - -pub mod mctp { - //! Contains helper functions for services that relay comms messages over MCTP - - /// Error type for MCTP relay operations - #[derive(Debug, Clone, Copy, PartialEq, Eq)] - #[cfg_attr(feature = "defmt", derive(defmt::Format))] - pub enum MctpError { - /// The endpoint ID does not correspond to a known service - UnknownEndpointId, - } - - /// Trait for types that are used by a relay service to relay messages from your service over the wire. - /// If you are implementing this trait, you should also implement RelayServiceHandler. - /// - pub trait RelayServiceHandlerTypes { - /// The request type that this service handler processes - type RequestType: super::SerializableMessage; - - /// The result type that this service handler processes - type ResultType: super::SerializableResult; - } - - /// Trait for a service that can be relayed over an external bus (e.g. battery service, thermal service, time-alarm service) - /// - pub trait RelayServiceHandler: RelayServiceHandlerTypes { - /// Process the provided request and yield a result. - fn process_request<'a>( - &'a self, - request: Self::RequestType, - ) -> impl core::future::Future + 'a; - } - - // Traits below this point are intended for consumption by relay services (e.g. the eSPI service), not individual services that want their messages relayed. - // In general, you should not implement these yourself; rather, you should leverage the `impl_odp_mctp_relay_handler` macro to do that for you. - - /// Contains additional methods that must be implemented on the relay header type. - /// Do not implement this yourself - rather, rely on the `impl_odp_mctp_relay_handler` macro to implement this. - #[doc(hidden)] - pub trait RelayHeader { - /// Return the ID of the service associated with the request - fn get_service_id(&self) -> ServiceIdType; - } - - /// Contains additional methods that must be implemented on the relay response type. - /// Do not implement this yourself - rather, rely on the `impl_odp_mctp_relay_handler` macro to implement this. - #[doc(hidden)] - pub trait RelayResponse { - /// Construct an MCTP header suitable for representing the result based on the provided service handler ID and result - fn create_header(&self, service_id: &ServiceIdType) -> HeaderType; - } - - /// Trait for aggregating collections of services that can be relayed over an external bus. - /// Do not implement this yourself - rather, rely on the `impl_odp_mctp_relay_handler` macro to implement this. - /// - pub trait RelayHandler { - /// The type that uniquely identifies individual services. Generally expected to be a C-style enum. - type ServiceIdType: Into + TryFrom + Copy; - - /// The header type used by request and result enums - type HeaderType: mctp_rs::MctpMessageHeaderTrait + RelayHeader; - - /// An enum over all possible request types - type RequestEnumType: for<'buf> mctp_rs::MctpMessageTrait<'buf, Header = Self::HeaderType>; - - /// An enum over all possible result types - type ResultEnumType: for<'buf> mctp_rs::MctpMessageTrait<'buf, Header = Self::HeaderType> - + RelayResponse; - - /// Process the provided request and yield a result. - fn process_request<'a>( - &'a self, - message: Self::RequestEnumType, - ) -> impl core::future::Future + 'a; - } - - /// This macro generates a relay type over a collection of message types, which can be used by a relay service to - /// receive messages over the wire and translate them into calls to a particular service on the EC. - /// - /// This is the recommended way to implement a relay handler - you should not implement the RelayHandler trait yourself. - /// - /// This macro will emit a type with the name you specify that is generic over a lifetime for the hardware (probably 'static in production code), - /// implements the `RelayHandler` trait, and has a single constructor method `new` that takes as arguments references to the service handler - /// types that you specify that have the 'hardware lifetime'. - /// - /// The macro takes the following inputs once: - /// relay_type_name: The name of the relay type to generate. This is arbitrary. The macro will emit a type with this name. - /// - /// Followed by a list of any number of service entries, which are specified by the following inputs: - /// service_name: A name to assign to generated identifiers associated with the service, e.g. "Battery". - /// This can be arbitrary. - /// service_id: A unique u8 that addresses that service on the EC. - /// service_handler_type: A type that implements the RelayServiceHandler trait, which will be used to process messages - /// for this service. - /// - /// Example usage: - /// - /// ```ignore - /// - /// impl_odp_mctp_relay_handler!( - /// MyRelayHandlerType; - /// Battery, 0x9, battery_service_relay::RelayHandler>; - /// TimeAlarm, 0xB, time_alarm_service_relay::RelayHandler>; - /// ); - /// - /// let relay_handler = MyRelayHandlerType::new(battery_service_instance, time_alarm_service_instance); - /// - /// // Then, pass relay_handler to your relay service (e.g. eSPI service), which should be generic over an `impl RelayHandler`. - /// - /// ``` - /// - #[macro_export] - macro_rules! impl_odp_mctp_relay_handler { - ( - $relay_type_name:ident; - $( - $service_name:ident, - $service_id:expr, - $service_handler_type:ty; - )+ - ) => { - $crate::_macro_internal::paste::paste! { - mod [< _odp_impl_ $relay_type_name:snake >] { - use $crate::_macro_internal::bitfield::bitfield; - use core::convert::Infallible; - use $crate::_macro_internal::mctp_rs::smbus_espi::SmbusEspiMedium; - use $crate::_macro_internal::mctp_rs::{MctpMedium, MctpMessageHeaderTrait, MctpMessageTrait, MctpPacketError, MctpPacketResult}; - use $crate::relay::{SerializableMessage, SerializableResult}; - use $crate::relay::mctp::RelayServiceHandler; - - #[derive(Debug, PartialEq, Eq, Clone, Copy)] - #[repr(u8)] - pub enum OdpService { - $( - $service_name = $service_id, - )+ - } - - impl From for u8 { - fn from(val: OdpService) -> u8 { - val as u8 - } - } - - impl TryFrom for OdpService { - type Error = u8; - fn try_from(value: u8) -> Result { - match value { - $( - $service_id => Ok(OdpService::$service_name), - )+ - other => Err(other), - } - } - } - - pub enum HostRequest { - $( - $service_name(<$service_handler_type as $crate::relay::mctp::RelayServiceHandlerTypes>::RequestType), - )+ - } - - impl MctpMessageTrait<'_> for HostRequest { - type Header = OdpHeader; - const MESSAGE_TYPE: u8 = 0x7D; // ODP message type - - fn serialize(self, buffer: &mut [u8]) -> MctpPacketResult { - match self { - $( - HostRequest::$service_name(request) => SerializableMessage::serialize(request, buffer) - .map_err(|_| MctpPacketError::SerializeError(concat!("Failed to serialize ", stringify!($service_name), " request"))), - )+ - } - } - - fn deserialize(header: &Self::Header, buffer: &'_ [u8]) -> MctpPacketResult { - Ok(match header.service { - $( - OdpService::$service_name => Self::$service_name( - <$service_handler_type as $crate::relay::mctp::RelayServiceHandlerTypes>::RequestType::deserialize(header.message_id, buffer) - .map_err(|_| MctpPacketError::CommandParseError(concat!("Could not parse ", stringify!($service_name), " request")))?, - ), - )+ - }) - } - } - - bitfield! { - /// Wire format for ODP MCTP headers. Not user-facing - use OdpHeader instead. - #[derive(Copy, Clone, PartialEq, Eq)] - struct OdpHeaderWireFormat(u32); - impl Debug; - impl new; - /// If true, represents a request; otherwise, represents a result - is_request, set_is_request: 25; - - /// The service ID that this message is related to - /// Note: Error checking is done when you access the field, not when you construct the OdpHeader. Take care when constructing a header. - u8, service_id, set_service_id: 23, 16; - - /// On results, indicates if the result message is an error. Unused on requests. - is_error, set_is_error: 15; - - /// The message type/discriminant - u16, message_id, set_message_id: 14, 0; - } - - #[derive(Copy, Clone, PartialEq, Eq)] - pub enum OdpMessageType { - Request, - Result { is_error: bool }, - } - - #[derive(Copy, Clone, PartialEq, Eq)] - pub struct OdpHeader { - pub message_type: OdpMessageType, - pub service: OdpService, - pub message_id: u16, - } - - impl From for OdpHeaderWireFormat { - fn from(src: OdpHeader) -> Self { - Self::new( - matches!(src.message_type, OdpMessageType::Request), - src.service.into(), - match src.message_type { - OdpMessageType::Request => false, // unused on requests - OdpMessageType::Result { is_error } => is_error, - }, - src.message_id, - ) - } - } - - impl TryFrom for OdpHeader { - type Error = MctpPacketError; - - fn try_from(src: OdpHeaderWireFormat) -> Result { - let service = OdpService::try_from(src.service_id()) - .map_err(|_| MctpPacketError::HeaderParseError("invalid odp service in odp header"))?; - - let message_type = if src.is_request() { - OdpMessageType::Request - } else { - OdpMessageType::Result { - is_error: src.is_error(), - } - }; - - Ok(OdpHeader { - message_type, - service, - message_id: src.message_id(), - }) - } - } - - impl MctpMessageHeaderTrait for OdpHeader { - fn serialize(self, buffer: &mut [u8]) -> MctpPacketResult { - let wire_format = OdpHeaderWireFormat::from(self); - let bytes = wire_format.0.to_be_bytes(); - buffer - .get_mut(0..bytes.len()) - .ok_or(MctpPacketError::SerializeError("buffer too small for odp header"))? - .copy_from_slice(&bytes); - - Ok(bytes.len()) - } - - fn deserialize(buffer: &[u8]) -> MctpPacketResult<(Self, &[u8]), M> { - let bytes = buffer - .get(0..core::mem::size_of::()) - .ok_or(MctpPacketError::HeaderParseError("buffer too small for odp header"))?; - let raw = u32::from_be_bytes( - bytes - .try_into() - .map_err(|_| MctpPacketError::HeaderParseError("buffer too small for odp header"))?, - ); - - let parsed_wire_format = OdpHeaderWireFormat(raw); - let header = OdpHeader::try_from(parsed_wire_format) - .map_err(|_| MctpPacketError::HeaderParseError("invalid odp header received"))?; - - Ok(( - header, - buffer - .get(core::mem::size_of::()..) - .ok_or(MctpPacketError::HeaderParseError("buffer too small for odp header"))?, - )) - } - } - - impl $crate::relay::mctp::RelayHeader for OdpHeader { - fn get_service_id(&self) -> OdpService { - self.service - } - } - - #[derive(Clone)] - pub enum HostResult { - $( - $service_name(<$service_handler_type as $crate::relay::mctp::RelayServiceHandlerTypes>::ResultType), - )+ - } - - impl $crate::relay::mctp::RelayResponse for HostResult { - fn create_header(&self, service_id: &OdpService) -> OdpHeader { - match (self) { - $( - (HostResult::$service_name(result)) => OdpHeader { - message_type: OdpMessageType::Result { is_error: !result.is_ok() }, - service: *service_id, - message_id: result.discriminant(), - }, - )+ - } - } - } - - impl MctpMessageTrait<'_> for HostResult { - const MESSAGE_TYPE: u8 = 0x7D; // ODP message type - type Header = OdpHeader; - - fn serialize(self, buffer: &mut [u8]) -> MctpPacketResult { - match self { - $( - HostResult::$service_name(result) => result - .serialize(buffer) - .map_err(|_| MctpPacketError::SerializeError(concat!("Failed to serialize ", stringify!($service_name), " result"))), - )+ - } - } - - fn deserialize(header: &Self::Header, buffer: &'_ [u8]) -> MctpPacketResult { - match header.service { - $( - OdpService::$service_name => { - match header.message_type { - OdpMessageType::Request => { - Err(MctpPacketError::CommandParseError(concat!("Received ", stringify!($service_name), " request when expecting result"))) - } - OdpMessageType::Result { is_error } => { - Ok(HostResult::$service_name(<$service_handler_type as $crate::relay::mctp::RelayServiceHandlerTypes>::ResultType::deserialize(is_error, header.message_id, buffer) - .map_err(|_| MctpPacketError::CommandParseError(concat!("Could not parse ", stringify!($service_name), " result")))?)) - } - } - }, - )+ - } - } - } - - - pub struct $relay_type_name { - $( - [<$service_name:snake _handler>]: $service_handler_type, - )+ - } - - impl $relay_type_name { - pub fn new( - $( - [<$service_name:snake _handler>]: $service_handler_type, - )+ - ) -> Self { - Self { - $( - [<$service_name:snake _handler>], - )+ - } - } - } - - impl $crate::relay::mctp::RelayHandler for $relay_type_name { - type ServiceIdType = OdpService; - type HeaderType = OdpHeader; - type RequestEnumType = HostRequest; - type ResultEnumType = HostResult; - - fn process_request<'a>( - &'a self, - message: HostRequest, - ) -> impl core::future::Future + 'a { - async move { - match message { - $( - HostRequest::$service_name(request) => { - let result = self.[<$service_name:snake _handler>].process_request(request).await; - HostResult::$service_name(result) - } - )+ - } - } - } - } - } // end mod __odp_impl - - // Allows this generated relay type to be publicly re-exported - pub use [< _odp_impl_ $relay_type_name:snake >]::$relay_type_name; - - } // end paste! - }; // end macro arm - } // end macro - - pub use impl_odp_mctp_relay_handler; -} +pub mod hid; +pub mod mctp; diff --git a/examples/rt685s-evk/Cargo.lock b/examples/rt685s-evk/Cargo.lock index b3a9fc1fd..4d2e8b1fb 100644 --- a/examples/rt685s-evk/Cargo.lock +++ b/examples/rt685s-evk/Cargo.lock @@ -448,7 +448,7 @@ dependencies = [ [[package]] name = "embassy-imxrt" version = "0.1.0" -source = "git+https://github.com/OpenDevicePartnership/embassy-imxrt#8b9a6f0e11d9b65f1616a9e487af2cf04aef3584" +source = "git+https://github.com/OpenDevicePartnership/embassy-imxrt#0e68fe138967b9469ecd952a702fc7e42852fe47" dependencies = [ "cfg-if", "cortex-m", @@ -669,7 +669,9 @@ dependencies = [ "defmt 0.3.100", "embassy-futures", "embassy-sync", + "generic-array", "mctp-rs", + "num_enum", "paste", "portable-atomic", "serde", @@ -774,6 +776,16 @@ dependencies = [ "windows-result", ] +[[package]] +name = "generic-array" +version = "1.4.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "fb130435a959a8d525e6bca66ff6c40981a300ee96d70e3ef56f046556d614a3" +dependencies = [ + "rustversion", + "typenum", +] + [[package]] name = "half" version = "2.7.1" @@ -810,6 +822,24 @@ version = "0.5.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "2304e00983f87ffb38b55b444b5e3b60a884b5d30c0fca7d82fe33449bbe55ea" +[[package]] +name = "hidi2c-target-service" +version = "0.1.0" +dependencies = [ + "defmt 0.3.100", + "embassy-futures", + "embassy-sync", + "embassy-time", + "embedded-hal 1.0.0", + "embedded-mcu-hal", + "embedded-services", + "generic-array", + "num_enum", + "odp-service-common", + "typenum", + "zerocopy", +] + [[package]] name = "ident_case" version = "1.0.1" @@ -1198,7 +1228,9 @@ dependencies = [ "embedded-mcu-hal", "embedded-services", "embedded-usb-pd", + "hidi2c-target-service", "mimxrt600-fcb 0.1.0", + "num_enum", "odp-service-common", "panic-probe", "platform-service", @@ -1212,6 +1244,8 @@ dependencies = [ "tps6699x", "type-c-interface", "type-c-service", + "typenum", + "zerocopy", ] [[package]] diff --git a/examples/rt685s-evk/Cargo.toml b/examples/rt685s-evk/Cargo.toml index 341e56ec0..fe383e74d 100644 --- a/examples/rt685s-evk/Cargo.toml +++ b/examples/rt685s-evk/Cargo.toml @@ -33,7 +33,7 @@ embassy-imxrt = { git = "https://github.com/OpenDevicePartnership/embassy-imxrt" ] } embassy-embedded-hal = { version = "0.6.0", features = ["defmt"] } - +embedded-mcu-hal = { version = "0.3.0", features = ["defmt"] } embassy-sync = { version = "0.8", features = ["defmt"] } embassy-executor = { version = "0.10.0", features = [ "platform-cortex-m", @@ -50,6 +50,11 @@ mimxrt600-fcb = "0.1.0" embedded-cfu-protocol = { git = "https://github.com/OpenDevicePartnership/embedded-cfu" } embedded-services = { path = "../../embedded-service", features = ["defmt"] } +hidi2c-target-service = { path = "../../hidi2c-target-service", features = [ + "defmt", +] } + +num_enum = { version = "0.7.5", default-features = false } odp-service-common = { path = "../../odp-service-common" } power-button-service = { path = "../../power-button-service", features = [ "defmt", @@ -77,6 +82,7 @@ time-alarm-service-interface = { path = "../../time-alarm-service-interface", fe time-alarm-service-relay = { path = "../../time-alarm-service-relay", features = [ "defmt", ] } +typenum = "1.20" static_cell = "2.1.0" @@ -84,7 +90,7 @@ platform-service = { path = "../../platform-service", features = [ "defmt", "imxrt685", ] } -embedded-mcu-hal = "0.3.0" +zerocopy = "0.8" cfu-service = { path = "../../cfu-service", features = ["defmt"] } diff --git a/examples/rt685s-evk/src/bin/mock_i2c_keyboard.rs b/examples/rt685s-evk/src/bin/mock_i2c_keyboard.rs new file mode 100644 index 000000000..c6c886720 --- /dev/null +++ b/examples/rt685s-evk/src/bin/mock_i2c_keyboard.rs @@ -0,0 +1,100 @@ +#![no_std] +#![no_main] + +use defmt::info; +use defmt_rtt as _; +use embassy_executor::Spawner; +use embassy_imxrt::i2c::slave::{Address, I2cSlave}; +use embassy_imxrt::i2c::{self, Async}; +use embassy_imxrt::{bind_interrupts, peripherals}; +use panic_probe as _; +use rt685s_evk_example::mocks::keyboard::{KeyCode, MockKeyboardHidRelay, MockKeyboardService}; +use static_cell::StaticCell; + +const SLAVE_ADDR: Option
= Address::new(0x15); + +bind_interrupts!(struct Irqs { + FLEXCOMM2 => i2c::InterruptHandler; +}); + +#[embassy_executor::main] +async fn main(spawner: Spawner) { + let p = embassy_imxrt::init(Default::default()); + info!("HID-I2C mock keyboard example starting..."); + let i2c = I2cSlave::new_async(p.FLEXCOMM2, p.PIO0_18, p.PIO0_17, Irqs, SLAVE_ADDR.unwrap(), p.DMA0_CH4).unwrap(); + + // GPIO on P0_28. + use embassy_imxrt::gpio; + let attn_pin = gpio::Output::new( + p.PIO0_28, + gpio::Level::High, + gpio::DriveMode::OpenDrain, + gpio::DriveStrength::Normal, + gpio::SlewRate::Standard, + ); + + static KEYBOARD_SERVICE: StaticCell = StaticCell::new(); + let keyboard_service = KEYBOARD_SERVICE.init(MockKeyboardService::new()); + + // NOTE: here's where the "aggregate HID devices" macro is currently missing. Compare with time_alarm.rs where we do this: + // + // impl_odp_mctp_relay_handler!( + // EspiRelayHandler; + // TimeAlarm, 0x0B, crate::TimeAlarmServiceRelayHandlerType; + // ); + // + // We will eventually write a macro that looks something like this: + // + // impl_hid_aggregate_device!( + // MyAggregateDevice; + // time_alarm_service_relay::hid::TimeAlarmHidRelay, + // battery_service_relay::hid::BatteryHidRelay, + // ... + // ); + // + // which will emit a type MyAggregateDevice that implements HidDevice and takes as construction parameters one instance + // of each of the handlers, in the order they were specified. + // + // For a concrete example of this pattern, see what we're doing with MCTP here: https://github.com/OpenDevicePartnership/odp-embedded-controller/blob/d6fb3ce5d9ae52ca51d6ef7b87518c6c4cb3c809/platform/platform-common/src/lib.rs#L20 + // + // This depends on writing the 'hid support library', though, so for now we're just going to directly use the time-alarm device for testing + // (pending getting actual hardware to test on, implementing I2C traits for embassy, etc). + + let mut hidsvc = odp_service_common::spawn_service!( + spawner, + hidi2c_target_service::Service< + 'static, + I2cSlave<'static, Async>, + gpio::Output<'static>, + MockKeyboardHidRelay<'static>, + >, + |resources| hidi2c_target_service::Service::new( + resources, + i2c, + attn_pin, + MockKeyboardHidRelay::new(keyboard_service), + hidi2c_target_service::HardwareVersionInfo { + vendor_id: hidi2c_target_service::VendorId::new(0x1234).unwrap(), + product_id: hidi2c_target_service::ProductId(0x5678), + version_id: hidi2c_target_service::VersionId(0x0001), + }, + hidi2c_target_service::TimeoutSettings::default() + ) + ) + .expect("Failed to spawn HID service"); + + info!("Waiting 10s before starting to send inputs"); + embassy_time::Timer::after(embassy_time::Duration::from_secs(10)).await; + + let mut i = 0; + loop { + info!("pressing key"); + keyboard_service.click_key(KeyCode::NumLock).await; + embassy_time::Timer::after(embassy_time::Duration::from_millis(2000)).await; + i += 1; + if i % 5 == 0 { + defmt::warn!("Manually triggering reset of HID service after 5 clicks to test reset handling"); + hidsvc.reset(); + } + } +} diff --git a/examples/rt685s-evk/src/bin/mock_i2c_mouse.rs b/examples/rt685s-evk/src/bin/mock_i2c_mouse.rs new file mode 100644 index 000000000..1819c4aba --- /dev/null +++ b/examples/rt685s-evk/src/bin/mock_i2c_mouse.rs @@ -0,0 +1,95 @@ +#![no_std] +#![no_main] + +use defmt::info; +use defmt_rtt as _; +use embassy_executor::Spawner; +use embassy_imxrt::i2c::slave::{Address, I2cSlave}; +use embassy_imxrt::i2c::{self, Async}; +use embassy_imxrt::{bind_interrupts, peripherals}; +use panic_probe as _; +use rt685s_evk_example::mocks::mouse::{MockMouseHidRelay, MockMouseResources, MockMouseService}; +use static_cell::StaticCell; + +const SLAVE_ADDR: Option
= Address::new(0x15); + +bind_interrupts!(struct Irqs { + FLEXCOMM2 => i2c::InterruptHandler; +}); + +#[embassy_executor::main] +async fn main(spawner: Spawner) { + let p = embassy_imxrt::init(Default::default()); + info!("HID-I2C mock mouse example starting..."); + let i2c = I2cSlave::new_async(p.FLEXCOMM2, p.PIO0_18, p.PIO0_17, Irqs, SLAVE_ADDR.unwrap(), p.DMA0_CH4).unwrap(); + + // GPIO on P0_28. + use embassy_imxrt::gpio; + let attn_pin = gpio::Output::new( + p.PIO0_28, + gpio::Level::High, + gpio::DriveMode::OpenDrain, + gpio::DriveStrength::Normal, + gpio::SlewRate::Standard, + ); + + static MOUSE_RESOURCES: StaticCell = StaticCell::new(); + let mouse_resources = MOUSE_RESOURCES.init(MockMouseResources::default()); + let (mouse_service, mut mouse_runner) = MockMouseService::new(mouse_resources); + + // NOTE: here's where the "aggregate HID devices" macro is currently missing. Compare with time_alarm.rs where we do this: + // + // impl_odp_mctp_relay_handler!( + // EspiRelayHandler; + // TimeAlarm, 0x0B, crate::TimeAlarmServiceRelayHandlerType; + // ); + // + // We will eventually write a macro that looks something like this: + // + // impl_hid_aggregate_device!( + // MyAggregateDevice; + // time_alarm_service_relay::hid::TimeAlarmHidRelay, + // battery_service_relay::hid::BatteryHidRelay, + // ... + // ); + // + // which will emit a type MyAggregateDevice that implements HidDevice and takes as construction parameters one instance + // of each of the handlers, in the order they were specified. + // + // For a concrete example of this pattern, see what we're doing with MCTP here: https://github.com/OpenDevicePartnership/odp-embedded-controller/blob/d6fb3ce5d9ae52ca51d6ef7b87518c6c4cb3c809/platform/platform-common/src/lib.rs#L20 + // + // This depends on writing the 'hid support library', though, so for now we're just going to directly use the time-alarm device for testing + // (pending getting actual hardware to test on, implementing I2C traits for embassy, etc). + + let _hidsvc = odp_service_common::spawn_service!( + spawner, + hidi2c_target_service::Service< + 'static, + I2cSlave<'static, Async>, + gpio::Output<'static>, + MockMouseHidRelay<'static>, + >, + |resources| hidi2c_target_service::Service::new( + resources, + i2c, + attn_pin, + MockMouseHidRelay::new(mouse_service), + hidi2c_target_service::HardwareVersionInfo { + vendor_id: hidi2c_target_service::VendorId::new(0x1234).unwrap(), + product_id: hidi2c_target_service::ProductId(0x5678), + version_id: hidi2c_target_service::VersionId(0x0001), + }, + hidi2c_target_service::TimeoutSettings::default() + ) + ) + .expect("Failed to spawn HID service"); + + info!("Waiting 10s before starting to send inputs"); + embassy_time::Timer::after(embassy_time::Duration::from_secs(10)).await; + + loop { + info!("clicking mouse"); + mouse_runner.send_click().await; + embassy_time::Timer::after(embassy_time::Duration::from_millis(2000)).await; + } +} diff --git a/examples/rt685s-evk/src/lib.rs b/examples/rt685s-evk/src/lib.rs index b40be19ff..df9c1115f 100644 --- a/examples/rt685s-evk/src/lib.rs +++ b/examples/rt685s-evk/src/lib.rs @@ -3,6 +3,8 @@ use mimxrt600_fcb::FlexSPIFlashConfigurationBlock; use {defmt_rtt as _, panic_probe as _}; +pub mod mocks; + #[unsafe(link_section = ".otfad")] #[used] static OTFAD: [u8; 256] = [0; 256]; diff --git a/examples/rt685s-evk/src/mocks/keyboard.rs b/examples/rt685s-evk/src/mocks/keyboard.rs new file mode 100644 index 000000000..1c6726f72 --- /dev/null +++ b/examples/rt685s-evk/src/mocks/keyboard.rs @@ -0,0 +1,269 @@ +//! Mock HID keyboard device used by the `mock_i2c_keyboard` example. +//! +//! Unlike the mouse mock, the keyboard copies each report out of a regular channel, which is simpler, +//! but can be costly for larger reports. + +use defmt::info; +use embedded_services::relay::hid::*; +use embedded_services::warn; +use zerocopy::{FromBytes, IntoBytes}; + +/// Implicit report ID used by the standalone keyboard: no report-ID item appears in +/// [`KEYBOARD_HID_REPORT_DESCRIPTOR_NO_ID`], so the transport service defaults to report ID 0. +pub const REPORTID_KEYBOARD_NONE: u8 = 0; + +/// Explicit report ID baked into [`KEYBOARD_HID_REPORT_DESCRIPTOR_WITH_ID`], used when the keyboard +/// is composed into an aggregate device (where every report ID must be explicit). +pub const REPORTID_KEYBOARD_AGGREGATE: u8 = 1; + +// This is adapted from the example keyboard HID descriptor packaged with the DT.exe tool / https://learn.microsoft.com/en-us/windows-hardware/design/component-guidelines/keyboard-collection-report-descriptor + +/// Keyboard report descriptor WITHOUT an explicit report ID. The transport service falls back to +/// report ID [`REPORTID_KEYBOARD_NONE`]. Suitable for a standalone keyboard that only exposes one +/// report. +#[rustfmt::skip] +pub const KEYBOARD_HID_REPORT_DESCRIPTOR_NO_ID: &[u8] = &[ + 0x05, 0x01, // USAGE_PAGE (Generic Desktop) + 0x09, 0x06, // USAGE (Keyboard) + 0xa1, 0x01, // COLLECTION (Application) + 0x05, 0x07, // USAGE_PAGE (Keyboard) + 0x19, 0xe0, // USAGE_MINIMUM (Keyboard LeftControl) + 0x29, 0xe7, // USAGE_MAXIMUM (Keyboard Right GUI) + 0x15, 0x00, // LOGICAL_MINIMUM (0) + 0x25, 0x01, // LOGICAL_MAXIMUM (1) + 0x75, 0x01, // REPORT_SIZE (1) + 0x95, 0x08, // REPORT_COUNT (8) + 0x81, 0x02, // INPUT (Data,Var,Abs) + 0x95, 0x01, // REPORT_COUNT (1) + 0x75, 0x08, // REPORT_SIZE (8) + 0x81, 0x03, // INPUT (Cnst,Var,Abs) + 0x95, 0x05, // REPORT_COUNT (5) + 0x75, 0x01, // REPORT_SIZE (1) + 0x05, 0x08, // USAGE_PAGE (LEDs) + 0x19, 0x01, // USAGE_MINIMUM (Num Lock) + 0x29, 0x05, // USAGE_MAXIMUM (Kana) + 0x91, 0x02, // OUTPUT (Data,Var,Abs) + 0x95, 0x01, // REPORT_COUNT (1) + 0x75, 0x03, // REPORT_SIZE (3) + 0x91, 0x03, // OUTPUT (Cnst,Var,Abs) + 0x95, 0x06, // REPORT_COUNT (6) + 0x75, 0x08, // REPORT_SIZE (8) + 0x15, 0x00, // LOGICAL_MINIMUM (0) + 0x25, 0x65, // LOGICAL_MAXIMUM (101) + 0x05, 0x07, // USAGE_PAGE (Keyboard) + 0x19, 0x00, // USAGE_MINIMUM (Reserved (no event indicated)) + 0x29, 0x65, // USAGE_MAXIMUM (Keyboard Application) + 0x81, 0x00, // INPUT (Data,Ary,Abs) + 0xc0, // END_COLLECTION +]; + +/// Keyboard report descriptor WITH an explicit report ID of [`REPORTID_KEYBOARD_AGGREGATE`]. Used +/// when composing the keyboard into an aggregate device, where report IDs must be explicit so they +/// can be renumbered to avoid collisions. +#[rustfmt::skip] +pub const KEYBOARD_HID_REPORT_DESCRIPTOR_WITH_ID: &[u8] = &[ + 0x05, 0x01, // USAGE_PAGE (Generic Desktop) + 0x09, 0x06, // USAGE (Keyboard) + 0xa1, 0x01, // COLLECTION (Application) + 0x85, REPORTID_KEYBOARD_AGGREGATE, // REPORT_ID + 0x05, 0x07, // USAGE_PAGE (Keyboard) + 0x19, 0xe0, // USAGE_MINIMUM (Keyboard LeftControl) + 0x29, 0xe7, // USAGE_MAXIMUM (Keyboard Right GUI) + 0x15, 0x00, // LOGICAL_MINIMUM (0) + 0x25, 0x01, // LOGICAL_MAXIMUM (1) + 0x75, 0x01, // REPORT_SIZE (1) + 0x95, 0x08, // REPORT_COUNT (8) + 0x81, 0x02, // INPUT (Data,Var,Abs) + 0x95, 0x01, // REPORT_COUNT (1) + 0x75, 0x08, // REPORT_SIZE (8) + 0x81, 0x03, // INPUT (Cnst,Var,Abs) + 0x95, 0x05, // REPORT_COUNT (5) + 0x75, 0x01, // REPORT_SIZE (1) + 0x05, 0x08, // USAGE_PAGE (LEDs) + 0x19, 0x01, // USAGE_MINIMUM (Num Lock) + 0x29, 0x05, // USAGE_MAXIMUM (Kana) + 0x91, 0x02, // OUTPUT (Data,Var,Abs) + 0x95, 0x01, // REPORT_COUNT (1) + 0x75, 0x03, // REPORT_SIZE (3) + 0x91, 0x03, // OUTPUT (Cnst,Var,Abs) + 0x95, 0x06, // REPORT_COUNT (6) + 0x75, 0x08, // REPORT_SIZE (8) + 0x15, 0x00, // LOGICAL_MINIMUM (0) + 0x25, 0x65, // LOGICAL_MAXIMUM (101) + 0x05, 0x07, // USAGE_PAGE (Keyboard) + 0x19, 0x00, // USAGE_MINIMUM (Reserved (no event indicated)) + 0x29, 0x65, // USAGE_MAXIMUM (Keyboard Application) + 0x81, 0x00, // INPUT (Data,Ary,Abs) + 0xc0, // END_COLLECTION +]; + +#[repr(C, packed)] +#[derive(Debug, Clone, Copy, Default, defmt::Format, zerocopy::FromBytes, zerocopy::IntoBytes, zerocopy::Immutable)] +pub struct KeyboardInputReport { + /// Left Ctrl .. Right GUI + pub modifiers: u8, + + /// Reserved byte required by boot keyboard format + pub reserved: u8, + + /// Up to 6 simultaneous key usages + pub keys: [u8; 6], +} + +#[repr(u8)] +#[derive(num_enum::IntoPrimitive, num_enum::TryFromPrimitive, Debug, Clone, Copy, defmt::Format)] +pub enum KeyCode { + NumLock = 0x53, + A = 0x04, +} + +#[repr(C, packed)] +#[derive(Debug, Clone, Copy, defmt::Format, zerocopy::FromBytes, zerocopy::IntoBytes, zerocopy::Immutable)] +pub struct KeyboardOutputReport { + /// LED state bits + pub leds: u8, +} + +/// Depth of the keyboard input-report channel. +const KEYBOARD_CHANNEL_DEPTH: usize = 5; + +/// Consumer side of the mock keyboard. Owns the channel that carries input reports and exposes a +/// method to inject key presses. +pub struct MockKeyboardService { + channel: + embassy_sync::channel::Channel, +} + +impl Default for MockKeyboardService { + fn default() -> Self { + Self::new() + } +} + +impl MockKeyboardService { + pub const fn new() -> Self { + Self { + channel: embassy_sync::channel::Channel::new(), + } + } + + pub async fn click_key(&self, key_code: KeyCode) { + // key down + let send_result = self.channel.try_send(KeyboardInputReport { + modifiers: 0, + reserved: 0, + keys: [key_code.into(), 0, 0, 0, 0, 0], + }); + + if let Err(e) = send_result { + warn!("Failed to send key down report: {:?}", e); + } + + embassy_time::Timer::after(embassy_time::Duration::from_millis(15)).await; + + // key up + let send_result = self.channel.try_send(KeyboardInputReport::default()); + if let Err(e) = send_result { + warn!("Failed to send key up report: {:?}", e); + } + } + + pub fn receiver( + &self, + ) -> embassy_sync::channel::Receiver< + '_, + embedded_services::GlobalRawMutex, + KeyboardInputReport, + KEYBOARD_CHANNEL_DEPTH, + > { + self.channel.receiver() + } +} + +/// Relay adapter that presents the mock keyboard to the HID-I2C service as a [`HidDevice`]. +/// +pub struct MockKeyboardHidRelay<'s> { + service: &'s MockKeyboardService, + report_id: ReportId, + descriptor: HidReportDescriptor<'static>, +} + +impl<'s> MockKeyboardHidRelay<'s> { + pub fn new(service: &'s MockKeyboardService) -> Self { + Self { + service, + report_id: ReportId(1), + descriptor: HidReportDescriptor::new(KEYBOARD_HID_REPORT_DESCRIPTOR_WITH_ID) + .expect("keyboard HID report descriptor should be valid"), + } + } +} + +impl embedded_services::relay::hid::HidDevice for MockKeyboardHidRelay<'_> { + type InputReportMaxSize = typenum::U8; + type OutputReportMaxSize = typenum::U1; + type FeatureReportMaxSize = typenum::U0; + + const MAX_REPORT_COUNT: u8 = 2; + + fn report_descriptor(&self) -> &HidReportDescriptor<'_> { + &self.descriptor + } + + async fn process_get_report( + &mut self, + _report_type: GetHidReportType, + report_id: ReportId, + process_report: impl AsyncFnOnce(GetHidReport<'_>) -> R, + ) -> Result { + info!("Received command to get report with ID {:?}", report_id); + if report_id == self.report_id { + let report = KeyboardInputReport::default(); + Ok(process_report(GetHidReport::Input(HidReport::new(report_id, report.as_bytes()))).await) + } else { + info!("Report ID {:?} not recognized", report_id); + Err(HidError::TriggerReset) + } + } + + async fn set_report(&mut self, report: &SetHidReport<'_>) -> Result<(), HidError> { + match report { + SetHidReport::Output(r) if r.id() == self.report_id => { + let output_report = KeyboardOutputReport::read_from_bytes(r.data()).unwrap(); + info!("Received keyboard output report: {:?}", output_report); + } + SetHidReport::Output(r) => { + info!("Report ID {:?} not recognized", r.id()); + return Err(HidError::TriggerReset); + } + SetHidReport::Feature(r) => info!("Received command to set feature report with ID {:?}", r.id()), + } + Ok(()) + } + + async fn wait_for_input_report(&mut self) { + self.service.receiver().ready_to_receive().await + } + + async fn process_next_input_report( + &mut self, + process_report: impl AsyncFnOnce(HidReport<'_>) -> R, + ) -> Result { + let input_report = self.service.receiver().receive().await; + Ok(process_report(HidReport::new(self.report_id, input_report.as_bytes())).await) + } + + fn has_pending_input_report(&mut self) -> bool { + !self.service.receiver().is_empty() + } + + async fn set_power_state(&mut self, state: HidDevicePowerState) -> Result<(), HidError> { + info!("Received command to set power state to {:?}", state); + Ok(()) + } + + async fn reset(&mut self) { + info!("Received reset command"); + self.service.receiver().clear(); + } +} diff --git a/examples/rt685s-evk/src/mocks/mod.rs b/examples/rt685s-evk/src/mocks/mod.rs new file mode 100644 index 000000000..ec4c227d3 --- /dev/null +++ b/examples/rt685s-evk/src/mocks/mod.rs @@ -0,0 +1,4 @@ +//! Mock HID sub-devices shared by the `mock_i2c_*` examples. + +pub mod keyboard; +pub mod mouse; diff --git a/examples/rt685s-evk/src/mocks/mouse.rs b/examples/rt685s-evk/src/mocks/mouse.rs new file mode 100644 index 000000000..da115a9b9 --- /dev/null +++ b/examples/rt685s-evk/src/mocks/mouse.rs @@ -0,0 +1,249 @@ +//! Mock HID mouse device used by the `mock_i2c_mouse` example +//! +//! This demonstrates a zero-copy input path: input reports live in a caller-provided ring buffer +//! and are written/read *in place, which saves a memcpy at the cost of some complexity. + +use defmt::info; +use embassy_sync::zerocopy_channel; +use embedded_services::GlobalRawMutex; +use embedded_services::relay::hid::*; +use zerocopy::IntoBytes; + +// This is adapted from the example mouse HID descriptor packaged with the DT.exe tool / https://learn.microsoft.com/en-us/windows-hardware/design/component-guidelines/mouse-collection-report-descriptor +const REPORTID_MOUSE: u8 = 1; + +#[rustfmt::skip] +const MOUSE_HID_REPORT_DESCRIPTOR: &[u8] = &[ + 0x05, 0x01, // Usage Page (Generic Desktop Ctrls) + 0x09, 0x02, // Usage (Mouse) + 0xA1, 0x01, // Collection (Application) + 0x85, REPORTID_MOUSE, // REPORT_ID (Touch pad) **** THIS IS ADAPTED FROM SAMPLE TOUCHPAD DESCRIPTOR, THE MOUSE EXAMPLE OMITTED IT BECAUSSE IT ONLY HAD 1 REPORT + 0x09, 0x01, // Usage (Pointer) + 0xA1, 0x00, // Collection (Physical) + 0x05, 0x09, // Usage Page (Button) + 0x19, 0x01, // Usage Minimum (0x01) + 0x29, 0x03, // Usage Maximum (0x03) + 0x15, 0x00, // Logical Minimum (0) + 0x25, 0x01, // Logical Maximum (1) + 0x95, 0x03, // Report Count (3) + 0x75, 0x01, // Report Size (1) + 0x81, 0x02, // Input (Data,Var,Abs,No Wrap,Linear,Preferred State,No Null Position) + 0x95, 0x01, // Report Count (1) + 0x75, 0x05, // Report Size (5) + 0x81, 0x03, // Input (Const,Var,Abs,No Wrap,Linear,Preferred State,No Null Position) + 0x05, 0x01, // Usage Page (Generic Desktop Ctrls) + 0x09, 0x30, // Usage (X) + 0x09, 0x31, // Usage (Y) + 0x15, 0x81, // Logical Minimum (-127) + 0x25, 0x7F, // Logical Maximum (127) + 0x75, 0x08, // Report Size (8) + 0x95, 0x02, // Report Count (2) + 0x81, 0x06, // Input (Data,Var,Rel,No Wrap,Linear,Preferred State,No Null Position) + 0xC0, // End Collection + 0xC0, // End Collection +]; + +const MOUSE_BUTTON_1: u8 = 0x01; +#[allow(dead_code)] +const MOUSE_BUTTON_2: u8 = 0x02; +#[allow(dead_code)] +const MOUSE_BUTTON_3: u8 = 0x04; + +#[repr(C)] +#[derive(Debug, Default, defmt::Format, zerocopy::FromBytes, zerocopy::IntoBytes, zerocopy::Immutable)] +struct MouseReport { + buttons: u8, // 3 bits used for buttons, 5 bits padding + x: i8, + y: i8, +} + +// Number of in-flight input reports the zero-copy channel can hold. Depth >= 2 lets the producer +// stage the next report while the consumer is still handing the current one to the host. +const MOUSE_CHANNEL_DEPTH: usize = 4; + +type MouseChannel<'d> = zerocopy_channel::Channel<'d, GlobalRawMutex, MouseReport>; +type MouseSender<'d> = zerocopy_channel::Sender<'d, GlobalRawMutex, MouseReport>; +type MouseReceiver<'d> = zerocopy_channel::Receiver<'d, GlobalRawMutex, MouseReport>; + +/// Memory resources for the mock mouse service. +/// +/// Following the pattern used by services like `time_alarm_service` and `hidi2c_target_service`, the +/// caller allocates a single `Resources` object (for example in a `StaticCell`) and hands a mutable +/// borrow to [`MockMouseService::new`] at construction time, instead of scattering `static` +/// singletons around. The struct is generic over the lifetime `'d` of that borrow rather than being +/// pinned to `'static`. +pub struct MockMouseResources<'d> { + /// Backing storage for the zero-copy channel's ring buffer. + buf: [MouseReport; MOUSE_CHANNEL_DEPTH], + /// The channel itself, created in [`MockMouseService::new`] from a borrow of `buf`. + channel: Option>, +} + +impl Default for MockMouseResources<'_> { + fn default() -> Self { + Self { + buf: core::array::from_fn(|_| MouseReport::default()), + channel: None, + } + } +} + +/// Consumer side of the mock mouse. Owns the channel `Receiver` and hands the host borrows that +/// point directly into the ring buffer — no intermediate copy of the report payload. +pub struct MockMouseService<'hw> { + receiver: MouseReceiver<'hw>, +} + +impl<'hw> MockMouseService<'hw> { + /// Wire up the zero-copy channel inside `resources` and split it into the service (which the + /// relay borrows) and a runner (used to inject mouse events). + pub fn new(resources: &'hw mut MockMouseResources<'hw>) -> (Self, MockMouseRunner<'hw>) { + // Destructure so the buffer and channel fields are borrowed disjointly: the channel borrows + // the buffer in place, and both live as long as `resources`. + let MockMouseResources { buf, channel } = resources; + let channel = channel.insert(MouseChannel::new(buf)); + let (sender, receiver) = channel.split(); + (Self { receiver }, MockMouseRunner { sender }) + } + + // Event receiver for the service. If you truly need this sort of zero-copy approach all the way from the service through the relay adapter to the tranport, + // you may have to make some layout concessions in the service. This implies that it's probably not possible to support zerocopy for a given message type + // across multiple tranports that have different layout requirements, and some knowledge of which layout to use may need to leak into the service for + // services that truly need zero-copy messages. This is probably not going to be common, though - in the case of a mouse, for example, the biggest report is + // 3 bytes, so the copy is trivial and the service in practice would just use a regular channel rather than a zerocopy channel. + fn receiver(&mut self) -> &mut MouseReceiver<'hw> { + &mut self.receiver + } +} + +/// In a production case, this would implement the Runner trait and be spawned as a task, but for the mock we just expose methods to inject mouse events +pub struct MockMouseRunner<'d> { + sender: MouseSender<'d>, +} + +impl MockMouseRunner<'_> { + /// Acquire the next free slot in the ring buffer and populate it in place, then publish it. + async fn send(&mut self, report: MouseReport) { + // `send()` waits for a free slot and yields `&mut MouseReport` pointing straight into the + // channel's ring buffer. Writing through it avoids copying the payload into the channel. + let slot = self.sender.send().await; + *slot = report; + // Publish the slot to the consumer. Nothing is copied here either. + self.sender.send_done(); + } + + pub async fn send_click(&mut self) { + // Mouse down + self.send(MouseReport { + buttons: MOUSE_BUTTON_1, + x: 0, + y: 0, + }) + .await; + + embassy_time::Timer::after(embassy_time::Duration::from_millis(15)).await; + + // Mouse up + self.send(MouseReport { buttons: 0, x: 0, y: 0 }).await; + } + + pub async fn move_mouse(&mut self) { + self.send(MouseReport { + buttons: MOUSE_BUTTON_1, + x: 10, + y: 10, + }) + .await; + } +} + +/// Relay adapter that presents the mock mouse to the HID-I2C service as a [`HidDevice`]. +pub struct MockMouseHidRelay<'d> { + service: MockMouseService<'d>, + descriptor: HidReportDescriptor<'static>, +} + +impl<'d> MockMouseHidRelay<'d> { + pub fn new(service: MockMouseService<'d>) -> Self { + Self { + service, + descriptor: HidReportDescriptor::new(MOUSE_HID_REPORT_DESCRIPTOR) + .expect("mouse HID report descriptor should be valid"), + } + } +} + +impl embedded_services::relay::hid::HidDevice for MockMouseHidRelay<'_> { + type InputReportMaxSize = typenum::U3; + type OutputReportMaxSize = typenum::U0; + type FeatureReportMaxSize = typenum::U0; + + const MAX_REPORT_COUNT: u8 = 3; + + fn report_descriptor(&self) -> &HidReportDescriptor<'_> { + &self.descriptor + } + + async fn process_get_report( + &mut self, + _report_type: GetHidReportType, + report_id: ReportId, + process_report: impl AsyncFnOnce(GetHidReport<'_>) -> R, + ) -> Result { + info!("Received command to get report with ID {:?}", report_id); + match report_id { + ReportId(REPORTID_MOUSE) => { + let report = MouseReport::default(); + Ok(process_report(GetHidReport::Input(HidReport::new(report_id, report.as_bytes()))).await) + } + _ => { + info!("Report ID {:?} not recognized", report_id); + Err(HidError::TriggerReset) + } + } + } + + async fn set_report(&mut self, report: &SetHidReport<'_>) -> Result<(), HidError> { + match report { + SetHidReport::Output(r) => info!("Received command to set output report with ID {:?}", r.id()), + SetHidReport::Feature(r) => info!("Received command to set feature report with ID {:?}", r.id()), + } + info!("SET_REPORT NOT IMPLEMENTED"); + Ok(()) + } + + async fn wait_for_input_report(&mut self) { + // `receive()` only peeks at the front slot - it doesn't treat the sample as consumed until `receive_done()` is called. + // Therefore, we can do this to wait until a report is ready, and then immediately drop it without losing the report. + let _ = self.service.receiver().receive().await; + } + + async fn process_next_input_report( + &mut self, + process_report: impl AsyncFnOnce(HidReport<'_>) -> R, + ) -> Result { + // Borrow the next report out of the channel and lend it to the transport for the duration of `process_report`. + // This is probably unnecessary for mice because of how small the reports are, but it demonstrates the technique. + // For a mouse it may make more sense to use a traditional Channel and just copy the 3 bytes around. + let slot = self.service.receiver().receive().await; + let result = process_report(HidReport::new(ReportId(REPORTID_MOUSE), slot.as_bytes())).await; + + // The transport is done with the borrow, so return the slot to the producer. This is required by zerocopy_channel. + self.service.receiver().receive_done(); + Ok(result) + } + + fn has_pending_input_report(&mut self) -> bool { + !self.service.receiver().is_empty() + } + + async fn set_power_state(&mut self, state: HidDevicePowerState) -> Result<(), HidError> { + info!("Received command to set power state to {:?}", state); + Ok(()) + } + + async fn reset(&mut self) { + info!("Received reset command"); + self.service.receiver().clear(); + } +} diff --git a/examples/std/Cargo.lock b/examples/std/Cargo.lock index 822e6e9df..ef87dd0c6 100644 --- a/examples/std/Cargo.lock +++ b/examples/std/Cargo.lock @@ -616,11 +616,14 @@ dependencies = [ "critical-section", "embassy-futures", "embassy-sync", + "generic-array", "log", "mctp-rs", + "num_enum", "paste", "portable-atomic", "serde", + "typenum", ] [[package]] @@ -717,6 +720,16 @@ dependencies = [ "windows-result", ] +[[package]] +name = "generic-array" +version = "1.4.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ab4e5aa225bc56696909483320f0ff9b600f1a971b52e07a17d70f3d9b43254b" +dependencies = [ + "rustversion", + "typenum", +] + [[package]] name = "hash32" version = "0.3.1" @@ -1440,6 +1453,12 @@ dependencies = [ "type-c-interface", ] +[[package]] +name = "typenum" +version = "1.20.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b6f5e870be6c3b371b77fe0ee0bafb859fa4964b4404c27de1d380043c4dda20" + [[package]] name = "ufmt-write" version = "0.1.0" diff --git a/hidi2c-target-service/Cargo.toml b/hidi2c-target-service/Cargo.toml new file mode 100644 index 000000000..38f0ead95 --- /dev/null +++ b/hidi2c-target-service/Cargo.toml @@ -0,0 +1,28 @@ +[package] +name = "hidi2c-target-service" +version.workspace = true +edition.workspace = true +rust-version.workspace = true +license.workspace = true +repository.workspace = true + +[dependencies] +defmt = { workspace = true, optional = true } + +embassy-futures.workspace = true +embassy-sync.workspace = true +embassy-time.workspace = true +embedded-hal.workspace = true +embedded-mcu-hal.workspace = true +embedded-services.workspace = true +generic-array.workspace = true +num_enum.workspace = true +odp-service-common.workspace = true +typenum.workspace = true +zerocopy = { workspace = true, features = ["derive"] } + +[features] +defmt = ["dep:defmt", "embedded-mcu-hal/defmt", "embedded-services/defmt"] + +[lints] +workspace = true diff --git a/hidi2c-target-service/src/attn_pin_handler.rs b/hidi2c-target-service/src/attn_pin_handler.rs new file mode 100644 index 000000000..7171eeb02 --- /dev/null +++ b/hidi2c-target-service/src/attn_pin_handler.rs @@ -0,0 +1,45 @@ +use crate::*; + +/// Handler for the ATTN pin, which is used to signal the host that we have an input report ready to be read. +/// This is a simple wrapper around an OutputPin that tracks whether we've asserted the interrupt or not, because +/// OutputPin doesn't have a built-in way to interrogate its own state. +/// +pub struct AttnPinHandler { + attn_pin: AttnPin, + asserted: bool, +} + +impl AttnPinHandler { + /// Construct a new handler that owns the provided GPIO hardware + pub fn new(attn_pin: AttnPin) -> Self { + let mut result = Self { + attn_pin, + asserted: false, + }; + result.clear_interrupt(); + result + } + + /// Clear the interrupt, which is done by setting the pin high. + pub fn clear_interrupt(&mut self) { + trace!("HID-I2C: ATTN: clear interrupt"); + self.asserted = false; + self.attn_pin + .set_high() + .unwrap_or_else(|_| error!("HID-I2C: Failed to clear interrupt on attn pin")); + } + + /// Assert the interrupt, which is done by pulling the pin low. + pub fn assert_interrupt(&mut self) { + trace!("HID-I2C: ATTN: assert interrupt"); + self.asserted = true; + self.attn_pin + .set_low() + .unwrap_or_else(|_| error!("HID-I2C: Failed to assert interrupt on attn pin")); + } + + /// Returns true if we are asserting the interrupt, false otherwise. + pub fn asserted(&self) -> bool { + self.asserted + } +} diff --git a/hidi2c-target-service/src/constrained_hid_device.rs b/hidi2c-target-service/src/constrained_hid_device.rs new file mode 100644 index 000000000..464799d3d --- /dev/null +++ b/hidi2c-target-service/src/constrained_hid_device.rs @@ -0,0 +1,43 @@ +use generic_array::ArrayLength; +use typenum::Max; + +mod sealed { + /// Traits that derive from this one are not allowed to be implemented by 3rd party code. + /// To have those traits implemented, you should satisfy the requirement for their blanket + /// implementation instead. + pub trait Sealed {} +} + +/// Extension of [`embedded_services::relay::hid::HidDevice`] that computes the max of feature/input and +/// feature/output report sizes as associated types so we can correctly size our +/// send/recv buffers. +/// +/// Any type that implements `embedded_services::relay::hid::HidDevice` will automatically implement this trait - +/// there's no type that satisfies ArrayLength that doesn't also satisfy these trait bounds. +/// However, due to some limitations in the Rust type system, we have to spell it out. +/// +/// We should be able to get rid of all of this once generic_const_exprs stabilises, since +/// then we don't need any of these trait bounds and can just do the math where we declare +/// the buffers. At that point, we should also consider moving from ArrayLength to just +/// const usizes since ArraySize is just a workaround for the lack of generic const expressions. +/// +pub trait ConstrainedHidDevice: embedded_services::relay::hid::HidDevice + sealed::Sealed { + /// `max(FeatureReportMaxSize, InputReportMaxSize)`. + type MaxInputOrFeatureSize: ArrayLength; + /// `max(FeatureReportMaxSize, OutputReportMaxSize)`. + type MaxOutputOrFeatureSize: ArrayLength; +} + +impl ConstrainedHidDevice for T +where + T: embedded_services::relay::hid::HidDevice, + T::FeatureReportMaxSize: Max, + T::FeatureReportMaxSize: Max, + >::Output: ArrayLength, + >::Output: ArrayLength, +{ + type MaxInputOrFeatureSize = >::Output; + type MaxOutputOrFeatureSize = >::Output; +} + +impl sealed::Sealed for T where T: ConstrainedHidDevice {} diff --git a/hidi2c-target-service/src/device_descriptor.rs b/hidi2c-target-service/src/device_descriptor.rs new file mode 100644 index 000000000..e86fbe43f --- /dev/null +++ b/hidi2c-target-service/src/device_descriptor.rs @@ -0,0 +1,121 @@ +use embedded_services::relay::hid; +use typenum::marker_traits::Unsigned; + +/// HID descriptor as specified in section 5.1 of the HID-I2C spec. Not to be confused with a HID report descriptor, which +/// expresses the report types that the HID device can handle. Field descriptions are taken directly from the HID-I2C spec. +#[repr(C)] +#[derive(Clone, Copy, Debug, Default, PartialEq, Eq, zerocopy::FromBytes, zerocopy::IntoBytes, zerocopy::Immutable)] +#[cfg_attr(feature = "defmt", derive(defmt::Format))] +pub struct DeviceDescriptor { + /// The length, in unsigned bytes, of the complete Hid Descriptor + w_hid_desc_length: u16, + + /// The version number, in binary coded decimal (BCD) format. DEVICE should default to 0x0100 + bcd_version: u16, + + /// The length, in unsigned bytes, of the Report Descriptor. + w_report_desc_length: u16, + + /// The register index containing the Report Descriptor on the DEVICE. + w_report_desc_register: u16, + + /// This field identifies, in unsigned bytes, the register number to read the input report from the DEVICE. + w_input_register: u16, + + /// This field identifies in unsigned bytes the length of the largest Input Report to be read from the Input Register (Complex HID Devices will need various sized reports). + w_max_input_length: u16, + + /// This field identifies, in unsigned bytes, the register number to send the output report to the DEVICE. + w_output_register: u16, + + /// This field identifies in unsigned bytes the length of the largest output Report to be sent to the Output Register (Complex HID Devices will need various sized reports). + w_max_output_length: u16, + + /// This field identifies, in unsigned bytes, the register number to send command requests to the DEVICE + w_command_register: u16, + + /// This field identifies in unsigned bytes the register number to exchange data with the Command Request + w_data_register: u16, + + /// This field identifies the DEVICE manufacturers Vendor ID. Must be non-zero. + w_vendor_id: u16, + + /// This field identifies the DEVICE’s unique model / Product ID. + w_product_id: u16, + + /// This field identifies the DEVICE’s firmware revision number. + w_version_id: u16, + + /// This field is reserved and should be set to 0. + reserved: [u8; 4], +} + +/// Hardware identifiers for the HID-I2C device +pub struct HardwareVersionInfo { + pub vendor_id: VendorId, + pub product_id: ProductId, + pub version_id: VersionId, +} + +/// Vendor ID, as assigned by the USB Implementers Forum (USB-IF). Must be non-zero. +pub struct VendorId(u16); +impl VendorId { + /// Creates a new VendorId. Returns None if the vendor_id is invalid (i.e. zero). + pub const fn new(vendor_id: u16) -> Option { + if vendor_id == 0 { None } else { Some(Self(vendor_id)) } + } + + /// The numeric value of the Vendor ID. + pub const fn value(&self) -> u16 { + self.0 + } +} + +/// Product ID, as assigned by the device manufacturer. +pub struct ProductId(pub u16); + +/// Version ID, as assigned by the device manufacturer. Recommended to be in BCD format, e.g. 0x0100 for version 1.00. +pub struct VersionId(pub u16); + +/// The number of bytes in a HID report header, which consists of a 2-byte length field. +pub const HID_REPORT_HEADER_SIZE_BYTES: u16 = 2; + +/// The number of bytes in a HID report ID field, which consists of a 1-byte report ID. +/// This field is only used if more than one report of any type is exposed by the HID device (i.e. you can +/// have a single input report, a single output report, and a single feature report and not need this, but +/// as soon as you add a second of any one of those you need this). +pub const HID_REPORT_ID_SIZE_BYTES: u16 = 1; + +impl DeviceDescriptor { + pub fn new(hid_device: &HidDevice, hwinfo: HardwareVersionInfo) -> Self { + const HID_I2C_PROTOCOL_VERSION: u16 = 0x0100; + Self { + w_hid_desc_length: core::mem::size_of::() as u16, + bcd_version: HID_I2C_PROTOCOL_VERSION, + w_report_desc_length: hid_device.report_descriptor().as_bytes().len() as u16, + w_report_desc_register: crate::HidI2cRegister::ReportDescriptor as u16, + w_input_register: crate::HidI2cRegister::Input.into(), + w_max_input_length: HidDevice::InputReportMaxSize::USIZE as u16 + + HID_REPORT_HEADER_SIZE_BYTES + + if hid_device.report_descriptor().report_ids_implicit() { + 0 + } else { + HID_REPORT_ID_SIZE_BYTES + }, + w_output_register: crate::HidI2cRegister::Output.into(), + w_max_output_length: HidDevice::OutputReportMaxSize::USIZE as u16 + + HID_REPORT_HEADER_SIZE_BYTES + + if hid_device.report_descriptor().report_ids_implicit() { + 0 + } else { + HID_REPORT_ID_SIZE_BYTES + }, + w_command_register: crate::HidI2cRegister::Command.into(), + w_data_register: crate::HidI2cRegister::Data.into(), + w_vendor_id: hwinfo.vendor_id.value(), + w_product_id: hwinfo.product_id.0, + w_version_id: hwinfo.version_id.0, + reserved: [0; 4], + } + } +} diff --git a/hidi2c-target-service/src/error.rs b/hidi2c-target-service/src/error.rs new file mode 100644 index 000000000..7f1208d35 --- /dev/null +++ b/hidi2c-target-service/src/error.rs @@ -0,0 +1,56 @@ +use embassy_time::TimeoutError; +use embedded_services::relay::hid::HidError; + +// HID errors +#[derive(Clone, Copy, Debug)] +#[cfg_attr(feature = "defmt", derive(defmt::Format))] +pub(crate) enum ProtocolError { + /// Invalid data + InvalidData, + /// Invalid size + InvalidSize, + /// Invalid register address + InvalidRegisterAddress, + /// Invalid command + InvalidCommand, + /// Invalid report type for command + InvalidReportType, + /// Timeout + Timeout, +} + +#[allow(dead_code)] // Dead code analysis ignores Debug, which is what we want the detail for +#[derive(Clone, Copy, Debug)] +#[cfg_attr(feature = "defmt", derive(defmt::Format))] +pub(crate) enum Error { + /// Error from the underlying bus + Bus(BusError), + /// HID protocol error + Protocol(ProtocolError), + /// Error reported from our underlying HidDevice in response to a request from us + Device(HidError), +} + +impl From for Error { + fn from(err: ProtocolError) -> Self { + Error::Protocol(err) + } +} + +impl From for Error { + fn from(err: HidError) -> Self { + Error::Device(err) + } +} + +impl From for Error { + fn from(_: generic_array::LengthError) -> Self { + Error::Protocol(ProtocolError::InvalidSize) + } +} + +impl From for Error { + fn from(_: TimeoutError) -> Self { + Error::Protocol(ProtocolError::Timeout) + } +} diff --git a/hidi2c-target-service/src/lib.rs b/hidi2c-target-service/src/lib.rs new file mode 100644 index 000000000..20c65f6b7 --- /dev/null +++ b/hidi2c-target-service/src/lib.rs @@ -0,0 +1,50 @@ +//! This crate contains a service that behaves as a HID target/slave device over I2C. + +#![no_std] + +mod device_descriptor; +use device_descriptor::DeviceDescriptor; +pub use device_descriptor::{HardwareVersionInfo, ProductId, VendorId, VersionId}; + +mod attn_pin_handler; +use attn_pin_handler::AttnPinHandler; + +mod error; +use error::*; + +mod constrained_hid_device; +pub use constrained_hid_device::ConstrainedHidDevice; + +mod service; +pub use service::{Runner, Service, TimeoutSettings}; + +use embedded_services::{error, info, trace, warn}; + +/// HID-I2C register addresses as specified in section 5.1 of the HID-I2C spec. +/// These specific values are our convention, not from the HID-I2C spec, but section 4.2 indicates +/// that all HID-I2C devices must have their own I2C bus address so there's no way to share a single +/// I2C address by leveraging different register addresses on the same I2C address. +/// +#[repr(u16)] +#[derive(num_enum::TryFromPrimitive, num_enum::IntoPrimitive, Debug, Clone, Copy)] +#[cfg_attr(feature = "defmt", derive(defmt::Format))] +enum HidI2cRegister { + /// HID descriptor register - see section 5.1 + /// NOTE: Per the HID-I2C spec, when using ACPI for enumeration, this value needs to be put in the _DSM. + DeviceDescriptor = 0x01, + + /// HID report descriptor register - see section 5.2 + ReportDescriptor = 0x02, + + /// Input report register - see section 6.1 + Input = 0x03, + + /// Output report register - see section 6.2 + Output = 0x04, + + /// Command register - see section 7.1.1 + Command = 0x05, + + /// Data register - see section 7.1.2 + Data = 0x06, +} diff --git a/hidi2c-target-service/src/service.rs b/hidi2c-target-service/src/service.rs new file mode 100644 index 000000000..18bb4c4e5 --- /dev/null +++ b/hidi2c-target-service/src/service.rs @@ -0,0 +1,699 @@ +use crate::*; + +use core::marker::PhantomData; +use embassy_time::{Duration, with_timeout}; +use embedded_mcu_hal::i2c::target::asynch::I2c as I2cTargetAsync; +use embedded_mcu_hal::i2c::target::{ReadStatus, Request, WriteStatus}; +use embedded_services::relay::hid; +use embedded_services::relay::hid::{GetHidReportType, HidError, HidReport, SetHidReport}; +use zerocopy::IntoBytes; + +/// HID-I2C Command Opcode as specified in section 7.1.1 of the HID-I2C spec +#[repr(u8)] +#[derive(num_enum::TryFromPrimitive, num_enum::IntoPrimitive, Debug, Clone, Copy)] +#[cfg_attr(feature = "defmt", derive(defmt::Format))] +enum Opcode { + // Reserved: 0x00 + Reset = 0x01, + GetReport = 0x02, + SetReport = 0x03, + + // The following commands are in the i2c spec but are listed as optional and + // "not sent by modern hosts", and therefore we do not implement them: + // + // GetIdle = 0x04, + // SetIdle = 0x05, + // GetProtocol = 0x06, + // SetProtocol = 0x07, + SetPower = 0x08, + // Reserved: 0x09 - 0x0D + // VendorReserved = 0x0E, + // Reserved: 0x0F +} + +/// I2C wire format representation for HID power states. +#[repr(u8)] +#[derive(num_enum::TryFromPrimitive, num_enum::IntoPrimitive, Debug, Clone, Copy)] +#[cfg_attr(feature = "defmt", derive(defmt::Format))] +enum I2cPowerState { + On = 0x00, + Sleep = 0x01, +} + +impl From for hid::HidDevicePowerState { + fn from(value: I2cPowerState) -> Self { + match value { + I2cPowerState::On => hid::HidDevicePowerState::On, + I2cPowerState::Sleep => hid::HidDevicePowerState::Sleep, + } + } +} + +#[derive(Debug, Clone, Copy)] +#[cfg_attr(feature = "defmt", derive(defmt::Format))] +enum HidI2cReportType { + Input, + Output, + Feature, +} + +impl TryFrom for GetHidReportType { + type Error = ProtocolError; + + fn try_from(value: HidI2cReportType) -> Result { + match value { + HidI2cReportType::Input => Ok(GetHidReportType::Input), + HidI2cReportType::Feature => Ok(GetHidReportType::Feature), + HidI2cReportType::Output => Err(ProtocolError::InvalidReportType), + } + } +} + +struct HidI2cReportCommandHeader { + /// The report type that this command is targeting + report_type: HidI2cReportType, + + /// The report ID that this command is targeting, or None if another byte must be read to get the full report ID (happens if report ID is >= 0xF) + report_id: Option, +} + +impl HidI2cReportCommandHeader { + fn try_from_command_byte(command_byte: u8) -> Result { + const HID_I2C_REPORT_TYPE_OFFSET: u8 = 4; + let report_type = match command_byte >> HID_I2C_REPORT_TYPE_OFFSET { + 0x01 => HidI2cReportType::Input, + 0x02 => HidI2cReportType::Output, + 0x03 => HidI2cReportType::Feature, + _ => return Err(ProtocolError::InvalidReportType), + }; + let report_id = if command_byte & 0x0F == 0x0F { + None + } else { + Some(embedded_services::relay::hid::ReportId(command_byte & 0x0F)) + }; + Ok(Self { report_type, report_id }) + } +} + +/// Resources used by the service +struct InnerResources { + reset_signal: embassy_sync::signal::Signal, +} + +/// Memory required for the HID-I2C target service. +pub struct Resources { + inner: Option, + + // We don't currently need these to be shared between the runner and the service, but we may in the future, + // and being generic over them now means that we can move stuff in here later without a breaking interface change. + _phantom: PhantomData<(Bus, AttnPin, HidDevice)>, +} + +impl Default + for Resources +{ + fn default() -> Self { + Self { + inner: None, + _phantom: PhantomData, + } + } +} + +/// Wrapper for the I2C trait that automatically handles timeouts and recovery +struct TimeoutBus { + bus: Bus, + + timeout_settings: TimeoutSettings, +} + +impl TimeoutBus { + /// Wait for the next controller-initiated event with no timeout. + fn listen_indefinitely(&mut self) -> impl core::future::Future> + '_ { + self.bus.listen() + } + + /// Wait for the controller to address us mid-transaction, applying the device-response timeout + /// and skipping repeated-start edges. + async fn listen_for_response(&mut self) -> Result> { + loop { + let result = with_timeout(self.timeout_settings.device_response_timeout, self.bus.listen()).await?; + let result = result.map_err(Error::Bus)?; + if let Request::RepeatedStart(_a) = result { + continue; + } + + return Ok(result); + } + } + + /// Read bytes the host is writing to us, applying the data-read timeout and recovering the bus on failure. + async fn read(&mut self, buffer: &mut [u8]) -> Result> { + match with_timeout( + self.timeout_settings.data_read_timeout, + self.bus.respond_to_write(buffer), + ) + .await + { + // Timed out waiting for the controller to drive the transfer. + Err(_timeout_error) => { + error!("Read request timeout"); + self.bus.recover().await.map_err(Error::Bus)?; + Err(Error::Protocol(ProtocolError::Timeout)) + } + // Controller finished writing; report how many bytes we drained. + Ok(Ok( + status @ (WriteStatus::Stopped(bytes) | WriteStatus::Restarted(bytes) | WriteStatus::BufferFull(bytes)), + )) => { + trace!("Host issued write command: {:?}", status); + Ok(bytes) + } + // Some other write status we don't expect while reading. + Ok(Ok(status)) => { + error!("Unexpected write status: {:?}", status); + Err(Error::Protocol(ProtocolError::InvalidData)) + } + // The bus peripheral itself reported an error. + Ok(Err(e)) => { + error!("Error during bus read"); + Err(Error::Bus(e)) + } + } + } + + /// Write all of `buffer` to the host, padding with zeros if the host asks for more bytes. + async fn write(&mut self, buffer: &[u8]) -> Result<(), Error> { + let mut write_buffer: &[u8] = buffer; + const PADDING_BUFFER: &[u8] = &[0u8; 8]; + while self.write_unterminated(write_buffer).await? { + write_buffer = PADDING_BUFFER; + trace!("Emitting a padding byte"); + } + Ok(()) + } + + /// Write `buffer` to the host; returns true if the host requested more bytes than we provided. + async fn write_unterminated(&mut self, buffer: &[u8]) -> Result> { + match with_timeout( + self.timeout_settings.device_response_timeout, + self.bus.respond_to_read(buffer), + ) + .await + { + Err(_timeout_error) => { + error!("Write request timeout"); + self.bus.recover().await.map_err(Error::Bus)?; + Err(Error::Protocol(ProtocolError::Timeout)) + } + Ok(result) => result + .map(|read_status| match read_status { + ReadStatus::NeedMore(_) => { + trace!("host requested more bytes than we provided"); + true + } + _ => false, + }) + .map_err(Error::Bus), + } + } +} + +/// Service runner for the HID-I2C service. You must call run() on the runner to drive the service. +pub struct Runner<'hw, Bus: I2cTargetAsync, AttnPin: embedded_hal::digital::OutputPin, HidDevice: ConstrainedHidDevice> +{ + bus: TimeoutBus, + attn_pin: AttnPinHandler, + hid_device: HidDevice, + device_descriptor: DeviceDescriptor, + + /// Buffer for receiving messages. + write_buf: generic_array::GenericArray, + + /// True if a reset has been triggered but not yet acknowledged by the host + pending_reset: bool, + + resources: &'hw InnerResources, +} + +impl< + 'hw, + Bus: I2cTargetAsync + 'hw, + AttnPin: embedded_hal::digital::OutputPin + 'hw, + HidDevice: ConstrainedHidDevice + 'hw, +> odp_service_common::runnable_service::ServiceRunner<'hw> for Runner<'hw, Bus, AttnPin, HidDevice> +{ + async fn run(mut self) -> embedded_services::Never { + loop { + let event = { + // If we've raised the interrupt, we know it won't be dismissed again until it's serviced by the host reading + // the input report, so we don't need to listen for another notification. + let input_report_ready_future = async { + if self.attn_pin.asserted() { + core::future::pending().await + } else { + self.hid_device.wait_for_input_report().await + } + }; + embassy_futures::select::select3( + self.bus.listen_indefinitely(), + input_report_ready_future, + self.resources.reset_signal.wait(), + ) + .await + }; + match event { + embassy_futures::select::Either3::First(bus_request) => { + trace!("HID-I2C: Processing request from host"); + match bus_request { + Ok(request) => { + self.process_request(request).await; + } + Err(bus_error) => { + error!( + "HID-I2C: Error during bus operation: {:?}", + embedded_mcu_hal::i2c::target::Error::kind(&bus_error) + ); + } + } + } + embassy_futures::select::Either3::Second(()) => { + trace!("HID-I2C: Signalling host that an input report is ready"); + self.attn_pin.assert_interrupt(); + } + embassy_futures::select::Either3::Third(()) => { + trace!("HID-I2C: Received reset request"); + self.reset().await; + } + } + } + } +} + +impl< + 'hw, + Bus: I2cTargetAsync + 'hw, + AttnPin: embedded_hal::digital::OutputPin + 'hw, + HidDevice: ConstrainedHidDevice + 'hw, +> Runner<'hw, Bus, AttnPin, HidDevice> +{ + async fn process_request(&mut self, request: Request) { + // TODO unlike the old trait where the address was fixed, this one can get multiple addresses. + // We may need to have some way to split the bus resources across multiple logical I2C devices, + // perhaps some sort of "I2cSocket" abstraction built on top of the I2cTargetAsync trait that can + // be used to scope the addressing to a single device or something. + // + // For now, assume that there's only one address on the bus and it's us. This will explode spectacularly + // if that's not the case, though, so we'll need to revisit this at some point. + // + let result = match request { + Request::Write(_address) => { + trace!("HID-I2C: Processing register access"); + self.process_register_access().await + } + Request::Read(_address) => { + trace!("HID-I2C: Processing request for input report"); + self.reply_with_input_report().await + } + _ => { + trace!("HID-I2C: Ignoring command type {:?}", request); + return; + } + }; + + match result { + Ok(_) => {} + Err(Error::Bus(bus_error)) => { + error!( + "HID-I2C: Error during bus operation: {:?}", + embedded_mcu_hal::i2c::target::Error::kind(&bus_error) + ); + } + Err(Error::Protocol(protocol_error)) => { + error!("HID-I2C: Protocol error during bus operation: {:?}", protocol_error); + } + Err(Error::Device(HidError::TriggerReset)) => { + warn!("HID-I2C: HID device requested device-initiated reset"); + self.reset().await; + } + } + } + + async fn process_register_access(&mut self) -> Result<(), Error> { + let mut reg = [0u8; 2]; + self.bus.read(&mut reg).await?; + + let register = HidI2cRegister::try_from(u16::from_le_bytes(reg)) + .map_err(|_| Error::Protocol(ProtocolError::InvalidRegisterAddress))?; + + info!("HID-I2C: Host requested to access register {:?}", register); + match register { + HidI2cRegister::DeviceDescriptor => { + let request = self.bus.listen_for_response().await?; + match request { + Request::Read(_address) => { + trace!( + "Responding to request for device descriptor with {} bytes", + self.device_descriptor.as_bytes().len() + ); + self.bus.write(self.device_descriptor.as_bytes()).await?; + + Ok(()) + } + _ => { + error!( + "Expected read request after device descriptor register access: {:?}", + request + ); + Err(Error::Protocol(ProtocolError::InvalidRegisterAddress)) + } + } + } + HidI2cRegister::ReportDescriptor => match self.bus.listen_for_response().await? { + Request::Read(_address) => { + trace!("Responding to request for report descriptor"); + self.bus.write(self.hid_device.report_descriptor().as_bytes()).await?; + Ok(()) + } + _ => { + error!("Expected read request after report descriptor register access"); + Err(Error::Protocol(ProtocolError::InvalidRegisterAddress)) + } + }, + HidI2cRegister::Input => self.process_input_report_read().await, + HidI2cRegister::Output => self.process_output_report_write().await, + HidI2cRegister::Command => self.process_command().await, + HidI2cRegister::Data => { + error!( + "HID-I2C: Got read to Data register without a preceding write to the Command register; this is unexpected and may indicate a bug in the service." + ); + Err(Error::Protocol(ProtocolError::InvalidRegisterAddress)) + } + } + } + + /// Process a request for an input report that we've asserted an interrupt for (i.e. not a request for a specific input report ID) + async fn process_input_report_read(&mut self) -> Result<(), Error> { + info!("Processing normal input report request"); + let read_request = self.bus.listen_for_response().await?; + if let Request::Read(_address) = read_request { + self.reply_with_input_report().await + } else { + error!( + "Expected read request after input report register access, got {:?}", + read_request + ); + Err(Error::Protocol(ProtocolError::InvalidCommand)) + } + } + + // Respond to the host with the next input report. + async fn reply_with_input_report(&mut self) -> Result<(), Error> { + if self.pending_reset { + info!("HID-I2C: Processing first input report read after reset"); + // We need to acknowledge that we've completed a reset by writing back 0's - see section 7.2.1 of the HID spec + self.bus.write(&[00, 00]).await?; + + self.pending_reset = false; + self.attn_pin.clear_interrupt(); + return Ok(()); + } + + // If the host reads the input register when we have no report queued, return an empty report. + // In general, this should not happen (the host should only poll us when we've asserted the interrupt, + // which we only do when we have a report ready), but if it does due to e.g. a host-side race condition, + // we'll stall the I2C bus if we don't respond. + // + if !self.hid_device.has_pending_input_report() { + warn!("HID-I2C: Host polled when no input report was pending; responding with zero-length report"); + self.bus.write(&[00, 00]).await?; + self.attn_pin.clear_interrupt(); + return Ok(()); + } + + let report_ids_implicit = self.hid_device.report_descriptor().report_ids_implicit(); + self.hid_device + .process_next_input_report(async |report| { + info!("HID-I2C: Got report to return - listening to bus for read request"); + + let size_bytes = report.data().len() as u16 + + device_descriptor::HID_REPORT_HEADER_SIZE_BYTES + + if report_ids_implicit { + 0 + } else { + device_descriptor::HID_REPORT_ID_SIZE_BYTES + }; + let [size_low, size_high] = size_bytes.to_le_bytes(); + + let header_slice: &[u8] = if report_ids_implicit { + &[size_low, size_high] + } else { + &[size_low, size_high, report.id().0] + }; + + trace!( + "Responding with input report {}: {:x} {:x}", + report.id(), + header_slice, + report.data() + ); + self.bus.write_unterminated(header_slice).await?; + self.bus.write(report.data()).await?; + Ok::<(), Error>(()) + }) + .await??; + + if !self.hid_device.has_pending_input_report() { + self.attn_pin.clear_interrupt(); + } + + Ok(()) + } + + async fn process_output_report_write(&mut self) -> Result<(), Error> { + let mut write_header_buf = [0u8; (device_descriptor::HID_REPORT_HEADER_SIZE_BYTES + + device_descriptor::HID_REPORT_ID_SIZE_BYTES) as usize]; + + // NOTE - if we're using implicit report IDs, we present that to HidDevice implementations as report ID 0. + let header_len = if self.hid_device.report_descriptor().report_ids_implicit() { + write_header_buf.len() - (device_descriptor::HID_REPORT_ID_SIZE_BYTES as usize) + } else { + write_header_buf.len() + }; + + let header_buf_slice = write_header_buf + .get_mut(..header_len) + .ok_or(Error::Protocol(ProtocolError::InvalidSize))?; + + self.bus.read(header_buf_slice).await?; + + let [len_low, len_high, report_id] = write_header_buf; + let length = u16::from_le_bytes([len_low, len_high]) as usize - header_len; // Note: per HID spec, the length field needs to include its own length (2 bytes) and the report ID (1 byte) + trace!("Reading {} bytes", length); + + let read_result = self.bus.read(&mut self.write_buf).await?; + + if read_result != length { + error!("Expected to read {} bytes but got {}", length, read_result); + return Err(Error::Protocol(ProtocolError::InvalidSize)); + } + + let output_report = embedded_services::relay::hid::SetHidReport::Output(HidReport::new( + embedded_services::relay::hid::ReportId(report_id), + self.write_buf + .get(..length) + .ok_or(Error::Protocol(ProtocolError::InvalidSize))?, + )); + + self.hid_device.set_report(&output_report).await?; + + Ok(()) + } + + async fn get_command_report_header( + &mut self, + command_byte: u8, + ) -> Result<(HidI2cReportType, embedded_services::relay::hid::ReportId), Error> { + let command_header = HidI2cReportCommandHeader::try_from_command_byte(command_byte)?; + let report_id = if let Some(report_id) = command_header.report_id { + report_id + } else { + let mut report_id = 0u8; + self.bus.read(core::slice::from_mut(&mut report_id)).await?; + embedded_services::relay::hid::ReportId(report_id) + }; + + Ok((command_header.report_type, report_id)) + } + + async fn process_command(&mut self) -> Result<(), Error> { + let [command_byte, opcode_byte] = { + let mut command_header_buffer = [0u8; 2]; + self.bus.read(&mut command_header_buffer).await?; + command_header_buffer + }; + + match Opcode::try_from(opcode_byte).map_err(|_| Error::Protocol(ProtocolError::InvalidCommand))? { + Opcode::Reset => { + warn!("HID-I2C: Host requested device reset"); + Err(Error::Device(HidError::TriggerReset)) + } + + Opcode::SetPower => { + trace!("Processing set power command"); + let power_state = I2cPowerState::try_from(command_byte) + .map_err(|_| Error::Protocol(ProtocolError::InvalidCommand))?; + self.hid_device.set_power_state(power_state.into()).await?; + Ok(()) + } + + Opcode::GetReport => { + trace!("Processing get report command"); + + let (report_type, report_id) = self.get_command_report_header(command_byte).await?; + self.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) + let len_header = (report.data().len() as u16 + device_descriptor::HID_REPORT_HEADER_SIZE_BYTES) + .to_le_bytes(); + self.bus.write(&len_header).await?; + self.bus.write(report.data()).await?; + Ok::<(), Error>(()) + }) + .await??; + + Ok(()) + } + + Opcode::SetReport => { + trace!("Processing set report command"); + let (report_type, report_id) = self.get_command_report_header(command_byte).await?; + let mut len_header = [0u8; core::mem::size_of::()]; + self.bus.read(&mut len_header).await?; + + // Note: per HID spec, the length field relayed over the wire needs to include its own length (2 bytes) + let report_size = + (u16::from_le_bytes(len_header) - device_descriptor::HID_REPORT_HEADER_SIZE_BYTES) as usize; + self.bus + .read( + self.write_buf + .get_mut(..report_size) + .ok_or(Error::Protocol(ProtocolError::InvalidSize))?, + ) + .await?; + + let set_report = match report_type { + HidI2cReportType::Input => { + error!("Host attempted to send us an input report, which is invalid"); + return Err(Error::Protocol(ProtocolError::InvalidReportType)); + } + HidI2cReportType::Output => SetHidReport::Output(HidReport::new( + report_id, + self.write_buf + .get(..report_size) + .ok_or(Error::Protocol(ProtocolError::InvalidSize))?, + )), + HidI2cReportType::Feature => SetHidReport::Feature(HidReport::new( + report_id, + self.write_buf + .get(..report_size) + .ok_or(Error::Protocol(ProtocolError::InvalidSize))?, + )), + }; + + self.hid_device.set_report(&set_report).await?; + + Ok(()) + } + } + } + + async fn reset(&mut self) { + warn!("HID-I2C: Executing device reset"); + self.hid_device.reset().await; + self.pending_reset = true; + self.attn_pin.assert_interrupt(); + } +} + +/// Control handle for an instance of the HID-I2C service, which presents a HID-I2C device over an (I2C bus, interrupt line) tuple +#[derive(Clone, Copy)] +pub struct Service<'hw, Bus: I2cTargetAsync, AttnPin: embedded_hal::digital::OutputPin, HidDevice: ConstrainedHidDevice> +{ + resources: &'hw InnerResources, + _phantom: core::marker::PhantomData<(Bus, AttnPin, HidDevice)>, +} + +impl< + 'hw, + Bus: I2cTargetAsync + 'hw, + AttnPin: embedded_hal::digital::OutputPin + 'hw, + HidDevice: ConstrainedHidDevice + 'hw, +> Service<'hw, Bus, AttnPin, HidDevice> +{ + /// Creates a new instance of the HID-I2C service and its associated runner. + /// You must call run() on the runner to drive the service. Consider using + /// this in conjunction with `odp_service_common::runnable_service::spawn_service!()` + pub async fn new( + storage: &'hw mut Resources, + bus: Bus, + attn_pin: AttnPin, + hid_device: HidDevice, + hwinfo: HardwareVersionInfo, + timeout_settings: TimeoutSettings, + ) -> Result<(Self, Runner<'hw, Bus, AttnPin, HidDevice>), core::convert::Infallible> { + let device_descriptor = DeviceDescriptor::new(&hid_device, hwinfo); + + let resources = storage.inner.insert(InnerResources { + reset_signal: embassy_sync::signal::Signal::new(), + }); + + Ok(( + Service { + resources, + _phantom: PhantomData, + }, + Runner { + bus: TimeoutBus { bus, timeout_settings }, + attn_pin: AttnPinHandler::new(attn_pin), + hid_device, + device_descriptor, + write_buf: generic_array::GenericArray::default(), + pending_reset: false, // The host is responsible for explicitly resetting us at boot, so we start in a non-reset state + resources, + }, + )) + } + + /// Causes the HID service to perform a device-initiated reset. + pub fn reset(&mut self) { + self.resources.reset_signal.signal(()); + } +} + +impl< + 'hw, + Bus: I2cTargetAsync + 'hw, + AttnPin: embedded_hal::digital::OutputPin + 'hw, + HidDevice: ConstrainedHidDevice + 'hw, +> odp_service_common::runnable_service::Service<'hw> for Service<'hw, Bus, AttnPin, HidDevice> +{ + type Runner = Runner<'hw, Bus, AttnPin, HidDevice>; + type Resources = Resources; +} + +/// Timeout configuration for I2C operations +pub struct TimeoutSettings { + /// 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 TimeoutSettings { + fn default() -> Self { + Self { + device_response_timeout: Duration::from_secs(1), + data_read_timeout: Duration::from_secs(1), + } + } +} diff --git a/thermal-service-relay/src/serialization.rs b/thermal-service-relay/src/serialization.rs index 6b1a7a63d..ac4d84871 100644 --- a/thermal-service-relay/src/serialization.rs +++ b/thermal-service-relay/src/serialization.rs @@ -1,5 +1,5 @@ use crate::DeciKelvin; -use embedded_services::relay::{MessageSerializationError, SerializableMessage}; +use embedded_services::relay::mctp::{MessageSerializationError, SerializableMessage}; // Standard MPTF requests expected by the thermal subsystem #[derive(num_enum::IntoPrimitive, num_enum::TryFromPrimitive, Copy, Clone, Debug, PartialEq)] diff --git a/time-alarm-service-relay/src/serialization.rs b/time-alarm-service-relay/src/serialization.rs index bc77f46e7..c581412da 100644 --- a/time-alarm-service-relay/src/serialization.rs +++ b/time-alarm-service-relay/src/serialization.rs @@ -1,5 +1,5 @@ use core::array::TryFromSliceError; -use embedded_services::relay::{MessageSerializationError, SerializableMessage}; +use embedded_services::relay::mctp::{MessageSerializationError, SerializableMessage}; use time_alarm_service_interface::{ AcpiDaylightSavingsTimeStatus, AcpiTimerId, AcpiTimestamp, AlarmExpiredWakePolicy, AlarmTimerSeconds, TimeAlarmDeviceCapabilities, TimerStatus, From fdc5d726cb9b3d3cbb26e7915efac5348b9f9c82 Mon Sep 17 00:00:00 2001 From: Billy Price Date: Wed, 22 Jul 2026 13:47:25 -0700 Subject: [PATCH 02/17] fix cargolock --- examples/pico-de-gallo/Cargo.lock | 18 ++++++++++++++++++ examples/std/Cargo.lock | 1 - 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/examples/pico-de-gallo/Cargo.lock b/examples/pico-de-gallo/Cargo.lock index 22f6b393f..8305d30a7 100644 --- a/examples/pico-de-gallo/Cargo.lock +++ b/examples/pico-de-gallo/Cargo.lock @@ -439,8 +439,10 @@ dependencies = [ "critical-section", "embassy-futures", "embassy-sync", + "generic-array", "log", "mctp-rs", + "num_enum", "paste", "portable-atomic", "serde", @@ -531,6 +533,16 @@ dependencies = [ "windows-result", ] +[[package]] +name = "generic-array" +version = "1.4.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ab4e5aa225bc56696909483320f0ff9b600f1a971b52e07a17d70f3d9b43254b" +dependencies = [ + "rustversion", + "typenum", +] + [[package]] name = "hash32" version = "0.2.1" @@ -1344,6 +1356,12 @@ dependencies = [ "syn", ] +[[package]] +name = "typenum" +version = "1.20.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b6f5e870be6c3b371b77fe0ee0bafb859fa4964b4404c27de1d380043c4dda20" + [[package]] name = "unicode-ident" version = "1.0.24" diff --git a/examples/std/Cargo.lock b/examples/std/Cargo.lock index ef87dd0c6..708717939 100644 --- a/examples/std/Cargo.lock +++ b/examples/std/Cargo.lock @@ -623,7 +623,6 @@ dependencies = [ "paste", "portable-atomic", "serde", - "typenum", ] [[package]] From aa8ef145da064466eb5b70e91e22bf98efa02003 Mon Sep 17 00:00:00 2001 From: Billy Price Date: Wed, 22 Jul 2026 14:06:08 -0700 Subject: [PATCH 03/17] fix logging on linux --- Cargo.lock | 1 + hidi2c-target-service/Cargo.toml | 2 ++ 2 files changed, 3 insertions(+) diff --git a/Cargo.lock b/Cargo.lock index 586ed7330..69f943bb2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1086,6 +1086,7 @@ dependencies = [ "embedded-mcu-hal", "embedded-services", "generic-array", + "log", "num_enum", "odp-service-common", "typenum", diff --git a/hidi2c-target-service/Cargo.toml b/hidi2c-target-service/Cargo.toml index 38f0ead95..5bc0fa40a 100644 --- a/hidi2c-target-service/Cargo.toml +++ b/hidi2c-target-service/Cargo.toml @@ -8,6 +8,7 @@ repository.workspace = true [dependencies] defmt = { workspace = true, optional = true } +log = { workspace = true, optional = true } embassy-futures.workspace = true embassy-sync.workspace = true @@ -23,6 +24,7 @@ zerocopy = { workspace = true, features = ["derive"] } [features] defmt = ["dep:defmt", "embedded-mcu-hal/defmt", "embedded-services/defmt"] +log = ["dep:log", "embedded-services/log"] [lints] workspace = true From cc28a787370303155f6bc8b8986224c75b7515f1 Mon Sep 17 00:00:00 2001 From: Billy Price Date: Wed, 22 Jul 2026 14:16:38 -0700 Subject: [PATCH 04/17] log/machete fixes --- hidi2c-target-service/Cargo.toml | 3 +++ hidi2c-target-service/src/service.rs | 8 -------- 2 files changed, 3 insertions(+), 8 deletions(-) diff --git a/hidi2c-target-service/Cargo.toml b/hidi2c-target-service/Cargo.toml index 5bc0fa40a..bbb07c1ac 100644 --- a/hidi2c-target-service/Cargo.toml +++ b/hidi2c-target-service/Cargo.toml @@ -6,6 +6,9 @@ rust-version.workspace = true license.workspace = true repository.workspace = true +[package.metadata.cargo-machete] +ignored = ["log"] + [dependencies] defmt = { workspace = true, optional = true } log = { workspace = true, optional = true } diff --git a/hidi2c-target-service/src/service.rs b/hidi2c-target-service/src/service.rs index 18bb4c4e5..42f259c6e 100644 --- a/hidi2c-target-service/src/service.rs +++ b/hidi2c-target-service/src/service.rs @@ -433,8 +433,6 @@ impl< let report_ids_implicit = self.hid_device.report_descriptor().report_ids_implicit(); self.hid_device .process_next_input_report(async |report| { - info!("HID-I2C: Got report to return - listening to bus for read request"); - let size_bytes = report.data().len() as u16 + device_descriptor::HID_REPORT_HEADER_SIZE_BYTES + if report_ids_implicit { @@ -450,12 +448,6 @@ impl< &[size_low, size_high, report.id().0] }; - trace!( - "Responding with input report {}: {:x} {:x}", - report.id(), - header_slice, - report.data() - ); self.bus.write_unterminated(header_slice).await?; self.bus.write(report.data()).await?; Ok::<(), Error>(()) From a5910a6c766a835ad05574eba0301ece9faf4288 Mon Sep 17 00:00:00 2001 From: Billy Price Date: Wed, 22 Jul 2026 14:40:07 -0700 Subject: [PATCH 05/17] cargo vet --- supply-chain/audits.toml | 5 +++++ supply-chain/imports.lock | 5 ++++- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/supply-chain/audits.toml b/supply-chain/audits.toml index 888dde1bd..70d8d3e17 100644 --- a/supply-chain/audits.toml +++ b/supply-chain/audits.toml @@ -736,6 +736,11 @@ criteria = "safe-to-deploy" delta = "0.8.7 -> 0.8.8" notes = "Delta audit: replaces heavy windows crate with lighter windows-link/windows-result + auto-generated bindings (windows-bindgen 0.65.0). Fixes fn-ptr-to-int casts for strict provenance. Improves TLS optimization bug workaround on macOS/Windows (compiler_fence + black_box replacing double thread::current hack). No new unsafe patterns; all changes consistent with stackful coroutine purpose. Assisted-by: GitHub Copilot:claude-opus-4.6" +[[audits.generic-array]] +who = "Billy Price " +criteria = "safe-to-deploy" +version = "1.4.2" + [[audits.gimli]] who = "Robert Zieba " criteria = "safe-to-run" diff --git a/supply-chain/imports.lock b/supply-chain/imports.lock index aa2565852..1dfd685e9 100644 --- a/supply-chain/imports.lock +++ b/supply-chain/imports.lock @@ -196,7 +196,10 @@ aggregated-from = "https://raw.githubusercontent.com/OpenDevicePartnership/embas who = "Felipe Balbi " criteria = "safe-to-deploy" version = "0.5.0" -aggregated-from = "https://raw.githubusercontent.com/OpenDevicePartnership/tps6699x/refs/heads/main/supply-chain/audits.toml" +aggregated-from = [ + "https://raw.githubusercontent.com/OpenDevicePartnership/tps6699x/refs/heads/main/supply-chain/audits.toml", + "https://raw.githubusercontent.com/OpenDevicePartnership/tps6699x/refs/heads/main/supply-chain/audits.toml", +] [[audits.OpenDevicePartnership.audits.embassy-time-queue-utils]] who = "Felipe Balbi " From f8a303f4b499fdd66e2c33efb546d730e3f0d066 Mon Sep 17 00:00:00 2001 From: Billy Price Date: Thu, 23 Jul 2026 12:30:54 -0700 Subject: [PATCH 06/17] pr feedback --- embedded-service/src/relay/hid.rs | 2 +- hidi2c-target-service/src/attn_pin_handler.rs | 4 ++-- hidi2c-target-service/src/constrained_hid_device.rs | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/embedded-service/src/relay/hid.rs b/embedded-service/src/relay/hid.rs index 9d06c602a..ab4701efe 100644 --- a/embedded-service/src/relay/hid.rs +++ b/embedded-service/src/relay/hid.rs @@ -28,7 +28,7 @@ pub enum HidDevicePowerState { Off, } -/// A HID report of no more than X bytes +/// A HID report pub struct HidReport<'buf> { id: ReportId, diff --git a/hidi2c-target-service/src/attn_pin_handler.rs b/hidi2c-target-service/src/attn_pin_handler.rs index 7171eeb02..c7c0f1c8b 100644 --- a/hidi2c-target-service/src/attn_pin_handler.rs +++ b/hidi2c-target-service/src/attn_pin_handler.rs @@ -23,19 +23,19 @@ impl AttnPinHandler { /// Clear the interrupt, which is done by setting the pin high. pub fn clear_interrupt(&mut self) { trace!("HID-I2C: ATTN: clear interrupt"); - self.asserted = false; self.attn_pin .set_high() .unwrap_or_else(|_| error!("HID-I2C: Failed to clear interrupt on attn pin")); + self.asserted = false; } /// Assert the interrupt, which is done by pulling the pin low. pub fn assert_interrupt(&mut self) { trace!("HID-I2C: ATTN: assert interrupt"); - self.asserted = true; self.attn_pin .set_low() .unwrap_or_else(|_| error!("HID-I2C: Failed to assert interrupt on attn pin")); + self.asserted = true; } /// Returns true if we are asserting the interrupt, false otherwise. diff --git a/hidi2c-target-service/src/constrained_hid_device.rs b/hidi2c-target-service/src/constrained_hid_device.rs index 464799d3d..3d48709a5 100644 --- a/hidi2c-target-service/src/constrained_hid_device.rs +++ b/hidi2c-target-service/src/constrained_hid_device.rs @@ -40,4 +40,4 @@ where type MaxOutputOrFeatureSize = >::Output; } -impl sealed::Sealed for T where T: ConstrainedHidDevice {} +impl sealed::Sealed for T where T: embedded_services::relay::hid::HidDevice {} From f5c70bee7887c22ae1669063caa95a6b4e2a9844 Mon Sep 17 00:00:00 2001 From: Billy Price Date: Thu, 23 Jul 2026 12:31:48 -0700 Subject: [PATCH 07/17] pr feedback --- hidi2c-target-service/src/service.rs | 29 ++++++++++++++++++++++------ 1 file changed, 23 insertions(+), 6 deletions(-) diff --git a/hidi2c-target-service/src/service.rs b/hidi2c-target-service/src/service.rs index 42f259c6e..2666c2b65 100644 --- a/hidi2c-target-service/src/service.rs +++ b/hidi2c-target-service/src/service.rs @@ -340,7 +340,11 @@ impl< async fn process_register_access(&mut self) -> Result<(), Error> { let mut reg = [0u8; 2]; - self.bus.read(&mut reg).await?; + let read = self.bus.read(&mut reg).await?; + if read != reg.len() { + error!("Expected to read {} bytes but got {}", reg.len(), read); + return Err(Error::Protocol(ProtocolError::InvalidData)); + } let register = HidI2cRegister::try_from(u16::from_le_bytes(reg)) .map_err(|_| Error::Protocol(ProtocolError::InvalidRegisterAddress))?; @@ -476,10 +480,16 @@ impl< .get_mut(..header_len) .ok_or(Error::Protocol(ProtocolError::InvalidSize))?; - self.bus.read(header_buf_slice).await?; + let header_read = self.bus.read(header_buf_slice).await?; + if header_read != header_len { + error!("Expected to read {} bytes but got {}", header_len, header_read); + return Err(Error::Protocol(ProtocolError::InvalidSize)); + } let [len_low, len_high, report_id] = write_header_buf; - let length = u16::from_le_bytes([len_low, len_high]) as usize - header_len; // Note: per HID spec, the length field needs to include its own length (2 bytes) and the report ID (1 byte) + let length = (u16::from_le_bytes([len_low, len_high]) as usize) + .checked_sub(header_len) + .ok_or(Error::Protocol(ProtocolError::InvalidSize))?; // Note: per HID spec, the length field needs to include its own length (2 bytes) and the report ID (1 byte) trace!("Reading {} bytes", length); let read_result = self.bus.read(&mut self.write_buf).await?; @@ -560,11 +570,18 @@ impl< trace!("Processing set report command"); let (report_type, report_id) = self.get_command_report_header(command_byte).await?; let mut len_header = [0u8; core::mem::size_of::()]; - self.bus.read(&mut len_header).await?; + + let header_read = self.bus.read(&mut len_header).await?; + if header_read != len_header.len() { + error!("Expected to read {} bytes but got {}", len_header.len(), header_read); + return Err(Error::Protocol(ProtocolError::InvalidSize)); + } // Note: per HID spec, the length field relayed over the wire needs to include its own length (2 bytes) - let report_size = - (u16::from_le_bytes(len_header) - device_descriptor::HID_REPORT_HEADER_SIZE_BYTES) as usize; + let report_size = (u16::from_le_bytes(len_header) + .checked_sub(device_descriptor::HID_REPORT_HEADER_SIZE_BYTES)) + .ok_or(Error::Protocol(ProtocolError::InvalidSize))? as usize; + self.bus .read( self.write_buf From 1d8805b4fc1b92accb3e523d4367be2732b59743 Mon Sep 17 00:00:00 2001 From: Billy Price Date: Mon, 27 Jul 2026 12:38:07 -0700 Subject: [PATCH 08/17] pr feedback --- embedded-service/src/relay/hid.rs | 2 ++ hidi2c-target-service/src/attn_pin_handler.rs | 3 ++- hidi2c-target-service/src/service.rs | 6 ++++++ 3 files changed, 10 insertions(+), 1 deletion(-) diff --git a/embedded-service/src/relay/hid.rs b/embedded-service/src/relay/hid.rs index ab4701efe..28f262b12 100644 --- a/embedded-service/src/relay/hid.rs +++ b/embedded-service/src/relay/hid.rs @@ -9,6 +9,7 @@ use num_enum::TryFromPrimitive; /// rather than swallowing them. #[derive(Clone, Copy, Debug, PartialEq, Eq)] #[cfg_attr(feature = "defmt", derive(defmt::Format))] +#[non_exhaustive] pub enum HidError { /// The operation has failed and a device-initiated reset should be triggered. TriggerReset, @@ -162,6 +163,7 @@ pub trait HidDevice { /// Blocks until the device is ready to yield an unsolicited input report. /// When this returns, the next call to process_next_input_report should be able to run without blocking on I/O. + /// Must be cancellation-safe. fn wait_for_input_report(&mut self) -> impl core::future::Future; /// Returns true if there is a pending input report that can be retrieved immediately with process_next_input_report(). diff --git a/hidi2c-target-service/src/attn_pin_handler.rs b/hidi2c-target-service/src/attn_pin_handler.rs index c7c0f1c8b..c08ebe049 100644 --- a/hidi2c-target-service/src/attn_pin_handler.rs +++ b/hidi2c-target-service/src/attn_pin_handler.rs @@ -2,7 +2,8 @@ use crate::*; /// Handler for the ATTN pin, which is used to signal the host that we have an input report ready to be read. /// This is a simple wrapper around an OutputPin that tracks whether we've asserted the interrupt or not, because -/// OutputPin doesn't have a built-in way to interrogate its own state. +/// OutputPin doesn't have a built-in way to interrogate its own state. There is a StatefulOutputPin trait that +/// does have that functionality, but not all pins support it so we just implement it ourselves here. /// pub struct AttnPinHandler { attn_pin: AttnPin, diff --git a/hidi2c-target-service/src/service.rs b/hidi2c-target-service/src/service.rs index 2666c2b65..bf07e4aca 100644 --- a/hidi2c-target-service/src/service.rs +++ b/hidi2c-target-service/src/service.rs @@ -335,6 +335,12 @@ impl< warn!("HID-I2C: HID device requested device-initiated reset"); self.reset().await; } + Err(Error::Device(hid_error)) => { + error!( + "HID-I2C: non-resetting HID device error during bus operation: {:?}", + hid_error + ); + } } } From 7c3ed497c114a3ff8232c01a71afe7b7d6d14e5b Mon Sep 17 00:00:00 2001 From: Billy Price Date: Mon, 27 Jul 2026 14:11:47 -0700 Subject: [PATCH 09/17] pr feedback - register addr r/w --- hidi2c-target-service/src/service.rs | 31 ++++++++++++++++++++++++---- 1 file changed, 27 insertions(+), 4 deletions(-) diff --git a/hidi2c-target-service/src/service.rs b/hidi2c-target-service/src/service.rs index bf07e4aca..612ed60e4 100644 --- a/hidi2c-target-service/src/service.rs +++ b/hidi2c-target-service/src/service.rs @@ -517,7 +517,10 @@ impl< Ok(()) } - async fn get_command_report_header( + /// Attempts to parse the provided command byte as an input / output command header, including reading any more bytes if + /// needed and verifying that the host followed the command header with the expected write to the data register address. + /// Upon success, callers should proceed to read or write the requested payload to/from the bus. + async fn get_io_command_report_header( &mut self, command_byte: u8, ) -> Result<(HidI2cReportType, embedded_services::relay::hid::ReportId), Error> { @@ -530,6 +533,26 @@ impl< embedded_services::relay::hid::ReportId(report_id) }; + let mut data_register_address = [0u8; core::mem::size_of::()]; + let data_register_address_read = self.bus.read(&mut data_register_address).await?; + if data_register_address_read != data_register_address.len() { + error!( + "Expected to read {} bytes but got {}", + data_register_address.len(), + data_register_address_read + ); + return Err(Error::Protocol(ProtocolError::InvalidSize)); + } + + if u16::from_le_bytes(data_register_address) != HidI2cRegister::Data as u16 { + error!( + "Expected the host to write the data register address after the header ({:?}) but got {:?}", + HidI2cRegister::Data, + u16::from_le_bytes(data_register_address) + ); + return Err(Error::Protocol(ProtocolError::InvalidRegisterAddress)); + } + Ok((command_header.report_type, report_id)) } @@ -557,7 +580,7 @@ impl< Opcode::GetReport => { trace!("Processing get report command"); - let (report_type, report_id) = self.get_command_report_header(command_byte).await?; + let (report_type, report_id) = self.get_io_command_report_header(command_byte).await?; self.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) @@ -574,9 +597,9 @@ impl< Opcode::SetReport => { trace!("Processing set report command"); - let (report_type, report_id) = self.get_command_report_header(command_byte).await?; - let mut len_header = [0u8; core::mem::size_of::()]; + let (report_type, report_id) = self.get_io_command_report_header(command_byte).await?; + let mut len_header = [0u8; core::mem::size_of::()]; let header_read = self.bus.read(&mut len_header).await?; if header_read != len_header.len() { error!("Expected to read {} bytes but got {}", len_header.len(), header_read); From d8e3a0919f638639c895f39524aaac161ef308db Mon Sep 17 00:00:00 2001 From: Billy Price Date: Mon, 27 Jul 2026 14:21:33 -0700 Subject: [PATCH 10/17] pr feedback --- .../src/device_descriptor.rs | 23 ++++++++----------- 1 file changed, 9 insertions(+), 14 deletions(-) diff --git a/hidi2c-target-service/src/device_descriptor.rs b/hidi2c-target-service/src/device_descriptor.rs index e86fbe43f..e8a9511bd 100644 --- a/hidi2c-target-service/src/device_descriptor.rs +++ b/hidi2c-target-service/src/device_descriptor.rs @@ -89,27 +89,22 @@ pub const HID_REPORT_ID_SIZE_BYTES: u16 = 1; impl DeviceDescriptor { pub fn new(hid_device: &HidDevice, hwinfo: HardwareVersionInfo) -> Self { const HID_I2C_PROTOCOL_VERSION: u16 = 0x0100; + let report_length_header_size = HID_REPORT_HEADER_SIZE_BYTES + + if hid_device.report_descriptor().report_ids_implicit() { + 0 + } else { + HID_REPORT_ID_SIZE_BYTES + }; + Self { w_hid_desc_length: core::mem::size_of::() as u16, bcd_version: HID_I2C_PROTOCOL_VERSION, w_report_desc_length: hid_device.report_descriptor().as_bytes().len() as u16, w_report_desc_register: crate::HidI2cRegister::ReportDescriptor as u16, w_input_register: crate::HidI2cRegister::Input.into(), - w_max_input_length: HidDevice::InputReportMaxSize::USIZE as u16 - + HID_REPORT_HEADER_SIZE_BYTES - + if hid_device.report_descriptor().report_ids_implicit() { - 0 - } else { - HID_REPORT_ID_SIZE_BYTES - }, + w_max_input_length: HidDevice::InputReportMaxSize::USIZE as u16 + report_length_header_size, w_output_register: crate::HidI2cRegister::Output.into(), - w_max_output_length: HidDevice::OutputReportMaxSize::USIZE as u16 - + HID_REPORT_HEADER_SIZE_BYTES - + if hid_device.report_descriptor().report_ids_implicit() { - 0 - } else { - HID_REPORT_ID_SIZE_BYTES - }, + w_max_output_length: HidDevice::OutputReportMaxSize::USIZE as u16 + report_length_header_size, w_command_register: crate::HidI2cRegister::Command.into(), w_data_register: crate::HidI2cRegister::Data.into(), w_vendor_id: hwinfo.vendor_id.value(), From 3bd0723ce758f3e140da6eaebdbe30d29cca76d9 Mon Sep 17 00:00:00 2001 From: Billy Price Date: Mon, 27 Jul 2026 14:33:56 -0700 Subject: [PATCH 11/17] pr feedback --- hidi2c-target-service/src/service.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/hidi2c-target-service/src/service.rs b/hidi2c-target-service/src/service.rs index 612ed60e4..400f86a52 100644 --- a/hidi2c-target-service/src/service.rs +++ b/hidi2c-target-service/src/service.rs @@ -581,6 +581,13 @@ impl< trace!("Processing get report command"); let (report_type, report_id) = self.get_io_command_report_header(command_byte).await?; + + // TODO - here, if the report ID is invalid, we're supposed to return a zero-length report. We should know from the + // report descriptor whether the report ID is valid or not, but we don't yet have the report descriptor parsing + // implemented, so we can't do that yet. For now, that responsibility has to fall on the HidDevice implementation, + // but as soon as the aggregation / HID library goes in, look into leveraging it for filtering out invalid report + // IDs here. + self.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) From f2443c5f85e8ec80e2f77d344d43e5e66efe23c7 Mon Sep 17 00:00:00 2001 From: Billy Price Date: Mon, 27 Jul 2026 14:57:02 -0700 Subject: [PATCH 12/17] pr feedback --- hidi2c-target-service/src/attn_pin_handler.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/hidi2c-target-service/src/attn_pin_handler.rs b/hidi2c-target-service/src/attn_pin_handler.rs index c08ebe049..4093965c9 100644 --- a/hidi2c-target-service/src/attn_pin_handler.rs +++ b/hidi2c-target-service/src/attn_pin_handler.rs @@ -5,14 +5,14 @@ use crate::*; /// OutputPin doesn't have a built-in way to interrogate its own state. There is a StatefulOutputPin trait that /// does have that functionality, but not all pins support it so we just implement it ourselves here. /// -pub struct AttnPinHandler { +pub(crate) struct AttnPinHandler { attn_pin: AttnPin, asserted: bool, } impl AttnPinHandler { /// Construct a new handler that owns the provided GPIO hardware - pub fn new(attn_pin: AttnPin) -> Self { + pub(crate) fn new(attn_pin: AttnPin) -> Self { let mut result = Self { attn_pin, asserted: false, @@ -22,7 +22,7 @@ impl AttnPinHandler { } /// Clear the interrupt, which is done by setting the pin high. - pub fn clear_interrupt(&mut self) { + pub(crate) fn clear_interrupt(&mut self) { trace!("HID-I2C: ATTN: clear interrupt"); self.attn_pin .set_high() @@ -31,7 +31,7 @@ impl AttnPinHandler { } /// Assert the interrupt, which is done by pulling the pin low. - pub fn assert_interrupt(&mut self) { + pub(crate) fn assert_interrupt(&mut self) { trace!("HID-I2C: ATTN: assert interrupt"); self.attn_pin .set_low() @@ -40,7 +40,7 @@ impl AttnPinHandler { } /// Returns true if we are asserting the interrupt, false otherwise. - pub fn asserted(&self) -> bool { + pub(crate) fn asserted(&self) -> bool { self.asserted } } From 8c5b13ded8b5816fdd98b8f0ae9b8b721a55c219 Mon Sep 17 00:00:00 2001 From: Billy Price Date: Wed, 29 Jul 2026 15:27:14 -0700 Subject: [PATCH 13/17] oneshot reads --- .../src/constrained_hid_device.rs | 24 ++ hidi2c-target-service/src/service.rs | 238 +++++++++--------- 2 files changed, 140 insertions(+), 122 deletions(-) diff --git a/hidi2c-target-service/src/constrained_hid_device.rs b/hidi2c-target-service/src/constrained_hid_device.rs index 3d48709a5..ac47290e3 100644 --- a/hidi2c-target-service/src/constrained_hid_device.rs +++ b/hidi2c-target-service/src/constrained_hid_device.rs @@ -26,6 +26,8 @@ pub trait ConstrainedHidDevice: embedded_services::relay::hid::HidDevice + seale type MaxInputOrFeatureSize: ArrayLength; /// `max(FeatureReportMaxSize, OutputReportMaxSize)`. type MaxOutputOrFeatureSize: ArrayLength; + /// `max(FeatureReportMaxSize, OutputReportMaxSize) + 9`. + type WriteBufferSize: ArrayLength; } impl ConstrainedHidDevice for T @@ -35,9 +37,31 @@ where T::FeatureReportMaxSize: Max, >::Output: ArrayLength, >::Output: ArrayLength, + >::Output: core::ops::Add, + <>::Output as core::ops::Add>::Output: + ArrayLength, { type MaxInputOrFeatureSize = >::Output; type MaxOutputOrFeatureSize = >::Output; + + /// To avoid splitting the received values across multiple I2C read calls (which injects await points between them and manifests + /// on the bus as clock stretching), we need to have a buffer that's large enough to consume the largest write that a host can do + /// in a single transaction. + /// + /// In this case, that write is for handling the Command: SetReport path (note: not the 'normal' output report register, the one that + /// goes through the Command/Data register path) - see section 7.2.3 of the HID spec. + /// In that path, the largest possible write is: + /// 2 bytes: command register address + /// 2 bytes: command register value (SetReport) + /// 1 byte: optional report ID extension to command register value for report IDs > 15 + /// 2 bytes: data register address + /// 2 bytes: data register length header + /// N bytes: length of the actual report payload + /// + /// Therefore, this buffer needs to be 9 bytes larger than the largest output or feature report + /// + type WriteBufferSize = + <>::Output as core::ops::Add>::Output; } impl sealed::Sealed for T where T: embedded_services::relay::hid::HidDevice {} diff --git a/hidi2c-target-service/src/service.rs b/hidi2c-target-service/src/service.rs index 400f86a52..10a631f48 100644 --- a/hidi2c-target-service/src/service.rs +++ b/hidi2c-target-service/src/service.rs @@ -148,7 +148,10 @@ impl TimeoutBus { } /// Read bytes the host is writing to us, applying the data-read timeout and recovering the bus on failure. - async fn read(&mut self, buffer: &mut [u8]) -> Result> { + /// Buffer must be as large as the largest possible write the host can do in a single transaction. If the host + /// writes more bytes than the provided buffer, we drop any remaining bytes so as to not stall the bus and return + /// an error. + async fn read<'buf>(&mut self, buffer: &'buf mut [u8]) -> Result<&'buf [u8], Error> { match with_timeout( self.timeout_settings.data_read_timeout, self.bus.respond_to_write(buffer), @@ -162,11 +165,15 @@ impl TimeoutBus { Err(Error::Protocol(ProtocolError::Timeout)) } // Controller finished writing; report how many bytes we drained. - Ok(Ok( - status @ (WriteStatus::Stopped(bytes) | WriteStatus::Restarted(bytes) | WriteStatus::BufferFull(bytes)), - )) => { + Ok(Ok(status @ (WriteStatus::Stopped(bytes) | WriteStatus::Restarted(bytes)))) => { trace!("Host issued write command: {:?}", status); - Ok(bytes) + + Ok(buffer.get(..bytes).ok_or(Error::Protocol(ProtocolError::InvalidData))?) + } + Ok(Ok(WriteStatus::BufferFull(_bytes))) => { + warn!("Host attempted to issue more bytes than we can handle - failing read"); + self.discard_remaining_bytes_from_host().await?; + Err(Error::Protocol(ProtocolError::InvalidData)) } // Some other write status we don't expect while reading. Ok(Ok(status)) => { @@ -181,6 +188,24 @@ impl TimeoutBus { } } + async fn discard_remaining_bytes_from_host(&mut self) -> Result<(), Error> { + let mut discard_buffer = [0u8; 16]; + loop { + let result = with_timeout( + self.timeout_settings.data_read_timeout, + self.bus.respond_to_write(&mut discard_buffer), + ) + .await? + .map_err(Error::Bus)?; + match result { + WriteStatus::BufferFull(_bytes) => { + continue; + } + _ => return Ok(()), + } + } + } + /// Write all of `buffer` to the host, padding with zeros if the host asks for more bytes. async fn write(&mut self, buffer: &[u8]) -> Result<(), Error> { let mut write_buffer: &[u8] = buffer; @@ -193,6 +218,8 @@ impl TimeoutBus { } /// Write `buffer` to the host; returns true if the host requested more bytes than we provided. + /// TODO - we should augment the I2C trait to allow us to write a slice of slices in a single operation so we don't have + /// multiple await points, which causes us to hog the bus. When we land that, remove this and switch to that API instead. async fn write_unterminated(&mut self, buffer: &[u8]) -> Result> { match with_timeout( self.timeout_settings.device_response_timeout, @@ -227,7 +254,7 @@ pub struct Runner<'hw, Bus: I2cTargetAsync, AttnPin: embedded_hal::digital::Outp device_descriptor: DeviceDescriptor, /// Buffer for receiving messages. - write_buf: generic_array::GenericArray, + write_buf: generic_array::GenericArray, /// True if a reset has been triggered but not yet acknowledged by the host pending_reset: bool, @@ -345,14 +372,13 @@ impl< } async fn process_register_access(&mut self) -> Result<(), Error> { - let mut reg = [0u8; 2]; - let read = self.bus.read(&mut reg).await?; - if read != reg.len() { - error!("Expected to read {} bytes but got {}", reg.len(), read); - return Err(Error::Protocol(ProtocolError::InvalidData)); - } + let data = self.bus.read(&mut self.write_buf).await?; + + let (®ister, data) = data + .split_first_chunk::<2>() + .ok_or(Error::Protocol(ProtocolError::InvalidData))?; - let register = HidI2cRegister::try_from(u16::from_le_bytes(reg)) + let register = HidI2cRegister::try_from(u16::from_le_bytes(register)) .map_err(|_| Error::Protocol(ProtocolError::InvalidRegisterAddress))?; info!("HID-I2C: Host requested to access register {:?}", register); @@ -390,8 +416,38 @@ impl< } }, HidI2cRegister::Input => self.process_input_report_read().await, - HidI2cRegister::Output => self.process_output_report_write().await, - HidI2cRegister::Command => self.process_command().await, + HidI2cRegister::Output => { + let (&report_length_bytes, data) = data + .split_first_chunk::<{ core::mem::size_of::() }>() + .ok_or(Error::Protocol(ProtocolError::InvalidSize))?; + + // NOTE - if we're using implicit report IDs, we present that to HidDevice implementations as report ID 0. + let (&report_id, data) = if self.hid_device.report_descriptor().report_ids_implicit() { + (&0, data) + } else { + data.split_first().ok_or(Error::Protocol(ProtocolError::InvalidSize))? + }; + + let header_size_bytes = 2 + if self.hid_device.report_descriptor().report_ids_implicit() { + 0 + } else { + 1 + }; + + let length = (u16::from_le_bytes(report_length_bytes) as usize) + .checked_sub(header_size_bytes) + .ok_or(Error::Protocol(ProtocolError::InvalidSize))?; // Note: per HID spec, the length field needs to include its own length (2 bytes) and the report ID (1 byte) + + let output_report = embedded_services::relay::hid::SetHidReport::Output(HidReport::new( + embedded_services::relay::hid::ReportId(report_id), + data.get(..length).ok_or(Error::Protocol(ProtocolError::InvalidSize))?, + )); + + self.hid_device.set_report(&output_report).await?; + + Ok(()) + } + HidI2cRegister::Command => Self::process_command(data, &mut self.bus, &mut self.hid_device).await, HidI2cRegister::Data => { error!( "HID-I2C: Got read to Data register without a preceding write to the Command register; this is unexpected and may indicate a bug in the service." @@ -471,97 +527,52 @@ impl< Ok(()) } - async fn process_output_report_write(&mut self) -> Result<(), Error> { - let mut write_header_buf = [0u8; (device_descriptor::HID_REPORT_HEADER_SIZE_BYTES - + device_descriptor::HID_REPORT_ID_SIZE_BYTES) as usize]; - - // NOTE - if we're using implicit report IDs, we present that to HidDevice implementations as report ID 0. - let header_len = if self.hid_device.report_descriptor().report_ids_implicit() { - write_header_buf.len() - (device_descriptor::HID_REPORT_ID_SIZE_BYTES as usize) - } else { - write_header_buf.len() - }; - - let header_buf_slice = write_header_buf - .get_mut(..header_len) - .ok_or(Error::Protocol(ProtocolError::InvalidSize))?; - - let header_read = self.bus.read(header_buf_slice).await?; - if header_read != header_len { - error!("Expected to read {} bytes but got {}", header_len, header_read); - return Err(Error::Protocol(ProtocolError::InvalidSize)); - } - - let [len_low, len_high, report_id] = write_header_buf; - let length = (u16::from_le_bytes([len_low, len_high]) as usize) - .checked_sub(header_len) - .ok_or(Error::Protocol(ProtocolError::InvalidSize))?; // Note: per HID spec, the length field needs to include its own length (2 bytes) and the report ID (1 byte) - trace!("Reading {} bytes", length); - - let read_result = self.bus.read(&mut self.write_buf).await?; - - if read_result != length { - error!("Expected to read {} bytes but got {}", length, read_result); - return Err(Error::Protocol(ProtocolError::InvalidSize)); - } - - let output_report = embedded_services::relay::hid::SetHidReport::Output(HidReport::new( - embedded_services::relay::hid::ReportId(report_id), - self.write_buf - .get(..length) - .ok_or(Error::Protocol(ProtocolError::InvalidSize))?, - )); - - self.hid_device.set_report(&output_report).await?; - - Ok(()) - } - - /// Attempts to parse the provided command byte as an input / output command header, including reading any more bytes if - /// needed and verifying that the host followed the command header with the expected write to the data register address. - /// Upon success, callers should proceed to read or write the requested payload to/from the bus. + /// Attempts to parse the provided command byte as an input / output command header, including consuming any additional + /// bytes from the read as needed. + /// Returns: + /// - The report type (input, output, feature) + /// - The report ID + /// - A slice over the remaining bytes from data async fn get_io_command_report_header( - &mut self, + data: &[u8], command_byte: u8, - ) -> Result<(HidI2cReportType, embedded_services::relay::hid::ReportId), Error> { + ) -> Result<(HidI2cReportType, embedded_services::relay::hid::ReportId, &[u8]), Error> { let command_header = HidI2cReportCommandHeader::try_from_command_byte(command_byte)?; - let report_id = if let Some(report_id) = command_header.report_id { - report_id + let (report_id, data) = if let Some(report_id) = command_header.report_id { + (report_id, data) } else { - let mut report_id = 0u8; - self.bus.read(core::slice::from_mut(&mut report_id)).await?; - embedded_services::relay::hid::ReportId(report_id) + let (&report_id, data) = data.split_first().ok_or(Error::Protocol(ProtocolError::InvalidSize))?; + (embedded_services::relay::hid::ReportId(report_id), data) }; - let mut data_register_address = [0u8; core::mem::size_of::()]; - let data_register_address_read = self.bus.read(&mut data_register_address).await?; - if data_register_address_read != data_register_address.len() { - error!( - "Expected to read {} bytes but got {}", - data_register_address.len(), - data_register_address_read - ); - return Err(Error::Protocol(ProtocolError::InvalidSize)); - } + let (&data_register_address, data) = data + .split_first_chunk::<{ core::mem::size_of::() }>() + .ok_or(Error::Protocol(ProtocolError::InvalidSize))?; - if u16::from_le_bytes(data_register_address) != HidI2cRegister::Data as u16 { + let data_register_address = u16::from_le_bytes(data_register_address); + if data_register_address != HidI2cRegister::Data as u16 { error!( "Expected the host to write the data register address after the header ({:?}) but got {:?}", HidI2cRegister::Data, - u16::from_le_bytes(data_register_address) + data_register_address ); return Err(Error::Protocol(ProtocolError::InvalidRegisterAddress)); } - Ok((command_header.report_type, report_id)) + Ok((command_header.report_type, report_id, data)) } - async fn process_command(&mut self) -> Result<(), Error> { - let [command_byte, opcode_byte] = { - let mut command_header_buffer = [0u8; 2]; - self.bus.read(&mut command_header_buffer).await?; - command_header_buffer - }; + async fn process_command( + data: &[u8], + bus: &mut TimeoutBus, + hid_device: &mut HidDevice, + ) -> Result<(), Error> { + let (&command_byte, data) = data + .split_first() + .ok_or(Error::Protocol(ProtocolError::InvalidCommand))?; + let (&opcode_byte, data) = data + .split_first() + .ok_or(Error::Protocol(ProtocolError::InvalidCommand))?; match Opcode::try_from(opcode_byte).map_err(|_| Error::Protocol(ProtocolError::InvalidCommand))? { Opcode::Reset => { @@ -573,14 +584,14 @@ impl< trace!("Processing set power command"); let power_state = I2cPowerState::try_from(command_byte) .map_err(|_| Error::Protocol(ProtocolError::InvalidCommand))?; - self.hid_device.set_power_state(power_state.into()).await?; + hid_device.set_power_state(power_state.into()).await?; Ok(()) } Opcode::GetReport => { trace!("Processing get report command"); - let (report_type, report_id) = self.get_io_command_report_header(command_byte).await?; + let (report_type, report_id, _data) = Self::get_io_command_report_header(data, command_byte).await?; // TODO - here, if the report ID is invalid, we're supposed to return a zero-length report. We should know from the // report descriptor whether the report ID is valid or not, but we don't yet have the report descriptor parsing @@ -588,13 +599,13 @@ impl< // but as soon as the aggregation / HID library goes in, look into leveraging it for filtering out invalid report // IDs here. - self.hid_device + 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) let len_header = (report.data().len() as u16 + device_descriptor::HID_REPORT_HEADER_SIZE_BYTES) .to_le_bytes(); - self.bus.write(&len_header).await?; - self.bus.write(report.data()).await?; + bus.write_unterminated(&len_header).await?; + bus.write(report.data()).await?; Ok::<(), Error>(()) }) .await??; @@ -604,48 +615,31 @@ impl< Opcode::SetReport => { trace!("Processing set report command"); - let (report_type, report_id) = self.get_io_command_report_header(command_byte).await?; + let (report_type, report_id, data) = Self::get_io_command_report_header(data, command_byte).await?; - let mut len_header = [0u8; core::mem::size_of::()]; - let header_read = self.bus.read(&mut len_header).await?; - if header_read != len_header.len() { - error!("Expected to read {} bytes but got {}", len_header.len(), header_read); - return Err(Error::Protocol(ProtocolError::InvalidSize)); - } + let (&len_header, data) = data + .split_first_chunk::<{ core::mem::size_of::() }>() + .ok_or(Error::Protocol(ProtocolError::InvalidSize))?; // Note: per HID spec, the length field relayed over the wire needs to include its own length (2 bytes) let report_size = (u16::from_le_bytes(len_header) .checked_sub(device_descriptor::HID_REPORT_HEADER_SIZE_BYTES)) .ok_or(Error::Protocol(ProtocolError::InvalidSize))? as usize; - self.bus - .read( - self.write_buf - .get_mut(..report_size) - .ok_or(Error::Protocol(ProtocolError::InvalidSize))?, - ) - .await?; + let report_data = data + .get(..report_size) + .ok_or(Error::Protocol(ProtocolError::InvalidSize))?; let set_report = match report_type { HidI2cReportType::Input => { error!("Host attempted to send us an input report, which is invalid"); return Err(Error::Protocol(ProtocolError::InvalidReportType)); } - HidI2cReportType::Output => SetHidReport::Output(HidReport::new( - report_id, - self.write_buf - .get(..report_size) - .ok_or(Error::Protocol(ProtocolError::InvalidSize))?, - )), - HidI2cReportType::Feature => SetHidReport::Feature(HidReport::new( - report_id, - self.write_buf - .get(..report_size) - .ok_or(Error::Protocol(ProtocolError::InvalidSize))?, - )), + HidI2cReportType::Output => SetHidReport::Output(HidReport::new(report_id, report_data)), + HidI2cReportType::Feature => SetHidReport::Feature(HidReport::new(report_id, report_data)), }; - self.hid_device.set_report(&set_report).await?; + hid_device.set_report(&set_report).await?; Ok(()) } From 0544c96a4ed832c9b275e132e003889cb0592913 Mon Sep 17 00:00:00 2001 From: Billy Price Date: Thu, 30 Jul 2026 14:32:45 -0700 Subject: [PATCH 14/17] refactor kb mock to better demonstrate correct usage --- .../rt685s-evk/src/bin/mock_i2c_keyboard.rs | 19 +- examples/rt685s-evk/src/mocks/keyboard.rs | 269 ------------------ .../rt685s-evk/src/mocks/keyboard/device.rs | 113 ++++++++ .../src/mocks/keyboard/interface.rs | 41 +++ examples/rt685s-evk/src/mocks/keyboard/mod.rs | 3 + .../rt685s-evk/src/mocks/keyboard/relay.rs | 162 +++++++++++ 6 files changed, 332 insertions(+), 275 deletions(-) delete mode 100644 examples/rt685s-evk/src/mocks/keyboard.rs create mode 100644 examples/rt685s-evk/src/mocks/keyboard/device.rs create mode 100644 examples/rt685s-evk/src/mocks/keyboard/interface.rs create mode 100644 examples/rt685s-evk/src/mocks/keyboard/mod.rs create mode 100644 examples/rt685s-evk/src/mocks/keyboard/relay.rs diff --git a/examples/rt685s-evk/src/bin/mock_i2c_keyboard.rs b/examples/rt685s-evk/src/bin/mock_i2c_keyboard.rs index c6c886720..a6268c2f3 100644 --- a/examples/rt685s-evk/src/bin/mock_i2c_keyboard.rs +++ b/examples/rt685s-evk/src/bin/mock_i2c_keyboard.rs @@ -8,7 +8,11 @@ use embassy_imxrt::i2c::slave::{Address, I2cSlave}; use embassy_imxrt::i2c::{self, Async}; use embassy_imxrt::{bind_interrupts, peripherals}; use panic_probe as _; -use rt685s_evk_example::mocks::keyboard::{KeyCode, MockKeyboardHidRelay, MockKeyboardService}; +use rt685s_evk_example::mocks::keyboard::{ + device::{MockKeyboardService, MockKeyboardServiceResources}, + interface::KeyCode, + relay::MockKeyboardHidRelay, +}; use static_cell::StaticCell; const SLAVE_ADDR: Option
= Address::new(0x15); @@ -33,8 +37,11 @@ async fn main(spawner: Spawner) { gpio::SlewRate::Standard, ); - static KEYBOARD_SERVICE: StaticCell = StaticCell::new(); - let keyboard_service = KEYBOARD_SERVICE.init(MockKeyboardService::new()); + const KB_SUBS: usize = 1; + static KB_RESOURCES: StaticCell> = StaticCell::new(); + let (keyboard_service, keyboard_runner) = MockKeyboardService::new(KB_RESOURCES.init(Default::default())); + + static KEYBOARD_SERVICE: StaticCell> = StaticCell::new(); // NOTE: here's where the "aggregate HID devices" macro is currently missing. Compare with time_alarm.rs where we do this: // @@ -66,13 +73,13 @@ async fn main(spawner: Spawner) { 'static, I2cSlave<'static, Async>, gpio::Output<'static>, - MockKeyboardHidRelay<'static>, + MockKeyboardHidRelay<'static, MockKeyboardService<'static, KB_SUBS>>, >, |resources| hidi2c_target_service::Service::new( resources, i2c, attn_pin, - MockKeyboardHidRelay::new(keyboard_service), + MockKeyboardHidRelay::new(KEYBOARD_SERVICE.init(keyboard_service)), hidi2c_target_service::HardwareVersionInfo { vendor_id: hidi2c_target_service::VendorId::new(0x1234).unwrap(), product_id: hidi2c_target_service::ProductId(0x5678), @@ -89,7 +96,7 @@ async fn main(spawner: Spawner) { let mut i = 0; loop { info!("pressing key"); - keyboard_service.click_key(KeyCode::NumLock).await; + keyboard_runner.click_key(KeyCode::NumLock).await; embassy_time::Timer::after(embassy_time::Duration::from_millis(2000)).await; i += 1; if i % 5 == 0 { diff --git a/examples/rt685s-evk/src/mocks/keyboard.rs b/examples/rt685s-evk/src/mocks/keyboard.rs deleted file mode 100644 index 1c6726f72..000000000 --- a/examples/rt685s-evk/src/mocks/keyboard.rs +++ /dev/null @@ -1,269 +0,0 @@ -//! Mock HID keyboard device used by the `mock_i2c_keyboard` example. -//! -//! Unlike the mouse mock, the keyboard copies each report out of a regular channel, which is simpler, -//! but can be costly for larger reports. - -use defmt::info; -use embedded_services::relay::hid::*; -use embedded_services::warn; -use zerocopy::{FromBytes, IntoBytes}; - -/// Implicit report ID used by the standalone keyboard: no report-ID item appears in -/// [`KEYBOARD_HID_REPORT_DESCRIPTOR_NO_ID`], so the transport service defaults to report ID 0. -pub const REPORTID_KEYBOARD_NONE: u8 = 0; - -/// Explicit report ID baked into [`KEYBOARD_HID_REPORT_DESCRIPTOR_WITH_ID`], used when the keyboard -/// is composed into an aggregate device (where every report ID must be explicit). -pub const REPORTID_KEYBOARD_AGGREGATE: u8 = 1; - -// This is adapted from the example keyboard HID descriptor packaged with the DT.exe tool / https://learn.microsoft.com/en-us/windows-hardware/design/component-guidelines/keyboard-collection-report-descriptor - -/// Keyboard report descriptor WITHOUT an explicit report ID. The transport service falls back to -/// report ID [`REPORTID_KEYBOARD_NONE`]. Suitable for a standalone keyboard that only exposes one -/// report. -#[rustfmt::skip] -pub const KEYBOARD_HID_REPORT_DESCRIPTOR_NO_ID: &[u8] = &[ - 0x05, 0x01, // USAGE_PAGE (Generic Desktop) - 0x09, 0x06, // USAGE (Keyboard) - 0xa1, 0x01, // COLLECTION (Application) - 0x05, 0x07, // USAGE_PAGE (Keyboard) - 0x19, 0xe0, // USAGE_MINIMUM (Keyboard LeftControl) - 0x29, 0xe7, // USAGE_MAXIMUM (Keyboard Right GUI) - 0x15, 0x00, // LOGICAL_MINIMUM (0) - 0x25, 0x01, // LOGICAL_MAXIMUM (1) - 0x75, 0x01, // REPORT_SIZE (1) - 0x95, 0x08, // REPORT_COUNT (8) - 0x81, 0x02, // INPUT (Data,Var,Abs) - 0x95, 0x01, // REPORT_COUNT (1) - 0x75, 0x08, // REPORT_SIZE (8) - 0x81, 0x03, // INPUT (Cnst,Var,Abs) - 0x95, 0x05, // REPORT_COUNT (5) - 0x75, 0x01, // REPORT_SIZE (1) - 0x05, 0x08, // USAGE_PAGE (LEDs) - 0x19, 0x01, // USAGE_MINIMUM (Num Lock) - 0x29, 0x05, // USAGE_MAXIMUM (Kana) - 0x91, 0x02, // OUTPUT (Data,Var,Abs) - 0x95, 0x01, // REPORT_COUNT (1) - 0x75, 0x03, // REPORT_SIZE (3) - 0x91, 0x03, // OUTPUT (Cnst,Var,Abs) - 0x95, 0x06, // REPORT_COUNT (6) - 0x75, 0x08, // REPORT_SIZE (8) - 0x15, 0x00, // LOGICAL_MINIMUM (0) - 0x25, 0x65, // LOGICAL_MAXIMUM (101) - 0x05, 0x07, // USAGE_PAGE (Keyboard) - 0x19, 0x00, // USAGE_MINIMUM (Reserved (no event indicated)) - 0x29, 0x65, // USAGE_MAXIMUM (Keyboard Application) - 0x81, 0x00, // INPUT (Data,Ary,Abs) - 0xc0, // END_COLLECTION -]; - -/// Keyboard report descriptor WITH an explicit report ID of [`REPORTID_KEYBOARD_AGGREGATE`]. Used -/// when composing the keyboard into an aggregate device, where report IDs must be explicit so they -/// can be renumbered to avoid collisions. -#[rustfmt::skip] -pub const KEYBOARD_HID_REPORT_DESCRIPTOR_WITH_ID: &[u8] = &[ - 0x05, 0x01, // USAGE_PAGE (Generic Desktop) - 0x09, 0x06, // USAGE (Keyboard) - 0xa1, 0x01, // COLLECTION (Application) - 0x85, REPORTID_KEYBOARD_AGGREGATE, // REPORT_ID - 0x05, 0x07, // USAGE_PAGE (Keyboard) - 0x19, 0xe0, // USAGE_MINIMUM (Keyboard LeftControl) - 0x29, 0xe7, // USAGE_MAXIMUM (Keyboard Right GUI) - 0x15, 0x00, // LOGICAL_MINIMUM (0) - 0x25, 0x01, // LOGICAL_MAXIMUM (1) - 0x75, 0x01, // REPORT_SIZE (1) - 0x95, 0x08, // REPORT_COUNT (8) - 0x81, 0x02, // INPUT (Data,Var,Abs) - 0x95, 0x01, // REPORT_COUNT (1) - 0x75, 0x08, // REPORT_SIZE (8) - 0x81, 0x03, // INPUT (Cnst,Var,Abs) - 0x95, 0x05, // REPORT_COUNT (5) - 0x75, 0x01, // REPORT_SIZE (1) - 0x05, 0x08, // USAGE_PAGE (LEDs) - 0x19, 0x01, // USAGE_MINIMUM (Num Lock) - 0x29, 0x05, // USAGE_MAXIMUM (Kana) - 0x91, 0x02, // OUTPUT (Data,Var,Abs) - 0x95, 0x01, // REPORT_COUNT (1) - 0x75, 0x03, // REPORT_SIZE (3) - 0x91, 0x03, // OUTPUT (Cnst,Var,Abs) - 0x95, 0x06, // REPORT_COUNT (6) - 0x75, 0x08, // REPORT_SIZE (8) - 0x15, 0x00, // LOGICAL_MINIMUM (0) - 0x25, 0x65, // LOGICAL_MAXIMUM (101) - 0x05, 0x07, // USAGE_PAGE (Keyboard) - 0x19, 0x00, // USAGE_MINIMUM (Reserved (no event indicated)) - 0x29, 0x65, // USAGE_MAXIMUM (Keyboard Application) - 0x81, 0x00, // INPUT (Data,Ary,Abs) - 0xc0, // END_COLLECTION -]; - -#[repr(C, packed)] -#[derive(Debug, Clone, Copy, Default, defmt::Format, zerocopy::FromBytes, zerocopy::IntoBytes, zerocopy::Immutable)] -pub struct KeyboardInputReport { - /// Left Ctrl .. Right GUI - pub modifiers: u8, - - /// Reserved byte required by boot keyboard format - pub reserved: u8, - - /// Up to 6 simultaneous key usages - pub keys: [u8; 6], -} - -#[repr(u8)] -#[derive(num_enum::IntoPrimitive, num_enum::TryFromPrimitive, Debug, Clone, Copy, defmt::Format)] -pub enum KeyCode { - NumLock = 0x53, - A = 0x04, -} - -#[repr(C, packed)] -#[derive(Debug, Clone, Copy, defmt::Format, zerocopy::FromBytes, zerocopy::IntoBytes, zerocopy::Immutable)] -pub struct KeyboardOutputReport { - /// LED state bits - pub leds: u8, -} - -/// Depth of the keyboard input-report channel. -const KEYBOARD_CHANNEL_DEPTH: usize = 5; - -/// Consumer side of the mock keyboard. Owns the channel that carries input reports and exposes a -/// method to inject key presses. -pub struct MockKeyboardService { - channel: - embassy_sync::channel::Channel, -} - -impl Default for MockKeyboardService { - fn default() -> Self { - Self::new() - } -} - -impl MockKeyboardService { - pub const fn new() -> Self { - Self { - channel: embassy_sync::channel::Channel::new(), - } - } - - pub async fn click_key(&self, key_code: KeyCode) { - // key down - let send_result = self.channel.try_send(KeyboardInputReport { - modifiers: 0, - reserved: 0, - keys: [key_code.into(), 0, 0, 0, 0, 0], - }); - - if let Err(e) = send_result { - warn!("Failed to send key down report: {:?}", e); - } - - embassy_time::Timer::after(embassy_time::Duration::from_millis(15)).await; - - // key up - let send_result = self.channel.try_send(KeyboardInputReport::default()); - if let Err(e) = send_result { - warn!("Failed to send key up report: {:?}", e); - } - } - - pub fn receiver( - &self, - ) -> embassy_sync::channel::Receiver< - '_, - embedded_services::GlobalRawMutex, - KeyboardInputReport, - KEYBOARD_CHANNEL_DEPTH, - > { - self.channel.receiver() - } -} - -/// Relay adapter that presents the mock keyboard to the HID-I2C service as a [`HidDevice`]. -/// -pub struct MockKeyboardHidRelay<'s> { - service: &'s MockKeyboardService, - report_id: ReportId, - descriptor: HidReportDescriptor<'static>, -} - -impl<'s> MockKeyboardHidRelay<'s> { - pub fn new(service: &'s MockKeyboardService) -> Self { - Self { - service, - report_id: ReportId(1), - descriptor: HidReportDescriptor::new(KEYBOARD_HID_REPORT_DESCRIPTOR_WITH_ID) - .expect("keyboard HID report descriptor should be valid"), - } - } -} - -impl embedded_services::relay::hid::HidDevice for MockKeyboardHidRelay<'_> { - type InputReportMaxSize = typenum::U8; - type OutputReportMaxSize = typenum::U1; - type FeatureReportMaxSize = typenum::U0; - - const MAX_REPORT_COUNT: u8 = 2; - - fn report_descriptor(&self) -> &HidReportDescriptor<'_> { - &self.descriptor - } - - async fn process_get_report( - &mut self, - _report_type: GetHidReportType, - report_id: ReportId, - process_report: impl AsyncFnOnce(GetHidReport<'_>) -> R, - ) -> Result { - info!("Received command to get report with ID {:?}", report_id); - if report_id == self.report_id { - let report = KeyboardInputReport::default(); - Ok(process_report(GetHidReport::Input(HidReport::new(report_id, report.as_bytes()))).await) - } else { - info!("Report ID {:?} not recognized", report_id); - Err(HidError::TriggerReset) - } - } - - async fn set_report(&mut self, report: &SetHidReport<'_>) -> Result<(), HidError> { - match report { - SetHidReport::Output(r) if r.id() == self.report_id => { - let output_report = KeyboardOutputReport::read_from_bytes(r.data()).unwrap(); - info!("Received keyboard output report: {:?}", output_report); - } - SetHidReport::Output(r) => { - info!("Report ID {:?} not recognized", r.id()); - return Err(HidError::TriggerReset); - } - SetHidReport::Feature(r) => info!("Received command to set feature report with ID {:?}", r.id()), - } - Ok(()) - } - - async fn wait_for_input_report(&mut self) { - self.service.receiver().ready_to_receive().await - } - - async fn process_next_input_report( - &mut self, - process_report: impl AsyncFnOnce(HidReport<'_>) -> R, - ) -> Result { - let input_report = self.service.receiver().receive().await; - Ok(process_report(HidReport::new(self.report_id, input_report.as_bytes())).await) - } - - fn has_pending_input_report(&mut self) -> bool { - !self.service.receiver().is_empty() - } - - async fn set_power_state(&mut self, state: HidDevicePowerState) -> Result<(), HidError> { - info!("Received command to set power state to {:?}", state); - Ok(()) - } - - async fn reset(&mut self) { - info!("Received reset command"); - self.service.receiver().clear(); - } -} diff --git a/examples/rt685s-evk/src/mocks/keyboard/device.rs b/examples/rt685s-evk/src/mocks/keyboard/device.rs new file mode 100644 index 000000000..5e29424f5 --- /dev/null +++ b/examples/rt685s-evk/src/mocks/keyboard/device.rs @@ -0,0 +1,113 @@ +//! Mock HID keyboard device used by the `mock_i2c_keyboard` example. +//! +//! Unlike the mouse mock, the keyboard copies each report out of a regular channel, which is simpler, +//! but can be costly for larger reports. + +use super::interface::*; +use embedded_services::{info, warn}; + +/// Depth of the keyboard input-report channel. In a production use case, you may want to make the service generic over this value rather than hardcoding it. +const KEYBOARD_CHANNEL_DEPTH: usize = 5; + +struct MockKeyboardServiceResourcesInner { + channel: embassy_sync::pubsub::PubSubChannel< + embedded_services::GlobalRawMutex, + KeyboardInputReport, + KEYBOARD_CHANNEL_DEPTH, + MAX_SUBS, + 1, + >, +} + +pub struct MockKeyboardServiceResources { + inner: Option>, +} + +impl Default for MockKeyboardServiceResources { + fn default() -> Self { + Self { + inner: Some(MockKeyboardServiceResourcesInner { + channel: embassy_sync::pubsub::PubSubChannel::new(), + }), + } + } +} + +/// Consumer side of the mock keyboard. Owns the channel that carries input reports and exposes a +/// method to inject key presses. +pub struct MockKeyboardService<'hw, const MAX_SUBS: usize> { + resources: &'hw MockKeyboardServiceResourcesInner, +} + +impl<'hw, const MAX_SUBS: usize> MockKeyboardService<'hw, MAX_SUBS> { + pub fn new( + resources: &'hw mut MockKeyboardServiceResources, + ) -> (Self, MockKeyboardServiceRunner<'hw, MAX_SUBS>) { + let resources = resources.inner.insert(MockKeyboardServiceResourcesInner { + channel: embassy_sync::pubsub::PubSubChannel::new(), + }); + ( + Self { resources }, + MockKeyboardServiceRunner { + publisher: resources + .channel + .publisher() + .expect("We know there's a free publisher because we just created the channel"), + }, + ) + } +} + +pub struct MockKeyboardServiceRunner<'hw, const MAX_SUBS: usize> { + publisher: embassy_sync::pubsub::Publisher< + 'hw, + embedded_services::GlobalRawMutex, + KeyboardInputReport, + KEYBOARD_CHANNEL_DEPTH, + MAX_SUBS, + 1, + >, +} + +// Note: on a normal keyboard service, this would just have a run() method that actually does the keyscanning. +// For the mock, we just let the user inject keystrokes instead. +impl<'hw, const MAX_SUBS: usize> MockKeyboardServiceRunner<'hw, MAX_SUBS> { + pub async fn click_key(&self, key_code: KeyCode) { + // key down + let send_result = self.publisher.try_publish(KeyboardInputReport { + modifiers: 0, + reserved: 0, + keys: [key_code.into(), 0, 0, 0, 0, 0], + }); + + if let Err(e) = send_result { + warn!("Failed to send key down report: {:?}", e); + } + + embassy_time::Timer::after(embassy_time::Duration::from_millis(15)).await; + + // key up + let send_result = self.publisher.try_publish(KeyboardInputReport::default()); + if let Err(e) = send_result { + warn!("Failed to send key up report: {:?}", e); + } + } +} + +impl<'hw, const MAX_SUBS: usize> KeyboardService<'hw> for MockKeyboardService<'hw, MAX_SUBS> { + async fn set_led(&mut self, state: u8) { + info!("Setting LED state (mock) - no-op. New state:"); + info!( + "NumLock: {}, CapsLock: {}, ScrollLock: {}", + state & (KeyboardLedFlags::NumLock as u8) != 0, + state & (KeyboardLedFlags::CapsLock as u8) != 0, + state & (KeyboardLedFlags::ScrollLock as u8) != 0 + ); + } + + fn subscriber( + &self, + ) -> Result, embassy_sync::pubsub::Error> { + self.resources.channel.dyn_subscriber() + } +} diff --git a/examples/rt685s-evk/src/mocks/keyboard/interface.rs b/examples/rt685s-evk/src/mocks/keyboard/interface.rs new file mode 100644 index 000000000..adff9b8d7 --- /dev/null +++ b/examples/rt685s-evk/src/mocks/keyboard/interface.rs @@ -0,0 +1,41 @@ +///! A simplified interface for a hypothetical keyboard service. +pub trait KeyboardService<'hw> { + fn set_led(&mut self, state: u8) -> impl core::future::Future + Send; + + fn subscriber( + &self, + ) -> Result, embassy_sync::pubsub::Error>; +} + +/// A report from the keyboard. Note that keyboards are very HID-centric so it makes sense that this has the same +/// underlying representation as a HID keyboard input report, but this is an optimization that may not make sense for +/// other service types (e.g. time-alarm may just emit a non-repr(C) enum of the kind of event and leave it up to +/// the relay handler to figure out how to translate that to HID (or MCTP or some other logical protocol). +/// +#[repr(C, packed)] +#[derive(Debug, Clone, Copy, Default, defmt::Format, zerocopy::FromBytes, zerocopy::IntoBytes, zerocopy::Immutable)] +pub struct KeyboardInputReport { + /// Left Ctrl .. Right GUI + pub modifiers: u8, + + /// Reserved byte required by boot keyboard format + pub reserved: u8, + + /// Up to 6 simultaneous key usages + pub keys: [u8; 6], +} + +/// A (very) simplified list of the possible keys that can be pressed on a keyboard. +#[repr(u8)] +#[derive(num_enum::IntoPrimitive, num_enum::TryFromPrimitive, Debug, Clone, Copy, defmt::Format)] +pub enum KeyCode { + NumLock = 0x53, + A = 0x04, +} + +#[repr(u8)] +pub enum KeyboardLedFlags { + NumLock = 0x01, + CapsLock = 0x02, + ScrollLock = 0x04, +} diff --git a/examples/rt685s-evk/src/mocks/keyboard/mod.rs b/examples/rt685s-evk/src/mocks/keyboard/mod.rs new file mode 100644 index 000000000..18f9c7d3e --- /dev/null +++ b/examples/rt685s-evk/src/mocks/keyboard/mod.rs @@ -0,0 +1,3 @@ +pub mod device; +pub mod interface; +pub mod relay; diff --git a/examples/rt685s-evk/src/mocks/keyboard/relay.rs b/examples/rt685s-evk/src/mocks/keyboard/relay.rs new file mode 100644 index 000000000..a9d053f99 --- /dev/null +++ b/examples/rt685s-evk/src/mocks/keyboard/relay.rs @@ -0,0 +1,162 @@ +use super::interface::*; +use defmt::info; +use embedded_services::relay::hid::*; +use zerocopy::{FromBytes, IntoBytes}; + +/// Implicit report ID used by the standalone keyboard: no report-ID item appears in +/// [`KEYBOARD_HID_REPORT_DESCRIPTOR_NO_ID`], so the transport service defaults to report ID 0. +const REPORTID_KEYBOARD_NO_ID: ReportId = ReportId(0); + +// This is adapted from the example keyboard HID descriptor packaged with the DT.exe tool / https://learn.microsoft.com/en-us/windows-hardware/design/component-guidelines/keyboard-collection-report-descriptor + +/// Keyboard report descriptor WITHOUT an explicit report ID. The transport service falls back to +/// report ID [`REPORTID_KEYBOARD_NO_ID`]. Suitable for a standalone keyboard that only exposes one +/// report. +#[rustfmt::skip] +const KEYBOARD_HID_REPORT_DESCRIPTOR_NO_ID: &[u8] = &[ + 0x05, 0x01, // USAGE_PAGE (Generic Desktop) + 0x09, 0x06, // USAGE (Keyboard) + 0xa1, 0x01, // COLLECTION (Application) + 0x05, 0x07, // USAGE_PAGE (Keyboard) + 0x19, 0xe0, // USAGE_MINIMUM (Keyboard LeftControl) + 0x29, 0xe7, // USAGE_MAXIMUM (Keyboard Right GUI) + 0x15, 0x00, // LOGICAL_MINIMUM (0) + 0x25, 0x01, // LOGICAL_MAXIMUM (1) + 0x75, 0x01, // REPORT_SIZE (1) + 0x95, 0x08, // REPORT_COUNT (8) + 0x81, 0x02, // INPUT (Data,Var,Abs) + 0x95, 0x01, // REPORT_COUNT (1) + 0x75, 0x08, // REPORT_SIZE (8) + 0x81, 0x03, // INPUT (Cnst,Var,Abs) + 0x95, 0x05, // REPORT_COUNT (5) + 0x75, 0x01, // REPORT_SIZE (1) + 0x05, 0x08, // USAGE_PAGE (LEDs) + 0x19, 0x01, // USAGE_MINIMUM (Num Lock) + 0x29, 0x05, // USAGE_MAXIMUM (Kana) + 0x91, 0x02, // OUTPUT (Data,Var,Abs) + 0x95, 0x01, // REPORT_COUNT (1) + 0x75, 0x03, // REPORT_SIZE (3) + 0x91, 0x03, // OUTPUT (Cnst,Var,Abs) + 0x95, 0x06, // REPORT_COUNT (6) + 0x75, 0x08, // REPORT_SIZE (8) + 0x15, 0x00, // LOGICAL_MINIMUM (0) + 0x25, 0x65, // LOGICAL_MAXIMUM (101) + 0x05, 0x07, // USAGE_PAGE (Keyboard) + 0x19, 0x00, // USAGE_MINIMUM (Reserved (no event indicated)) + 0x29, 0x65, // USAGE_MAXIMUM (Keyboard Application) + 0x81, 0x00, // INPUT (Data,Ary,Abs) + 0xc0, // END_COLLECTION +]; + +#[repr(C, packed)] +#[derive(Debug, Clone, Copy, defmt::Format, zerocopy::FromBytes, zerocopy::IntoBytes, zerocopy::Immutable)] +struct KeyboardOutputReport { + /// LED state bits + pub leds: u8, +} + +/// Relay adapter that presents the mock keyboard to the HID-I2C service as a [`HidDevice`]. +/// +pub struct MockKeyboardHidRelay<'s, Service: KeyboardService<'s>> { + service: &'s mut Service, + subscriber: embassy_sync::pubsub::DynSubscriber<'s, KeyboardInputReport>, + + descriptor: HidReportDescriptor<'static>, + + pending_input_report: Option, +} + +impl<'s, Service: KeyboardService<'s>> MockKeyboardHidRelay<'s, Service> { + pub fn new(service: &'s mut Service) -> Self { + let subscriber = service + .subscriber() + .expect("keyboard service didn't have enough subscriber slots to create a relay"); + Self { + service, + subscriber, + descriptor: HidReportDescriptor::new(KEYBOARD_HID_REPORT_DESCRIPTOR_NO_ID) + .expect("keyboard HID report descriptor should be valid"), + pending_input_report: None, + } + } +} + +impl<'s, Service: KeyboardService<'s>> embedded_services::relay::hid::HidDevice for MockKeyboardHidRelay<'s, Service> { + type InputReportMaxSize = typenum::U8; + type OutputReportMaxSize = typenum::U1; + type FeatureReportMaxSize = typenum::U0; + + const MAX_REPORT_COUNT: u8 = 1; + + fn report_descriptor(&self) -> &HidReportDescriptor<'_> { + &self.descriptor + } + + async fn process_get_report( + &mut self, + _report_type: GetHidReportType, + report_id: ReportId, + process_report: impl AsyncFnOnce(GetHidReport<'_>) -> R, + ) -> Result { + info!("Received command to get report with ID {:?}", report_id); + if report_id == REPORTID_KEYBOARD_NO_ID { + let report = KeyboardInputReport::default(); + Ok(process_report(GetHidReport::Input(HidReport::new(report_id, report.as_bytes()))).await) + } else { + info!("Report ID {:?} not recognized", report_id); + Err(HidError::TriggerReset) + } + } + + async fn set_report(&mut self, report: &SetHidReport<'_>) -> Result<(), HidError> { + match report { + SetHidReport::Output(r) if r.id() == REPORTID_KEYBOARD_NO_ID => { + let output_report = KeyboardOutputReport::read_from_bytes(r.data()).unwrap(); + self.service.set_led(output_report.leds).await; + } + SetHidReport::Output(r) => { + info!("Report ID {:?} not recognized", r.id()); + return Err(HidError::TriggerReset); + } + SetHidReport::Feature(r) => info!("Received command to set feature report with ID {:?}", r.id()), + } + Ok(()) + } + + async fn wait_for_input_report(&mut self) { + if self.pending_input_report.is_none() { + self.pending_input_report = Some(self.subscriber.next_message_pure().await); + } + } + + async fn process_next_input_report( + &mut self, + process_report: impl AsyncFnOnce(HidReport<'_>) -> R, + ) -> Result { + self.wait_for_input_report().await; + + Ok(process_report(HidReport::new( + REPORTID_KEYBOARD_NO_ID, + self.pending_input_report + .take() + .expect("We just forced this to be populated in wait_for_input_report()") + .as_bytes(), + )) + .await) + } + + fn has_pending_input_report(&mut self) -> bool { + self.pending_input_report.is_some() || !self.subscriber.is_empty() + } + + async fn set_power_state(&mut self, state: HidDevicePowerState) -> Result<(), HidError> { + info!("Received command to set power state to {:?}", state); + Ok(()) + } + + async fn reset(&mut self) { + info!("Received reset command"); + self.pending_input_report = None; + self.subscriber.clear(); + } +} From ebcc8f25fe87347bc8d4de49ec2bf3ad15e31665 Mon Sep 17 00:00:00 2001 From: Billy Price Date: Thu, 30 Jul 2026 14:48:43 -0700 Subject: [PATCH 15/17] refactor mouse mock to better demonstrate correct usage --- examples/rt685s-evk/src/bin/mock_i2c_mouse.rs | 17 +- .../rt685s-evk/src/mocks/keyboard/device.rs | 3 - .../src/mocks/keyboard/interface.rs | 2 +- examples/rt685s-evk/src/mocks/mouse.rs | 249 ------------------ examples/rt685s-evk/src/mocks/mouse/device.rs | 115 ++++++++ .../rt685s-evk/src/mocks/mouse/interface.rs | 24 ++ examples/rt685s-evk/src/mocks/mouse/mod.rs | 3 + examples/rt685s-evk/src/mocks/mouse/relay.rs | 142 ++++++++++ 8 files changed, 296 insertions(+), 259 deletions(-) delete mode 100644 examples/rt685s-evk/src/mocks/mouse.rs create mode 100644 examples/rt685s-evk/src/mocks/mouse/device.rs create mode 100644 examples/rt685s-evk/src/mocks/mouse/interface.rs create mode 100644 examples/rt685s-evk/src/mocks/mouse/mod.rs create mode 100644 examples/rt685s-evk/src/mocks/mouse/relay.rs diff --git a/examples/rt685s-evk/src/bin/mock_i2c_mouse.rs b/examples/rt685s-evk/src/bin/mock_i2c_mouse.rs index 1819c4aba..b6f2cd943 100644 --- a/examples/rt685s-evk/src/bin/mock_i2c_mouse.rs +++ b/examples/rt685s-evk/src/bin/mock_i2c_mouse.rs @@ -8,7 +8,10 @@ use embassy_imxrt::i2c::slave::{Address, I2cSlave}; use embassy_imxrt::i2c::{self, Async}; use embassy_imxrt::{bind_interrupts, peripherals}; use panic_probe as _; -use rt685s_evk_example::mocks::mouse::{MockMouseHidRelay, MockMouseResources, MockMouseService}; +use rt685s_evk_example::mocks::mouse::{ + device::{MockMouseService, MockMouseServiceResources}, + relay::MockMouseHidRelay, +}; use static_cell::StaticCell; const SLAVE_ADDR: Option
= Address::new(0x15); @@ -33,9 +36,11 @@ async fn main(spawner: Spawner) { gpio::SlewRate::Standard, ); - static MOUSE_RESOURCES: StaticCell = StaticCell::new(); - let mouse_resources = MOUSE_RESOURCES.init(MockMouseResources::default()); - let (mouse_service, mut mouse_runner) = MockMouseService::new(mouse_resources); + const MOUSE_SUBS: usize = 1; + static MOUSE_RESOURCES: StaticCell> = StaticCell::new(); + let (mouse_service, mouse_runner) = MockMouseService::new(MOUSE_RESOURCES.init(Default::default())); + + static MOUSE_SERVICE: StaticCell> = StaticCell::new(); // NOTE: here's where the "aggregate HID devices" macro is currently missing. Compare with time_alarm.rs where we do this: // @@ -67,13 +72,13 @@ async fn main(spawner: Spawner) { 'static, I2cSlave<'static, Async>, gpio::Output<'static>, - MockMouseHidRelay<'static>, + MockMouseHidRelay<'static, MockMouseService<'static, MOUSE_SUBS>>, >, |resources| hidi2c_target_service::Service::new( resources, i2c, attn_pin, - MockMouseHidRelay::new(mouse_service), + MockMouseHidRelay::new(MOUSE_SERVICE.init(mouse_service)), hidi2c_target_service::HardwareVersionInfo { vendor_id: hidi2c_target_service::VendorId::new(0x1234).unwrap(), product_id: hidi2c_target_service::ProductId(0x5678), diff --git a/examples/rt685s-evk/src/mocks/keyboard/device.rs b/examples/rt685s-evk/src/mocks/keyboard/device.rs index 5e29424f5..b22c9e558 100644 --- a/examples/rt685s-evk/src/mocks/keyboard/device.rs +++ b/examples/rt685s-evk/src/mocks/keyboard/device.rs @@ -1,7 +1,4 @@ //! Mock HID keyboard device used by the `mock_i2c_keyboard` example. -//! -//! Unlike the mouse mock, the keyboard copies each report out of a regular channel, which is simpler, -//! but can be costly for larger reports. use super::interface::*; use embedded_services::{info, warn}; diff --git a/examples/rt685s-evk/src/mocks/keyboard/interface.rs b/examples/rt685s-evk/src/mocks/keyboard/interface.rs index adff9b8d7..3d69cb892 100644 --- a/examples/rt685s-evk/src/mocks/keyboard/interface.rs +++ b/examples/rt685s-evk/src/mocks/keyboard/interface.rs @@ -1,4 +1,4 @@ -///! A simplified interface for a hypothetical keyboard service. +//! A simplified interface for a hypothetical keyboard service. pub trait KeyboardService<'hw> { fn set_led(&mut self, state: u8) -> impl core::future::Future + Send; diff --git a/examples/rt685s-evk/src/mocks/mouse.rs b/examples/rt685s-evk/src/mocks/mouse.rs deleted file mode 100644 index da115a9b9..000000000 --- a/examples/rt685s-evk/src/mocks/mouse.rs +++ /dev/null @@ -1,249 +0,0 @@ -//! Mock HID mouse device used by the `mock_i2c_mouse` example -//! -//! This demonstrates a zero-copy input path: input reports live in a caller-provided ring buffer -//! and are written/read *in place, which saves a memcpy at the cost of some complexity. - -use defmt::info; -use embassy_sync::zerocopy_channel; -use embedded_services::GlobalRawMutex; -use embedded_services::relay::hid::*; -use zerocopy::IntoBytes; - -// This is adapted from the example mouse HID descriptor packaged with the DT.exe tool / https://learn.microsoft.com/en-us/windows-hardware/design/component-guidelines/mouse-collection-report-descriptor -const REPORTID_MOUSE: u8 = 1; - -#[rustfmt::skip] -const MOUSE_HID_REPORT_DESCRIPTOR: &[u8] = &[ - 0x05, 0x01, // Usage Page (Generic Desktop Ctrls) - 0x09, 0x02, // Usage (Mouse) - 0xA1, 0x01, // Collection (Application) - 0x85, REPORTID_MOUSE, // REPORT_ID (Touch pad) **** THIS IS ADAPTED FROM SAMPLE TOUCHPAD DESCRIPTOR, THE MOUSE EXAMPLE OMITTED IT BECAUSSE IT ONLY HAD 1 REPORT - 0x09, 0x01, // Usage (Pointer) - 0xA1, 0x00, // Collection (Physical) - 0x05, 0x09, // Usage Page (Button) - 0x19, 0x01, // Usage Minimum (0x01) - 0x29, 0x03, // Usage Maximum (0x03) - 0x15, 0x00, // Logical Minimum (0) - 0x25, 0x01, // Logical Maximum (1) - 0x95, 0x03, // Report Count (3) - 0x75, 0x01, // Report Size (1) - 0x81, 0x02, // Input (Data,Var,Abs,No Wrap,Linear,Preferred State,No Null Position) - 0x95, 0x01, // Report Count (1) - 0x75, 0x05, // Report Size (5) - 0x81, 0x03, // Input (Const,Var,Abs,No Wrap,Linear,Preferred State,No Null Position) - 0x05, 0x01, // Usage Page (Generic Desktop Ctrls) - 0x09, 0x30, // Usage (X) - 0x09, 0x31, // Usage (Y) - 0x15, 0x81, // Logical Minimum (-127) - 0x25, 0x7F, // Logical Maximum (127) - 0x75, 0x08, // Report Size (8) - 0x95, 0x02, // Report Count (2) - 0x81, 0x06, // Input (Data,Var,Rel,No Wrap,Linear,Preferred State,No Null Position) - 0xC0, // End Collection - 0xC0, // End Collection -]; - -const MOUSE_BUTTON_1: u8 = 0x01; -#[allow(dead_code)] -const MOUSE_BUTTON_2: u8 = 0x02; -#[allow(dead_code)] -const MOUSE_BUTTON_3: u8 = 0x04; - -#[repr(C)] -#[derive(Debug, Default, defmt::Format, zerocopy::FromBytes, zerocopy::IntoBytes, zerocopy::Immutable)] -struct MouseReport { - buttons: u8, // 3 bits used for buttons, 5 bits padding - x: i8, - y: i8, -} - -// Number of in-flight input reports the zero-copy channel can hold. Depth >= 2 lets the producer -// stage the next report while the consumer is still handing the current one to the host. -const MOUSE_CHANNEL_DEPTH: usize = 4; - -type MouseChannel<'d> = zerocopy_channel::Channel<'d, GlobalRawMutex, MouseReport>; -type MouseSender<'d> = zerocopy_channel::Sender<'d, GlobalRawMutex, MouseReport>; -type MouseReceiver<'d> = zerocopy_channel::Receiver<'d, GlobalRawMutex, MouseReport>; - -/// Memory resources for the mock mouse service. -/// -/// Following the pattern used by services like `time_alarm_service` and `hidi2c_target_service`, the -/// caller allocates a single `Resources` object (for example in a `StaticCell`) and hands a mutable -/// borrow to [`MockMouseService::new`] at construction time, instead of scattering `static` -/// singletons around. The struct is generic over the lifetime `'d` of that borrow rather than being -/// pinned to `'static`. -pub struct MockMouseResources<'d> { - /// Backing storage for the zero-copy channel's ring buffer. - buf: [MouseReport; MOUSE_CHANNEL_DEPTH], - /// The channel itself, created in [`MockMouseService::new`] from a borrow of `buf`. - channel: Option>, -} - -impl Default for MockMouseResources<'_> { - fn default() -> Self { - Self { - buf: core::array::from_fn(|_| MouseReport::default()), - channel: None, - } - } -} - -/// Consumer side of the mock mouse. Owns the channel `Receiver` and hands the host borrows that -/// point directly into the ring buffer — no intermediate copy of the report payload. -pub struct MockMouseService<'hw> { - receiver: MouseReceiver<'hw>, -} - -impl<'hw> MockMouseService<'hw> { - /// Wire up the zero-copy channel inside `resources` and split it into the service (which the - /// relay borrows) and a runner (used to inject mouse events). - pub fn new(resources: &'hw mut MockMouseResources<'hw>) -> (Self, MockMouseRunner<'hw>) { - // Destructure so the buffer and channel fields are borrowed disjointly: the channel borrows - // the buffer in place, and both live as long as `resources`. - let MockMouseResources { buf, channel } = resources; - let channel = channel.insert(MouseChannel::new(buf)); - let (sender, receiver) = channel.split(); - (Self { receiver }, MockMouseRunner { sender }) - } - - // Event receiver for the service. If you truly need this sort of zero-copy approach all the way from the service through the relay adapter to the tranport, - // you may have to make some layout concessions in the service. This implies that it's probably not possible to support zerocopy for a given message type - // across multiple tranports that have different layout requirements, and some knowledge of which layout to use may need to leak into the service for - // services that truly need zero-copy messages. This is probably not going to be common, though - in the case of a mouse, for example, the biggest report is - // 3 bytes, so the copy is trivial and the service in practice would just use a regular channel rather than a zerocopy channel. - fn receiver(&mut self) -> &mut MouseReceiver<'hw> { - &mut self.receiver - } -} - -/// In a production case, this would implement the Runner trait and be spawned as a task, but for the mock we just expose methods to inject mouse events -pub struct MockMouseRunner<'d> { - sender: MouseSender<'d>, -} - -impl MockMouseRunner<'_> { - /// Acquire the next free slot in the ring buffer and populate it in place, then publish it. - async fn send(&mut self, report: MouseReport) { - // `send()` waits for a free slot and yields `&mut MouseReport` pointing straight into the - // channel's ring buffer. Writing through it avoids copying the payload into the channel. - let slot = self.sender.send().await; - *slot = report; - // Publish the slot to the consumer. Nothing is copied here either. - self.sender.send_done(); - } - - pub async fn send_click(&mut self) { - // Mouse down - self.send(MouseReport { - buttons: MOUSE_BUTTON_1, - x: 0, - y: 0, - }) - .await; - - embassy_time::Timer::after(embassy_time::Duration::from_millis(15)).await; - - // Mouse up - self.send(MouseReport { buttons: 0, x: 0, y: 0 }).await; - } - - pub async fn move_mouse(&mut self) { - self.send(MouseReport { - buttons: MOUSE_BUTTON_1, - x: 10, - y: 10, - }) - .await; - } -} - -/// Relay adapter that presents the mock mouse to the HID-I2C service as a [`HidDevice`]. -pub struct MockMouseHidRelay<'d> { - service: MockMouseService<'d>, - descriptor: HidReportDescriptor<'static>, -} - -impl<'d> MockMouseHidRelay<'d> { - pub fn new(service: MockMouseService<'d>) -> Self { - Self { - service, - descriptor: HidReportDescriptor::new(MOUSE_HID_REPORT_DESCRIPTOR) - .expect("mouse HID report descriptor should be valid"), - } - } -} - -impl embedded_services::relay::hid::HidDevice for MockMouseHidRelay<'_> { - type InputReportMaxSize = typenum::U3; - type OutputReportMaxSize = typenum::U0; - type FeatureReportMaxSize = typenum::U0; - - const MAX_REPORT_COUNT: u8 = 3; - - fn report_descriptor(&self) -> &HidReportDescriptor<'_> { - &self.descriptor - } - - async fn process_get_report( - &mut self, - _report_type: GetHidReportType, - report_id: ReportId, - process_report: impl AsyncFnOnce(GetHidReport<'_>) -> R, - ) -> Result { - info!("Received command to get report with ID {:?}", report_id); - match report_id { - ReportId(REPORTID_MOUSE) => { - let report = MouseReport::default(); - Ok(process_report(GetHidReport::Input(HidReport::new(report_id, report.as_bytes()))).await) - } - _ => { - info!("Report ID {:?} not recognized", report_id); - Err(HidError::TriggerReset) - } - } - } - - async fn set_report(&mut self, report: &SetHidReport<'_>) -> Result<(), HidError> { - match report { - SetHidReport::Output(r) => info!("Received command to set output report with ID {:?}", r.id()), - SetHidReport::Feature(r) => info!("Received command to set feature report with ID {:?}", r.id()), - } - info!("SET_REPORT NOT IMPLEMENTED"); - Ok(()) - } - - async fn wait_for_input_report(&mut self) { - // `receive()` only peeks at the front slot - it doesn't treat the sample as consumed until `receive_done()` is called. - // Therefore, we can do this to wait until a report is ready, and then immediately drop it without losing the report. - let _ = self.service.receiver().receive().await; - } - - async fn process_next_input_report( - &mut self, - process_report: impl AsyncFnOnce(HidReport<'_>) -> R, - ) -> Result { - // Borrow the next report out of the channel and lend it to the transport for the duration of `process_report`. - // This is probably unnecessary for mice because of how small the reports are, but it demonstrates the technique. - // For a mouse it may make more sense to use a traditional Channel and just copy the 3 bytes around. - let slot = self.service.receiver().receive().await; - let result = process_report(HidReport::new(ReportId(REPORTID_MOUSE), slot.as_bytes())).await; - - // The transport is done with the borrow, so return the slot to the producer. This is required by zerocopy_channel. - self.service.receiver().receive_done(); - Ok(result) - } - - fn has_pending_input_report(&mut self) -> bool { - !self.service.receiver().is_empty() - } - - async fn set_power_state(&mut self, state: HidDevicePowerState) -> Result<(), HidError> { - info!("Received command to set power state to {:?}", state); - Ok(()) - } - - async fn reset(&mut self) { - info!("Received reset command"); - self.service.receiver().clear(); - } -} diff --git a/examples/rt685s-evk/src/mocks/mouse/device.rs b/examples/rt685s-evk/src/mocks/mouse/device.rs new file mode 100644 index 000000000..2c0df977b --- /dev/null +++ b/examples/rt685s-evk/src/mocks/mouse/device.rs @@ -0,0 +1,115 @@ +//! Mock HID mouse device used by the `mock_i2c_mouse` example. + +use super::interface::*; +use embedded_services::warn; + +/// Depth of the mouse input-report channel. In a production use case, you may want to make the service generic over this value rather than hardcoding it. +const MOUSE_CHANNEL_DEPTH: usize = 4; + +const MOUSE_BUTTON_1: u8 = 0x01; +#[allow(dead_code)] +const MOUSE_BUTTON_2: u8 = 0x02; +#[allow(dead_code)] +const MOUSE_BUTTON_3: u8 = 0x04; + +struct MockMouseServiceResourcesInner { + channel: embassy_sync::pubsub::PubSubChannel< + embedded_services::GlobalRawMutex, + MouseInputReport, + MOUSE_CHANNEL_DEPTH, + MAX_SUBS, + 1, + >, +} + +pub struct MockMouseServiceResources { + inner: Option>, +} + +impl Default for MockMouseServiceResources { + fn default() -> Self { + Self { + inner: Some(MockMouseServiceResourcesInner { + channel: embassy_sync::pubsub::PubSubChannel::new(), + }), + } + } +} + +/// Consumer side of the mock mouse. Owns the channel that carries input reports and exposes methods +/// to inject mouse events. +pub struct MockMouseService<'hw, const MAX_SUBS: usize> { + resources: &'hw MockMouseServiceResourcesInner, +} + +impl<'hw, const MAX_SUBS: usize> MockMouseService<'hw, MAX_SUBS> { + pub fn new( + resources: &'hw mut MockMouseServiceResources, + ) -> (Self, MockMouseServiceRunner<'hw, MAX_SUBS>) { + let resources = resources.inner.insert(MockMouseServiceResourcesInner { + channel: embassy_sync::pubsub::PubSubChannel::new(), + }); + ( + Self { resources }, + MockMouseServiceRunner { + publisher: resources + .channel + .publisher() + .expect("We know there's a free publisher because we just created the channel"), + }, + ) + } +} + +pub struct MockMouseServiceRunner<'hw, const MAX_SUBS: usize> { + publisher: embassy_sync::pubsub::Publisher< + 'hw, + embedded_services::GlobalRawMutex, + MouseInputReport, + MOUSE_CHANNEL_DEPTH, + MAX_SUBS, + 1, + >, +} + +// Note: on a normal mouse service, this would just have a run() method that actually polls the sensor. +// For the mock, we just let the user inject mouse events instead. +impl<'hw, const MAX_SUBS: usize> MockMouseServiceRunner<'hw, MAX_SUBS> { + async fn send(&self, report: MouseInputReport) { + if let Err(e) = self.publisher.try_publish(report) { + warn!("Failed to send mouse report: {:?}", e); + } + } + + pub async fn send_click(&self) { + // Mouse down + self.send(MouseInputReport { + buttons: MOUSE_BUTTON_1, + x: 0, + y: 0, + }) + .await; + + embassy_time::Timer::after(embassy_time::Duration::from_millis(15)).await; + + // Mouse up + self.send(MouseInputReport { buttons: 0, x: 0, y: 0 }).await; + } + + pub async fn move_mouse(&self) { + self.send(MouseInputReport { + buttons: MOUSE_BUTTON_1, + x: 10, + y: 10, + }) + .await; + } +} + +impl<'hw, const MAX_SUBS: usize> MouseService<'hw> for MockMouseService<'hw, MAX_SUBS> { + fn subscriber( + &self, + ) -> Result, embassy_sync::pubsub::Error> { + self.resources.channel.dyn_subscriber() + } +} diff --git a/examples/rt685s-evk/src/mocks/mouse/interface.rs b/examples/rt685s-evk/src/mocks/mouse/interface.rs new file mode 100644 index 000000000..bf263f51c --- /dev/null +++ b/examples/rt685s-evk/src/mocks/mouse/interface.rs @@ -0,0 +1,24 @@ +//! A simplified interface for a hypothetical mouse service. +pub trait MouseService<'hw> { + fn subscriber( + &self, + ) -> Result, embassy_sync::pubsub::Error>; +} + +/// A report from the mouse. Note that mice are very HID-centric so it makes sense that this has the same +/// underlying representation as a HID mouse input report, but this is an optimization that may not make sense for +/// other service types (e.g. time-alarm may just emit a non-repr(C) enum of the kind of event and leave it up to +/// the relay handler to figure out how to translate that to HID (or MCTP or some other logical protocol). +/// +#[repr(C)] +#[derive(Debug, Clone, Copy, Default, defmt::Format, zerocopy::FromBytes, zerocopy::IntoBytes, zerocopy::Immutable)] +pub struct MouseInputReport { + /// 3 bits used for buttons, 5 bits padding + pub buttons: u8, + + /// Relative X movement + pub x: i8, + + /// Relative Y movement + pub y: i8, +} diff --git a/examples/rt685s-evk/src/mocks/mouse/mod.rs b/examples/rt685s-evk/src/mocks/mouse/mod.rs new file mode 100644 index 000000000..18f9c7d3e --- /dev/null +++ b/examples/rt685s-evk/src/mocks/mouse/mod.rs @@ -0,0 +1,3 @@ +pub mod device; +pub mod interface; +pub mod relay; diff --git a/examples/rt685s-evk/src/mocks/mouse/relay.rs b/examples/rt685s-evk/src/mocks/mouse/relay.rs new file mode 100644 index 000000000..e366aebb4 --- /dev/null +++ b/examples/rt685s-evk/src/mocks/mouse/relay.rs @@ -0,0 +1,142 @@ +use super::interface::*; +use defmt::info; +use embedded_services::relay::hid::*; +use zerocopy::IntoBytes; + +// This is adapted from the example mouse HID descriptor packaged with the DT.exe tool / https://learn.microsoft.com/en-us/windows-hardware/design/component-guidelines/mouse-collection-report-descriptor +const REPORTID_MOUSE: u8 = 1; + +#[rustfmt::skip] +const MOUSE_HID_REPORT_DESCRIPTOR: &[u8] = &[ + 0x05, 0x01, // Usage Page (Generic Desktop Ctrls) + 0x09, 0x02, // Usage (Mouse) + 0xA1, 0x01, // Collection (Application) + 0x85, REPORTID_MOUSE, // REPORT_ID (mouse report) + 0x09, 0x01, // Usage (Pointer) + 0xA1, 0x00, // Collection (Physical) + 0x05, 0x09, // Usage Page (Button) + 0x19, 0x01, // Usage Minimum (0x01) + 0x29, 0x03, // Usage Maximum (0x03) + 0x15, 0x00, // Logical Minimum (0) + 0x25, 0x01, // Logical Maximum (1) + 0x95, 0x03, // Report Count (3) + 0x75, 0x01, // Report Size (1) + 0x81, 0x02, // Input (Data,Var,Abs,No Wrap,Linear,Preferred State,No Null Position) + 0x95, 0x01, // Report Count (1) + 0x75, 0x05, // Report Size (5) + 0x81, 0x03, // Input (Const,Var,Abs,No Wrap,Linear,Preferred State,No Null Position) + 0x05, 0x01, // Usage Page (Generic Desktop Ctrls) + 0x09, 0x30, // Usage (X) + 0x09, 0x31, // Usage (Y) + 0x15, 0x81, // Logical Minimum (-127) + 0x25, 0x7F, // Logical Maximum (127) + 0x75, 0x08, // Report Size (8) + 0x95, 0x02, // Report Count (2) + 0x81, 0x06, // Input (Data,Var,Rel,No Wrap,Linear,Preferred State,No Null Position) + 0xC0, // End Collection + 0xC0, // End Collection +]; + +/// Relay adapter that presents the mock mouse to the HID-I2C service as a [`HidDevice`]. +pub struct MockMouseHidRelay<'s, Service: MouseService<'s>> { + _service: &'s mut Service, // This simplified example mouse doesn't include output/feature reports to control e.g. mouse DPI settings, but if they did, you'd route them through this + subscriber: embassy_sync::pubsub::DynSubscriber<'s, MouseInputReport>, + + descriptor: HidReportDescriptor<'static>, + + pending_input_report: Option, +} + +impl<'s, Service: MouseService<'s>> MockMouseHidRelay<'s, Service> { + pub fn new(service: &'s mut Service) -> Self { + let subscriber = service + .subscriber() + .expect("mouse service didn't have enough subscriber slots to create a relay"); + Self { + _service: service, + subscriber, + descriptor: HidReportDescriptor::new(MOUSE_HID_REPORT_DESCRIPTOR) + .expect("mouse HID report descriptor should be valid"), + pending_input_report: None, + } + } +} + +impl<'s, Service: MouseService<'s>> embedded_services::relay::hid::HidDevice for MockMouseHidRelay<'s, Service> { + type InputReportMaxSize = typenum::U3; + type OutputReportMaxSize = typenum::U0; + type FeatureReportMaxSize = typenum::U0; + + const MAX_REPORT_COUNT: u8 = 3; + + fn report_descriptor(&self) -> &HidReportDescriptor<'_> { + &self.descriptor + } + + async fn process_get_report( + &mut self, + _report_type: GetHidReportType, + report_id: ReportId, + process_report: impl AsyncFnOnce(GetHidReport<'_>) -> R, + ) -> Result { + info!("Received command to get report with ID {:?}", report_id); + match report_id { + ReportId(REPORTID_MOUSE) => { + let report = MouseInputReport::default(); + Ok(process_report(GetHidReport::Input(HidReport::new(report_id, report.as_bytes()))).await) + } + _ => { + info!("Report ID {:?} not recognized", report_id); + Err(HidError::TriggerReset) + } + } + } + + async fn set_report(&mut self, report: &SetHidReport<'_>) -> Result<(), HidError> { + match report { + SetHidReport::Output(r) => info!("Received command to set output report with ID {:?}", r.id()), + SetHidReport::Feature(r) => info!("Received command to set feature report with ID {:?}", r.id()), + } + info!("SET_REPORT NOT IMPLEMENTED"); + Ok(()) + } + + async fn wait_for_input_report(&mut self) { + // Peek the next report into a local slot so a report is never lost if the future driving this + // relay is dropped by a `select`-style combinator before `process_next_input_report` runs. + if self.pending_input_report.is_none() { + self.pending_input_report = Some(self.subscriber.next_message_pure().await); + } + } + + async fn process_next_input_report( + &mut self, + process_report: impl AsyncFnOnce(HidReport<'_>) -> R, + ) -> Result { + self.wait_for_input_report().await; + + Ok(process_report(HidReport::new( + ReportId(REPORTID_MOUSE), + self.pending_input_report + .take() + .expect("We just forced this to be populated in wait_for_input_report()") + .as_bytes(), + )) + .await) + } + + fn has_pending_input_report(&mut self) -> bool { + self.pending_input_report.is_some() || !self.subscriber.is_empty() + } + + async fn set_power_state(&mut self, state: HidDevicePowerState) -> Result<(), HidError> { + info!("Received command to set power state to {:?}", state); + Ok(()) + } + + async fn reset(&mut self) { + info!("Received reset command"); + self.pending_input_report = None; + self.subscriber.clear(); + } +} From da07af8c84e3b29ccc7efa9cc12e14fecc5ce183 Mon Sep 17 00:00:00 2001 From: Billy Price Date: Thu, 30 Jul 2026 15:08:38 -0700 Subject: [PATCH 16/17] pr feedback --- hidi2c-target-service/src/attn_pin_handler.rs | 16 +++++++-------- hidi2c-target-service/src/service.rs | 20 ++++++++++++++----- 2 files changed, 22 insertions(+), 14 deletions(-) diff --git a/hidi2c-target-service/src/attn_pin_handler.rs b/hidi2c-target-service/src/attn_pin_handler.rs index 4093965c9..003cb578a 100644 --- a/hidi2c-target-service/src/attn_pin_handler.rs +++ b/hidi2c-target-service/src/attn_pin_handler.rs @@ -17,26 +17,24 @@ impl AttnPinHandler { attn_pin, asserted: false, }; - result.clear_interrupt(); + let _ = result.clear_interrupt(); result } /// Clear the interrupt, which is done by setting the pin high. - pub(crate) fn clear_interrupt(&mut self) { + pub(crate) fn clear_interrupt(&mut self) -> Result<(), AttnPin::Error> { trace!("HID-I2C: ATTN: clear interrupt"); - self.attn_pin - .set_high() - .unwrap_or_else(|_| error!("HID-I2C: Failed to clear interrupt on attn pin")); + self.attn_pin.set_high()?; self.asserted = false; + Ok(()) } /// Assert the interrupt, which is done by pulling the pin low. - pub(crate) fn assert_interrupt(&mut self) { + pub(crate) fn assert_interrupt(&mut self) -> Result<(), AttnPin::Error> { trace!("HID-I2C: ATTN: assert interrupt"); - self.attn_pin - .set_low() - .unwrap_or_else(|_| error!("HID-I2C: Failed to assert interrupt on attn pin")); + self.attn_pin.set_low()?; self.asserted = true; + Ok(()) } /// Returns true if we are asserting the interrupt, false otherwise. diff --git a/hidi2c-target-service/src/service.rs b/hidi2c-target-service/src/service.rs index 10a631f48..8f8a49b8c 100644 --- a/hidi2c-target-service/src/service.rs +++ b/hidi2c-target-service/src/service.rs @@ -305,7 +305,9 @@ impl< } embassy_futures::select::Either3::Second(()) => { trace!("HID-I2C: Signalling host that an input report is ready"); - self.attn_pin.assert_interrupt(); + self.attn_pin + .assert_interrupt() + .unwrap_or_else(|_| error!("HID-I2C: Failed to assert interrupt on attn pin")); } embassy_futures::select::Either3::Third(()) => { trace!("HID-I2C: Received reset request"); @@ -480,7 +482,9 @@ impl< self.bus.write(&[00, 00]).await?; self.pending_reset = false; - self.attn_pin.clear_interrupt(); + self.attn_pin + .clear_interrupt() + .unwrap_or_else(|_| error!("HID-I2C: Failed to clear interrupt on attn pin")); return Ok(()); } @@ -492,7 +496,9 @@ impl< if !self.hid_device.has_pending_input_report() { warn!("HID-I2C: Host polled when no input report was pending; responding with zero-length report"); self.bus.write(&[00, 00]).await?; - self.attn_pin.clear_interrupt(); + self.attn_pin + .clear_interrupt() + .unwrap_or_else(|_| error!("HID-I2C: Failed to clear interrupt on attn pin")); return Ok(()); } @@ -521,7 +527,9 @@ impl< .await??; if !self.hid_device.has_pending_input_report() { - self.attn_pin.clear_interrupt(); + self.attn_pin + .clear_interrupt() + .unwrap_or_else(|_| error!("HID-I2C: Failed to clear interrupt on attn pin")); } Ok(()) @@ -650,7 +658,9 @@ impl< warn!("HID-I2C: Executing device reset"); self.hid_device.reset().await; self.pending_reset = true; - self.attn_pin.assert_interrupt(); + self.attn_pin + .assert_interrupt() + .unwrap_or_else(|_| error!("HID-I2C: Failed to assert interrupt on attn pin")); } } From 58c9c14cce05e0d8bf96629acb38521381acebdd Mon Sep 17 00:00:00 2001 From: Billy Price Date: Fri, 31 Jul 2026 09:57:26 -0700 Subject: [PATCH 17/17] fix timeout recovery when host writes more bytes than we told it we could accept --- hidi2c-target-service/src/service.rs | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/hidi2c-target-service/src/service.rs b/hidi2c-target-service/src/service.rs index 8f8a49b8c..4fead52a6 100644 --- a/hidi2c-target-service/src/service.rs +++ b/hidi2c-target-service/src/service.rs @@ -195,9 +195,14 @@ impl TimeoutBus { self.timeout_settings.data_read_timeout, self.bus.respond_to_write(&mut discard_buffer), ) - .await? - .map_err(Error::Bus)?; - match result { + .await; + + let Ok(result) = result else { + self.bus.recover().await.map_err(Error::Bus)?; + return Err(Error::Protocol(ProtocolError::Timeout)); + }; + + match result.map_err(Error::Bus)? { WriteStatus::BufferFull(_bytes) => { continue; }