Make uart-service::Service generic over MctpMedium; delete fake-PEC strips

espi-service/src/espi_service.rs

@@ -163,13 +163,6 @@ impl<'hw, RelayHandler: embedded_services::relay::mctp::RelayHandler> ServiceInn
                     let src_slice =
                         unsafe { slice::from_raw_parts(port_event.base_addr as *const u8, port_event.length) };
 
-                    // TODO: This is a workaround because mctp_rs expects a PEC byte, so we hardcode a 0 at the end.
-                    // We should add functionality to mctp_rs to disable PEC.
-                    let mut with_pec = [0u8; 100];
-                    with_pec[..src_slice.len()].copy_from_slice(src_slice);
-                    with_pec[src_slice.len()] = 0;
-                    let with_pec = &with_pec[..=src_slice.len()];
-
                     #[cfg(feature = "defmt")] // Required because without defmt, there is no implementation of UpperHex for [u8]
                     embedded_services::debug!("OOB message: {:02X}", &src_slice[0..]);
 
@@ -179,7 +172,7 @@ impl<'hw, RelayHandler: embedded_services::relay::mctp::RelayHandler> ServiceInn
                         assembly_buf.as_mut_slice(),
                     );
 
-                    match mctp_ctx.deserialize_packet(with_pec) {
+                    match mctp_ctx.deserialize_packet(src_slice) {
                         Ok(Some(message)) => {
                             trace!("MCTP packet successfully deserialized");
                             match message.parse_as::<RelayHandler::RequestEnumType>() {
@@ -302,8 +295,6 @@ impl<'hw, RelayHandler: embedded_services::relay::mctp::RelayHandler> ServiceInn
                 error!("serialize_packet_from_subsystem: {:?}", e);
                 Error::Serialize
             })?;
-            // Last byte is PEC, ignore for now
-            let packet = &packet[..packet.len() - 1];
             trace!("Sending MCTP response: {:?}", packet);
 
             self.write_to_hw(espi, packet).map_err(|e| {

mctp-rs/src/lib.rs

@@ -17,7 +17,8 @@
 //! # impl MctpMedium for MyMedium { type Frame=MyMediumFrame; type Error=&'static str; type ReplyContext=(); type Encoding=PassthroughEncoding;
 //! #   fn max_message_body_size(&self)->usize{self.mtu}
 //! #   fn deserialize<'b>(&self,p:&'b [u8])->MctpPacketResult<(Self::Frame,EncodingDecoder<'b,Self::Encoding>),Self>{Ok((MyMediumFrame{packet_size:p.len()},EncodingDecoder::new(p)))}
-//! #   fn serialize<'b,F>(&self,_:Self::ReplyContext,b:&'b mut [u8],w:F)->MctpPacketResult<&'b [u8],Self> where F: for<'a> FnOnce(&mut EncodingEncoder<'a,Self::Encoding>)->MctpPacketResult<(),Self>{let n={let mut e=EncodingEncoder::<Self::Encoding>::new(b);w(&mut e)?;e.wire_position()};Ok(&b[..n])}}
+//! #   fn serialize<'b,F>(&self,_:Self::ReplyContext,b:&'b mut [u8],w:F)->MctpPacketResult<&'b [u8],Self> where F: for<'a> FnOnce(&mut EncodingEncoder<'a,Self::Encoding>)->MctpPacketResult<(),Self>{let n={let mut e=EncodingEncoder::<Self::Encoding>::new(b);w(&mut e)?;e.wire_position()};Ok(&b[..n])}
+//! #   fn frame_complete(&self,b:&[u8])->MctpPacketResult<Option<usize>,Self>{if b.is_empty(){Ok(None)}else{Ok(Some(b.len()))}}}
 //! # impl MctpMediumFrame<MyMedium> for MyMediumFrame { fn packet_size(&self)->usize{self.packet_size} fn reply_context(&self)->(){()}}
 //! let mut assembly_buffer = [0u8; 1024];
 //! let medium = MyMedium { mtu: 256 };
@@ -61,7 +62,8 @@
 //! # #[derive(Debug, Clone, Copy)] struct MyMediumFrame { packet_size: usize }
 //! # impl MctpMedium for MyMedium { type Frame=MyMediumFrame; type Error=&'static str; type ReplyContext=(); type Encoding=PassthroughEncoding; fn max_message_body_size(&self)->usize{self.mtu}
 //! #   fn deserialize<'b>(&self,p:&'b [u8])->MctpPacketResult<(Self::Frame,EncodingDecoder<'b,Self::Encoding>),Self>{Ok((MyMediumFrame{packet_size:p.len()},EncodingDecoder::new(p)))}
-//! #   fn serialize<'b,F>(&self,_:Self::ReplyContext,b:&'b mut [u8],w:F)->MctpPacketResult<&'b [u8],Self> where F: for<'a> FnOnce(&mut EncodingEncoder<'a,Self::Encoding>)->MctpPacketResult<(),Self>{let n={let mut e=EncodingEncoder::<Self::Encoding>::new(b);w(&mut e)?;e.wire_position()};Ok(&b[..n])}}
+//! #   fn serialize<'b,F>(&self,_:Self::ReplyContext,b:&'b mut [u8],w:F)->MctpPacketResult<&'b [u8],Self> where F: for<'a> FnOnce(&mut EncodingEncoder<'a,Self::Encoding>)->MctpPacketResult<(),Self>{let n={let mut e=EncodingEncoder::<Self::Encoding>::new(b);w(&mut e)?;e.wire_position()};Ok(&b[..n])}
+//! #   fn frame_complete(&self,b:&[u8])->MctpPacketResult<Option<usize>,Self>{if b.is_empty(){Ok(None)}else{Ok(Some(b.len()))}}}
 //! # impl MctpMediumFrame<MyMedium> for MyMediumFrame { fn packet_size(&self)->usize{self.packet_size} fn reply_context(&self)->(){()}}
 //! let mut buf = [0u8; 1024];
 //! let mut ctx = MctpPacketContext::new(MyMedium { mtu: 64 }, &mut buf);
@@ -147,6 +149,11 @@
 //!         };
 //!         Ok(&buffer[..written])
 //!     }
+//!
+//!     fn frame_complete(&self, buf: &[u8]) -> MctpPacketResult<Option<usize>, Self> {
+//!         // Trivial: treat any non-empty buffer as a complete frame.
+//!         if buf.is_empty() { Ok(None) } else { Ok(Some(buf.len())) }
+//!     }
 //! }
 //!
 //! impl MctpMediumFrame<MyMedium> for MyMediumFrame {
@@ -204,6 +211,22 @@ pub struct MctpMessageBuffer<'buffer> {
     rest: &'buffer [u8],
 }
 
+impl<'buffer> MctpMessageBuffer<'buffer> {
+    /// The raw message body bytes (after the message-type byte).
+    ///
+    /// Use when the caller wants to parse the body without going through
+    /// `MctpMessage::parse_as` (e.g., when the body is a small fixed-layout
+    /// payload and a typed `MctpMessageTrait` impl would be overkill).
+    pub fn body(&self) -> &'buffer [u8] {
+        self.rest
+    }
+
+    /// The MCTP message-type byte (e.g., `0x7E` for VendorDefinedPci).
+    pub fn message_type(&self) -> u8 {
+        self.message_type
+    }
+}
+
 impl<'buffer, M: MctpMedium> MctpMessage<'buffer, M> {
     pub fn can_parse_as<P: MctpMessageTrait<'buffer>>(&self) -> bool {
         self.message_buffer.message_type == P::MESSAGE_TYPE

mctp-rs/src/mctp_packet_context.rs

@@ -12,7 +12,7 @@ use crate::{
 /// Represents the state needed to construct a repsonse to a request:
 /// the MCTP transport source/destination, the sequence number to use for
 /// the reply, and the medium-specific context that came with the request.
-#[derive(Debug, PartialEq, Eq)]
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
 #[cfg_attr(feature = "defmt", derive(defmt::Format))]
 pub struct MctpReplyContext<M: MctpMedium> {
     pub destination_endpoint_id: EndpointId,

mctp-rs/src/medium/mod.rs

@@ -20,8 +20,8 @@ pub trait MctpMedium: Sized {
     type ReplyContext: core::fmt::Debug + Copy + Clone + PartialEq + Eq;
 
     /// the byte-stuffing transform used by this medium when (de)serializing
-    /// wire bytes. Stateless — see [`crate::buffer_encoding`]. Most media
-    /// use [`PassthroughEncoding`](crate::buffer_encoding::PassthroughEncoding)
+    /// wire bytes. Stateless — see [`BufferEncoding`](crate::BufferEncoding).
+    /// Most media use [`PassthroughEncoding`](crate::PassthroughEncoding)
     /// (no transform); media that need byte-stuffing (e.g., DSP0253 serial)
     /// supply their own impl.
     type Encoding: BufferEncoding;
@@ -55,6 +55,17 @@ pub trait MctpMedium: Sized {
     ) -> MctpPacketResult<&'buf [u8], Self>
     where
         F: for<'a> FnOnce(&mut EncodingEncoder<'a, Self::Encoding>) -> MctpPacketResult<(), Self>;
+
+    /// Returns `Ok(Some(len))` when `buf` contains a complete medium-framed
+    /// packet starting at `buf[0]`, where `len` is the total byte count of
+    /// that packet. Returns `Ok(None)` when `buf` has a partial frame
+    /// (caller should read more bytes and retry). Returns `Err(...)` when
+    /// `buf` is malformed.
+    ///
+    /// Used by generic consumers (e.g., `uart-service::Service<R, M>`)
+    /// to assemble packets from byte streams without knowing medium
+    /// framing details.
+    fn frame_complete(&self, buf: &[u8]) -> MctpPacketResult<Option<usize>, Self>;
 }
 
 pub trait MctpMediumFrame<M: MctpMedium>: Clone + Copy {

mctp-rs/src/medium/serial.rs

@@ -293,6 +293,27 @@ impl MctpMedium for MctpSerialMedium {
 
         Ok(&buffer[..end_pos + 1])
     }
+
+    fn frame_complete(&self, buf: &[u8]) -> MctpPacketResult<Option<usize>, Self> {
+        // This medium's `serialize` emits `[revision, byte_count, ...stuffed body,
+        // fcs_msb, fcs_lsb, 0x7E]` — only an END flag, no leading START flag.
+        //
+        // The `body_start` check below is forward-defensive: DSP0253 variants are
+        // permitted to emit a leading 0x7E flag, and this lets `frame_complete`
+        // accept that case too (skip the leading flag, then find the next 0x7E).
+        // Either pattern returns the full frame length to the caller.
+        if buf.is_empty() {
+            return Ok(None);
+        }
+        let body_start = if buf[0] == END_FLAG { 1 } else { 0 };
+        if body_start >= buf.len() {
+            return Ok(None);
+        }
+        match buf[body_start..].iter().position(|&b| b == END_FLAG) {
+            Some(idx) => Ok(Some(body_start + idx + 1)),
+            None => Ok(None),
+        }
+    }
 }
 
 #[cfg(test)]
@@ -713,4 +734,60 @@ mod medium_tests {
         assert!(packet_count >= 2, "expected multi-packet split, got {packet_count}");
         assert_eq!(total_decoded_body, payload.len());
     }
+
+    // ----- frame_complete tests -----
+
+    #[test]
+    fn frame_complete_empty_buf_returns_none() {
+        assert_eq!(MctpSerialMedium.frame_complete(&[]).unwrap(), None);
+    }
+
+    #[test]
+    fn frame_complete_no_end_flag_returns_none() {
+        // Bytes present but no 0x7E anywhere → partial frame
+        let buf = [0x01, 0x05, 0xAA, 0xBB, 0xCC];
+        assert_eq!(MctpSerialMedium.frame_complete(&buf).unwrap(), None);
+    }
+
+    #[test]
+    fn frame_complete_only_end_flag_returns_none() {
+        // A lone 0x7E is treated as a leading flag (skipped); after
+        // skipping there are no bytes left, so partial.
+        let buf = [END_FLAG];
+        assert_eq!(MctpSerialMedium.frame_complete(&buf).unwrap(), None);
+    }
+
+    #[test]
+    fn frame_complete_no_leading_flag_finds_end_flag() {
+        // [ revision, byte_count, ...body, fcs_msb, fcs_lsb, 0x7E ]
+        // matches what MctpSerialMedium::serialize emits — no leading flag.
+        let buf = [SERIAL_REVISION, 0x03, 0xAA, 0xBB, 0xCC, 0xDD, 0xEE, END_FLAG];
+        assert_eq!(MctpSerialMedium.frame_complete(&buf).unwrap(), Some(8));
+    }
+
+    #[test]
+    fn frame_complete_with_leading_flag_skips_and_finds_end_flag() {
+        // Forward-defensive: some DSP0253 variants emit a leading START
+        // flag. frame_complete should skip the leading 0x7E and find the
+        // trailing one, returning the FULL length (including leading flag).
+        let buf = [END_FLAG, SERIAL_REVISION, 0x02, 0xAA, 0xBB, 0xCC, 0xDD, END_FLAG];
+        assert_eq!(MctpSerialMedium.frame_complete(&buf).unwrap(), Some(8));
+    }
+
+    #[test]
+    fn frame_complete_extra_bytes_after_end_flag_returns_first_frame_len() {
+        // First frame is 6 bytes (incl. END flag); 2 trailing bytes belong
+        // to the next frame (caller's problem).
+        let buf = [SERIAL_REVISION, 0x01, 0xAA, 0xBB, 0xCC, END_FLAG, 0x99, 0x88];
+        assert_eq!(MctpSerialMedium.frame_complete(&buf).unwrap(), Some(6));
+    }
+
+    #[test]
+    fn frame_complete_back_to_back_after_leading_flag() {
+        // Edge case: leading flag, then payload, then end flag, then more
+        // bytes. Should return length of first frame INCLUDING the leading
+        // flag; trailing bytes belong to a subsequent frame.
+        let buf = [END_FLAG, SERIAL_REVISION, 0x01, 0xAA, 0xBB, END_FLAG, 0x77];
+        assert_eq!(MctpSerialMedium.frame_complete(&buf).unwrap(), Some(6));
+    }
 }

mctp-rs/src/medium/smbus_espi.rs

@@ -106,6 +106,18 @@ impl MctpMedium for SmbusEspiMedium {
     fn max_message_body_size(&self) -> usize {
         32
     }
+
+    fn frame_complete(&self, buf: &[u8]) -> MctpPacketResult<Option<usize>, Self> {
+        // SmbusEspi framing: [dst_addr | src_addr | byte_count | cmd_code]
+        //                    [ body bytes (byte_count) ] [ PEC byte ]
+        // Total: 4 + byte_count + 1 = 5 + byte_count
+        if buf.len() < 4 {
+            return Ok(None);
+        }
+        let byte_count = buf[2] as usize;
+        let total = 4 + byte_count + 1;
+        if buf.len() < total { Ok(None) } else { Ok(Some(total)) }
+    }
 }
 
 #[repr(u8)]
@@ -845,4 +857,63 @@ mod tests {
             MctpPacketError::SerializeError("encode error")
         );
     }
+
+    // ----- frame_complete tests -----
+
+    #[test]
+    fn frame_complete_empty_buf_returns_none() {
+        assert_eq!(SmbusEspiMedium.frame_complete(&[]).unwrap(), None);
+    }
+
+    #[test]
+    fn frame_complete_partial_header_returns_none() {
+        // SmbusEspi header is 4 bytes (dst, src, byte_count, cmd_code).
+        // Any byte count from 1 to 3 means we don't know byte_count yet.
+        for n in 1..4 {
+            let buf: Vec<u8> = (0..n).map(|_| 0u8).collect();
+            assert_eq!(
+                SmbusEspiMedium.frame_complete(&buf).unwrap(),
+                None,
+                "partial header ({} bytes) should be incomplete",
+                n
+            );
+        }
+    }
+
+    #[test]
+    fn frame_complete_exact_frame_returns_total_len() {
+        // header (4) + body (byte_count = 3) + PEC (1) = 8 bytes
+        let buf: [u8; 8] = [0x20, 0x10, 0x03, 0x0F, 0xAA, 0xBB, 0xCC, 0xDD];
+        assert_eq!(SmbusEspiMedium.frame_complete(&buf).unwrap(), Some(8));
+    }
+
+    #[test]
+    fn frame_complete_short_of_body_returns_none() {
+        // byte_count says 5, but we only have 4 header + 3 body bytes (no PEC yet)
+        let buf: [u8; 7] = [0x20, 0x10, 0x05, 0x0F, 0xAA, 0xBB, 0xCC];
+        assert_eq!(SmbusEspiMedium.frame_complete(&buf).unwrap(), None);
+    }
+
+    #[test]
+    fn frame_complete_short_of_pec_returns_none() {
+        // byte_count = 2 → expects 4 + 2 + 1 = 7 bytes; we have 6
+        let buf: [u8; 6] = [0x20, 0x10, 0x02, 0x0F, 0xAA, 0xBB];
+        assert_eq!(SmbusEspiMedium.frame_complete(&buf).unwrap(), None);
+    }
+
+    #[test]
+    fn frame_complete_extra_bytes_after_frame_returns_first_frame_len() {
+        // First frame: 4 + 1 + 1 = 6 bytes. Buffer has 8 bytes (2 trailing).
+        // frame_complete reports the length of the FIRST frame; trailing
+        // bytes are the caller's problem (e.g., next iteration of the loop).
+        let buf: [u8; 8] = [0x20, 0x10, 0x01, 0x0F, 0xAA, 0xBB, 0xCC, 0xDD];
+        assert_eq!(SmbusEspiMedium.frame_complete(&buf).unwrap(), Some(6));
+    }
+
+    #[test]
+    fn frame_complete_zero_byte_count_returns_5_bytes() {
+        // Edge case: byte_count = 0 means 4 header + 0 body + 1 PEC = 5 bytes
+        let buf: [u8; 5] = [0x20, 0x10, 0x00, 0x0F, 0xCC];
+        assert_eq!(SmbusEspiMedium.frame_complete(&buf).unwrap(), Some(5));
+    }
 }

mctp-rs/src/test_util.rs

@@ -62,6 +62,15 @@ impl MctpMedium for TestMedium {
     fn max_message_body_size(&self) -> usize {
         self.mtu
     }
+    fn frame_complete(&self, buf: &[u8]) -> MctpPacketResult<Option<usize>, Self> {
+        // TestMedium frames are length-bounded by the buffer passed in;
+        // for tests we treat any non-empty buffer as a complete frame.
+        if buf.len() < self.header.len() + self.trailer.len() {
+            Ok(None)
+        } else {
+            Ok(Some(buf.len()))
+        }
+    }
     fn serialize<'buf, F>(
         &self,
         _: Self::ReplyContext,