Skip to content

ec-test-lib::serial: use mctp-rs directly instead of hand-rolling MCTP framing #93

@dymk

Description

@dymk

Description

A code comment in serial.rs (line 169) explains why this duplication exists:

NOTE: The mctp-rs crate does not appear to support serializing requests and deserializing responses (only the opposite), so we have to do manual serialization until that is changed.

That comment is no longer accurate. MctpPacketContext::serialize_packet supports the host→EC direction and MctpPacketContext::deserialize_packet supports the EC→host direction.

The cost of the duplication is structural fragility: every time the wire format changes in embedded-services, ec-test-lib needs a parallel manual update. The most recent example is embedded-services#852, which made SmbusEspiMedium spec-compliant (emit + verify SMBus PEC byte). ec-test-lib did not emit PEC, so a manual fix was needed — see #92 for that change.

Proposed shape

Prerequisite (embedded-services, separate cross-org PR)

This repo's scope

Rewrite ec/test-lib/src/serial.rs::send to:

  1. Define a host-side HostRequest enum (Battery / Thermal / TimeAlarm) and HostResponse enum. Impl MctpMessageTrait<'_> for both, mirroring the EC-side enums the macro generates.
  2. Construct an OdpHeader { message_type: Request, service, message_id }.
  3. Use MctpPacketContext::<SmbusEspiMedium>::new(...).serialize_packet(reply_context, (header, host_request)), iterate the returned SerializePacketState, write each packet to the UART.
  4. On RX: read bytes, call SmbusEspiMedium::frame_complete(buf) to detect complete frames, then MctpPacketContext::deserialize_packet(frame)Option<MctpMessage> (handles SOM/EOM/reassembly internally). On Some, call MctpMessage::parse_as::<HostResponse>().
  5. Delete prepend_headers, append_cmd, Destination, all 8 hand-defined wire constants, the manual PEC compute/verify, and the multi-packet SOM/EOM loop.

Scope estimate

  • ec/test-lib/Cargo.toml: add mctp-rs as a direct dependency (currently transitive via embedded-services); drop smbus-pec (becomes mctp-rs transitive again).
  • ec/test-lib/src/serial.rs: replace ~80 LOC of hand-rolled framing with ~120 LOC of explicit HostRequest/HostResponse enums + MctpMessageTrait impls + new send(). Net ~+27 LOC but no implicit-flag-bit math, no hand-defined offsets, no parallel-source wire format.
  • No public API change to ec-test-cli (same CLI surface, same on-wire bytes).

Out of scope

  • Adding new media to ec-test-cli (e.g. --source dsp0253-serial using MctpSerialMedium). That becomes trivial once send() is parameterized over M: MctpMedium, but is a separate enhancement.
  • Reorganizing the Acpi (Windows IOCTL) or Mock data sources — they don't construct MCTP packets.

Acceptance

  • cargo build --release -p ec-test-cli and cargo clippy -p ec-test-cli -- -D warnings clean.
  • odp-embedded-controller's scripts/integration-test.sh continues to pass after EC_TEST_CLI_REV is bumped past the merge commit — same 18 commands (thermal × 8, battery × 3, rtc × 7) return valid responses.
  • Wire-format bytes are byte-identical to pre-refactor when cmd_code < 0x8000 (verifiable by capturing UART traffic with socat).
  • The stale code comment at serial.rs:169 is deleted.

Bonus payoff

Once parameterized over M: MctpMedium, a follow-up can add --source dsp0253-serial using MctpSerialMedium, letting ec-test-cli talk to dev-qemu configured with DSP0253 serial transport.

Sequencing

  1. PR on embedded-services exposing OdpHeader + friends — merge first.
  2. This repo's PR consuming the newly-exposed types — merge second.
  3. odp-embedded-controller bumps EC_TEST_CLI_REV.

This lands independently of and after #92 (the PEC fix).

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or request

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions