From 513a55b80749a3a762aac6b3cb0216a119563713 Mon Sep 17 00:00:00 2001 From: Dylan Knutson Date: Tue, 19 May 2026 15:34:10 -0700 Subject: [PATCH] Add SMBus PEC byte to serial transport The serial transport in ec-test-lib currently sends MCTP frames without the trailing SMBus PEC (Packet Error Code) byte that the SMBus protocol specifies as the trailing byte of each frame. embedded-services' uart-service historically compensated for this on the device side by stripping the PEC byte on TX and skipping the PEC byte on RX. embedded-services PR OpenDevicePartnership/embedded-services#852 makes uart-service strictly spec-compliant: SmbusEspiMedium now emits a PEC byte on every TX frame and requires a PEC byte on every RX frame. Without a matching change on the host side, `ec-test-cli --source serial` against any post-#852 uart-service consumer (dev-imxrt, dev-mcxa, dev-npcx, dev-qemu) hangs waiting for a PEC byte the EC never sends, and the extra PEC byte sent by ec-test-cli corrupts the EC's next request. This change adds PEC computation on TX (one byte appended after the existing frame) and verification on RX (one byte read + compared against `smbus_pec::pec()` over the received frame), using the `smbus_pec` crate already in the transitive dependency graph. Coordination note: this PR and embedded-services #852 must land in tight sequence. After #852 merges and `odp-embedded-controller` bumps its `embedded-services` pin past it, `EC_TEST_CLI_REV` in `.github/workflows/check.yml` must be bumped past this commit at the same time, otherwise the `integration-test:` CI job added by OpenDevicePartnership/odp-embedded-controller#14 will fail. Validated end-to-end against `odp-embedded-controller`'s `scripts/integration-test.sh` running dev-qemu built against embedded-services with PR #852 applied: all 18 commands (thermal x8, battery x3, rtc x7) succeed. Assisted-by: GitHub Copilot CLI:claude-opus-4.7-1m-internal --- ec/Cargo.lock | 1 + ec/test-lib/Cargo.toml | 1 + ec/test-lib/src/serial.rs | 12 ++++++++++++ 3 files changed, 14 insertions(+) diff --git a/ec/Cargo.lock b/ec/Cargo.lock index 36fa8f3..1a35313 100644 --- a/ec/Cargo.lock +++ b/ec/Cargo.lock @@ -491,6 +491,7 @@ dependencies = [ "num_enum", "scopeguard", "serialport", + "smbus-pec", "thermal-service-messages", "time-alarm-service-messages", "uuid", diff --git a/ec/test-lib/Cargo.toml b/ec/test-lib/Cargo.toml index 5be842f..195758c 100644 --- a/ec/test-lib/Cargo.toml +++ b/ec/test-lib/Cargo.toml @@ -29,6 +29,7 @@ scopeguard = { version = "1.2" } # Serial specific serialport = { version = "4.8.1" } +smbus-pec = "1.0.1" embedded-services = { git = "https://github.com/OpenDevicePartnership/embedded-services", branch = "v0.2.0" } thermal-service-messages = { git = "https://github.com/OpenDevicePartnership/embedded-services", branch = "v0.2.0" } diff --git a/ec/test-lib/src/serial.rs b/ec/test-lib/src/serial.rs index bca1d94..eb3cb92 100644 --- a/ec/test-lib/src/serial.rs +++ b/ec/test-lib/src/serial.rs @@ -184,6 +184,8 @@ impl Serial { .map_err(|e| Error::Io(format!("{e:?}")))?; port.write_all(&buffer[..HEADER_SZ + request_sz]) .map_err(|e| Error::Io(format!("{e:?}")))?; + let pec = smbus_pec::pec(&buffer[..HEADER_SZ + request_sz]); + port.write_all(&[pec]).map_err(|e| Error::Io(format!("{e:?}")))?; port.flush().map_err(|e| Error::Io(format!("{e:?}")))?; // Read response packets @@ -207,6 +209,16 @@ impl Serial { .ok_or_else(|| Error::Protocol("Response does not fit in buffer".into()))?; port.read_exact(packet_slice).map_err(|e| Error::Io(format!("{e:?}")))?; + let mut pec_buf = [0u8; 1]; + port.read_exact(&mut pec_buf).map_err(|e| Error::Io(format!("{e:?}")))?; + let computed = smbus_pec::pec(&buffer[..SMBUS_HEADER_SZ + len]); + if pec_buf[0] != computed { + return Err(Error::Protocol(format!( + "PEC mismatch: received {:#04x}, computed {:#04x}", + pec_buf[0], computed + ))); + } + let flags = buffer[MCTP_FLAGS_IDX]; // If this is a SOM packet, skip ODP header (we don't use it) and grab the command code/discriminant