Skip to content

time-alarm-service: initial implementation of HID protocol relay handler - #967

Merged
williampMSFT merged 7 commits into
OpenDevicePartnership:mainfrom
williampMSFT:user/williamp/hid-tad
Oct 2, 2026
Merged

williampMSFT merged 7 commits into
OpenDevicePartnership:mainfrom
williampMSFT:user/williamp/hid-tad

Conversation

@williampMSFT

Copy link
Copy Markdown
Collaborator

This provides an implementation of the HID protocol relay handler for the time-alarm service based on the current draft of the HID time-alarm device spec.

To support this, it also introduces some support functions/macros for packing/unpacking HID reports.

The draft spec is subject to change, and in particular currently has some limitations around wake policy and expired timers that are being litigated and will almost certainly change in some capacity.

The actual waking is supposed to be done out-of-band (since a time-alarm device is responsible for powering on the device when it's powered off and the HID transport itself is therefore also powered off), but we don't have a power service implemented yet so that's out of scope for this change.

@williampMSFT
williampMSFT force-pushed the user/williamp/hid-tad branch from 495090b to 011247c Compare October 1, 2026 00:20
@williampMSFT

Copy link
Copy Markdown
Collaborator Author

Tested in conjunction with OpenDevicePartnership/odp-windows-drivers#13

@williampMSFT
williampMSFT marked this pull request as ready for review October 1, 2026 17:32
@williampMSFT
williampMSFT requested a review from a team as a code owner October 1, 2026 17:32
@williampMSFT
williampMSFT requested review from RobertZ2011, felipebalbi, jerrysxie and tullom and a balanced review from Copilot October 1, 2026 17:32

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Time-zone null handling and report-count metadata are incorrect, and the MCTP reorganization breaks existing public paths.

Review effort: Balanced
Findings: 1 High severity · 3 Medium severity

Open (4)
What changed in this PR

Adds HID protocol support for the time-alarm service while retaining MCTP relay functionality.

Changes:

  • Adds HID descriptors, report serialization, and relay handling.
  • Introduces reusable bit-level HID report utilities.
  • Adds an RT685 HID example and reorganizes MCTP exports.
File Description
time-alarm-service-relay/​src/​mctp/​serialization.rs Houses MCTP message serialization.
time-alarm-service-relay/​src/​mctp/​mod.rs Houses the MCTP relay handler.
time-alarm-service-relay/​src/​lib.rs Exposes HID and MCTP modules.
time-alarm-service-relay/​src/​hid/​SystemWakeTimerAndRtc.wara Defines the HID report descriptor source.
time-alarm-service-relay/​src/​hid/​serialization.rs Implements HID report encoding and descriptor data.
time-alarm-service-relay/​src/​hid/​mod.rs Implements the time-alarm HID device adapter.
time-alarm-service-relay/​Cargo.toml Adds HID-related dependency and feature wiring.
examples/​rt685s-evk/​src/​bin/​time_alarm_mctp.rs Updates the MCTP handler path.
examples/​rt685s-evk/​src/​bin/​time_alarm_hid.rs Adds an RT685 HID-over-I²C example.
examples/​rt685s-evk/​Cargo.lock Records the example dependency update.
embedded-service/​src/​relay/​hid/​reports.rs Adds generic bit-level HID report utilities.
embedded-service/​src/​relay/​hid/​mod.rs Exports the report utilities and macro.
Cargo.lock Records the workspace dependency update.

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread time-alarm-service-relay/src/hid/mod.rs Outdated
Comment thread time-alarm-service-relay/src/hid/serialization.rs Outdated
Comment thread time-alarm-service-relay/src/hid/serialization.rs Outdated
Comment thread time-alarm-service-relay/src/lib.rs
kurtjd
kurtjd previously approved these changes Oct 2, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🔵 Needs a closer look

The failed time report emits date values outside the logical ranges declared by its HID descriptor.

Review effort: Balanced
Findings: None

Resolved since last review (4)
Previously missed (1)

In code that hasn't changed since last review

Medium severity Use valid placeholder date values when clock read fails

time-alarm-service-relay/​src/​hid/​serialization.rs:182

Default::default() emits year/month/day as zero, but these input fields are declared with logical ranges 1900–9999, 1–12, and 1–31 and are not marked NullState. Thus a failed clock read sends a report that violates its own HID descriptor. Use in-range placeholder date values while current_state carries Failed.

RobertZ2011
RobertZ2011 previously approved these changes Oct 2, 2026
Comment thread embedded-service/src/relay/hid/reports.rs Outdated

@felipebalbi felipebalbi left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be nice to see tests for src/hid/*

Comment thread time-alarm-service-relay/src/hid/serialization.rs
Comment thread time-alarm-service-relay/src/hid/serialization.rs Outdated
@williampMSFT
williampMSFT merged commit b291460 into OpenDevicePartnership:main Oct 2, 2026
16 of 32 checks passed
@williampMSFT
williampMSFT deleted the user/williamp/hid-tad branch October 2, 2026 21:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants