Flatten suppressable request structs into public data bags

.gitignore

@@ -326,3 +326,7 @@ Testing/
 # Fuzz corpus
 fuzz/corpus/
 fuzz/artifacts/
+
+.omniscient/
+
+.DS_Store

src/lib.rs

@@ -47,8 +47,8 @@ pub use services::{
     RequestDownloadRequest, RequestDownloadResponse, RequestFileTransferRequest,
     RequestFileTransferResponse, RequestTransferExitRequest, RequestTransferExitResponse,
     ResetType, RoutineControlRequest, RoutineControlResponse, RoutineControlSubFunction,
-    SecurityAccessRequest, SecurityAccessResponse, SecurityAccessType, SentDataPayload,
-    SizePayload, TesterPresentRequest, TesterPresentResponse, TransferDataRequest,
+    SecurityAccessLevel, SecurityAccessRequest, SecurityAccessResponse, SecurityAccessType,
+    SentDataPayload, SizePayload, TesterPresentRequest, TesterPresentResponse, TransferDataRequest,
     TransferDataResponse, WriteDataByIdentifierRequest, WriteDataByIdentifierResponse,
 };
 
@@ -206,7 +206,13 @@ mod no_std_api_tests {
     fn const_construction() {
         // Verify const construction works at compile time
         const _REQ: TransferDataRequest<'static> = TransferDataRequest::new(1, &[0x01, 0x02, 0x03]);
-        const _SEC: SecurityAccessRequest<'static> =
-            SecurityAccessRequest::new(false, SecurityAccessType::RequestSeed(0x01), &[0xAA, 0xBB]);
+        const _SEC: SecurityAccessRequest<'static> = SecurityAccessRequest::new(
+            false,
+            SecurityAccessType::RequestSeed(match SecurityAccessLevel::new(0x01) {
+                Ok(level) => level,
+                Err(_) => panic!("0x01 is a valid security access level"),
+            }),
+            &[0xAA, 0xBB],
+        );
     }
 }

src/request.rs

@@ -184,12 +184,12 @@ impl Request<'_> {
     pub fn is_positive_response_suppressed(&self) -> bool {
         match self {
             Self::CommunicationControl(req) => req.suppress_positive_response(),
-            Self::ControlDTCSettings(req) => req.suppress_positive_response(),
-            Self::DiagnosticSessionControl(req) => req.suppress_positive_response(),
-            Self::EcuReset(req) => req.suppress_positive_response(),
-            Self::RoutineControl(req) => req.suppress_positive_response(),
-            Self::SecurityAccess(req) => req.suppress_positive_response(),
-            Self::TesterPresent(req) => req.suppress_positive_response(),
+            Self::ControlDTCSettings(req) => req.suppress_positive_response,
+            Self::DiagnosticSessionControl(req) => req.suppress_positive_response,
+            Self::EcuReset(req) => req.suppress_positive_response,
+            Self::RoutineControl(req) => req.suppress_positive_response,
+            Self::SecurityAccess(req) => req.suppress_positive_response,
+            Self::TesterPresent(req) => req.suppress_positive_response,
             _ => false,
         }
     }

src/services/communication_control.rs

@@ -34,14 +34,26 @@ pub enum CommunicationControlType {
     /// shall be enabled for the specified [`CommunicationType`]
     /// Additionally, enhanced address information shall be included in the request
     EnableRxAndTxWithEnhancedAddressInfo,
-    /// These values are reserved by the ISO 14229-1 Specification
+    /// These values are reserved by the ISO 14229-1 Specification.
+    ///
+    /// Construct through [`CommunicationControlType::try_from`] so the raw byte is
+    /// range-checked and can never collide with the SPRMIB bit.
     #[cfg_attr(feature = "clap", clap(skip))]
+    #[non_exhaustive]
     ISOSAEReserved(u8),
-    /// Values reserved for use by vehicle manufacturers
+    /// Values reserved for use by vehicle manufacturers.
+    ///
+    /// Construct through [`CommunicationControlType::try_from`] so the raw byte is
+    /// range-checked and can never collide with the SPRMIB bit.
     #[cfg_attr(feature = "clap", clap(skip))]
+    #[non_exhaustive]
     VehicleManufacturerSpecific(u8),
-    /// Values reserved for use by system suppliers
+    /// Values reserved for use by system suppliers.
+    ///
+    /// Construct through [`CommunicationControlType::try_from`] so the raw byte is
+    /// range-checked and can never collide with the SPRMIB bit.
     #[cfg_attr(feature = "clap", clap(skip))]
+    #[non_exhaustive]
     SystemSupplierSpecific(u8),
 }
 

src/services/control_dtc_settings.rs

@@ -43,29 +43,21 @@ impl TryFrom<u8> for DtcSettings {
 #[derive(Clone, Copy, Debug, Eq, PartialEq)]
 #[non_exhaustive]
 pub struct ControlDTCSettingsRequest {
-    setting: SuppressablePositiveResponse<DtcSettings>,
+    /// Whether the server should suppress the positive response (SPRMIB).
+    pub suppress_positive_response: bool,
+    /// The requested DTC logging setting.
+    pub setting: DtcSettings,
 }
 
 impl ControlDTCSettingsRequest {
     /// Create a new `ControlDTCSettingsRequest`.
     #[must_use]
     pub const fn new(suppress_positive_response: bool, setting: DtcSettings) -> Self {
         Self {
-            setting: SuppressablePositiveResponse::new(suppress_positive_response, setting),
+            suppress_positive_response,
+            setting,
         }
     }
-
-    /// Returns the requested DTC logging setting.
-    #[must_use]
-    pub fn setting(&self) -> DtcSettings {
-        self.setting.value()
-    }
-
-    /// Whether the server should suppress the positive response (SPRMIB).
-    #[must_use]
-    pub fn suppress_positive_response(&self) -> bool {
-        self.setting.suppress_positive_response()
-    }
 }
 
 impl Encode for ControlDTCSettingsRequest {
@@ -74,8 +66,10 @@ impl Encode for ControlDTCSettingsRequest {
     }
 
     fn encode(&self, writer: &mut impl embedded_io::Write) -> Result<usize, Error> {
+        let sub_function =
+            SuppressablePositiveResponse::new(self.suppress_positive_response, self.setting);
         writer
-            .write_all(&[u8::from(self.setting)])
+            .write_all(&[u8::from(sub_function)])
             .map_err(Error::io)?;
         Ok(1)
     }
@@ -86,8 +80,14 @@ impl<'a> Decode<'a> for ControlDTCSettingsRequest {
         if buf.is_empty() {
             return Err(Error::InsufficientData(1));
         }
-        let setting = SuppressablePositiveResponse::try_from(buf[0])?;
-        Ok((Self { setting }, &buf[1..]))
+        let sub_function = SuppressablePositiveResponse::<DtcSettings>::try_from(buf[0])?;
+        Ok((
+            Self {
+                suppress_positive_response: sub_function.suppress_positive_response(),
+                setting: sub_function.value(),
+            },
+            &buf[1..],
+        ))
     }
 }
 
@@ -152,8 +152,8 @@ mod request {
         assert_eq!(req.encoded_size(), buffer.len());
 
         let (parsed, _) = <ControlDTCSettingsRequest as Decode>::decode(&buffer).unwrap();
-        assert_eq!(parsed.setting(), DtcSettings::On);
-        assert!(parsed.suppress_positive_response());
+        assert_eq!(parsed.setting, DtcSettings::On);
+        assert!(parsed.suppress_positive_response);
         assert_encode_size_agrees(&req);
     }
 

src/services/diagnostic_session_control.rs

@@ -25,7 +25,10 @@ use crate::{Decode, Encode, Error, NegativeResponseCode};
 #[non_exhaustive]
 pub enum DiagnosticSessionType {
     /// This value is reserved by the ISO 14229-1 Specification
+    ///
+    /// Construct through [`DiagnosticSessionType::try_from`] so the raw byte is range-checked and can never collide with the SPRMIB bit.
     #[cfg_attr(feature = "clap", clap(skip))]
+    #[non_exhaustive]
     ISOSAEReserved(u8),
     /// The `DefaultSession` (0x01) enables the standard diagnostic functionality
     /// - No `TesterPresent` messages are required to remain in this session
@@ -42,10 +45,16 @@ pub enum DiagnosticSessionType {
     /// The `SafetySystemDiagnosticSession` (0x04) enables diagnostics functionality for safety systems
     SafetySystemDiagnosticSession,
     /// Value reserved for use by vehicle manufacturers
+    ///
+    /// Construct through [`DiagnosticSessionType::try_from`] so the raw byte is range-checked and can never collide with the SPRMIB bit.
     #[cfg_attr(feature = "clap", clap(skip))]
+    #[non_exhaustive]
     VehicleManufacturerSpecificSession(u8),
     /// Value reserved for use by system suppliers
+    ///
+    /// Construct through [`DiagnosticSessionType::try_from`] so the raw byte is range-checked and can never collide with the SPRMIB bit.
     #[cfg_attr(feature = "clap", clap(skip))]
+    #[non_exhaustive]
     SystemSupplierSpecificSession(u8),
 }
 
@@ -163,7 +172,10 @@ const DIAGNOSTIC_SESSION_CONTROL_NEGATIVE_RESPONSE_CODES: [NegativeResponseCode;
 #[derive(Clone, Copy, Debug, Eq, PartialEq)]
 #[non_exhaustive]
 pub struct DiagnosticSessionControlRequest {
-    session_type: SuppressablePositiveResponse<DiagnosticSessionType>,
+    /// Whether a positive response should be suppressed.
+    pub suppress_positive_response: bool,
+    /// The requested diagnostic session type.
+    pub session_type: DiagnosticSessionType,
 }
 
 impl DiagnosticSessionControlRequest {
@@ -174,25 +186,11 @@ impl DiagnosticSessionControlRequest {
         session_type: DiagnosticSessionType,
     ) -> Self {
         Self {
-            session_type: SuppressablePositiveResponse::new(
-                suppress_positive_response,
-                session_type,
-            ),
+            suppress_positive_response,
+            session_type,
         }
     }
 
-    /// Getter for whether a positive response should be suppressed
-    #[must_use]
-    pub fn suppress_positive_response(&self) -> bool {
-        self.session_type.suppress_positive_response()
-    }
-
-    /// Getter for the requested session type
-    #[must_use]
-    pub fn session_type(&self) -> DiagnosticSessionType {
-        self.session_type.value()
-    }
-
     /// Get the allowed [`NegativeResponseCode`] variants for this request
     #[must_use]
     pub fn allowed_nack_codes() -> &'static [NegativeResponseCode] {
@@ -205,8 +203,10 @@ impl Encode for DiagnosticSessionControlRequest {
     }
 
     fn encode(&self, writer: &mut impl embedded_io::Write) -> Result<usize, Error> {
+        let sub_function =
+            SuppressablePositiveResponse::new(self.suppress_positive_response, self.session_type);
         writer
-            .write_all(&[u8::from(self.session_type)])
+            .write_all(&[u8::from(sub_function)])
             .map_err(Error::io)?;
         Ok(1)
     }
@@ -217,8 +217,14 @@ impl<'a> Decode<'a> for DiagnosticSessionControlRequest {
         if buf.is_empty() {
             return Err(Error::InsufficientData(1));
         }
-        let session_type = SuppressablePositiveResponse::try_from(buf[0])?;
-        Ok((Self { session_type }, &buf[1..]))
+        let sub_function = SuppressablePositiveResponse::<DiagnosticSessionType>::try_from(buf[0])?;
+        Ok((
+            Self {
+                suppress_positive_response: sub_function.suppress_positive_response(),
+                session_type: sub_function.value(),
+            },
+            &buf[1..],
+        ))
     }
 }
 
@@ -301,11 +307,8 @@ mod request {
     fn test_diagnostic_session_control_request() {
         let bytes: [u8; 1] = [0x02];
         let (req, _) = <DiagnosticSessionControlRequest as Decode>::decode(&bytes).unwrap();
-        assert!(!req.suppress_positive_response());
-        assert_eq!(
-            req.session_type(),
-            DiagnosticSessionType::ProgrammingSession
-        );
+        assert!(!req.suppress_positive_response);
+        assert_eq!(req.session_type, DiagnosticSessionType::ProgrammingSession);
 
         let mut buffer = Vec::new();
         Encode::encode(&req, &mut buffer).unwrap();

src/services/ecu_reset.rs

@@ -15,8 +15,12 @@ use crate::{Decode, Encode, Error, NegativeResponseCode};
 #[derive(Clone, Copy, Debug, Eq, PartialEq)]
 #[non_exhaustive]
 pub enum ResetType {
-    /// This value is reserved
+    /// This value is reserved.
+    ///
+    /// Construct reserved values through [`ResetType::try_from`] so the raw byte is
+    /// range-checked (`0x00..=0x7F`) and can never collide with the SPRMIB bit.
     #[cfg_attr(feature = "clap", clap(skip))]
+    #[non_exhaustive]
     ISOSAEReserved(u8),
     /// This `SubFunction` identifies a "hard reset" condition which simulates the power-on/start-up sequence
     /// typically performed after a server has been previously disconnected from its power supply (i.e. battery).
@@ -49,11 +53,19 @@ pub enum ResetType {
     EnableRapidPowerShutDown,
     /// This `SubFunction` requests the server to disable the previously enabled "rapid power shut down" function.
     DisableRapidPowerShutDown,
-    /// Reserved for use by vehicle manufacturers
+    /// Reserved for use by vehicle manufacturers.
+    ///
+    /// Construct through [`ResetType::try_from`] so the raw byte is range-checked
+    /// (`0x40..=0x5F`) and can never collide with the SPRMIB bit.
     #[cfg_attr(feature = "clap", clap(skip))]
+    #[non_exhaustive]
     VehicleManufacturerSpecific(u8),
-    /// Reserved for use by system suppliers
+    /// Reserved for use by system suppliers.
+    ///
+    /// Construct through [`ResetType::try_from`] so the raw byte is range-checked
+    /// (`0x60..=0x7E`) and can never collide with the SPRMIB bit.
     #[cfg_attr(feature = "clap", clap(skip))]
+    #[non_exhaustive]
     SystemSupplierSpecific(u8),
 }
 
@@ -175,30 +187,22 @@ const ECU_RESET_NEGATIVE_RESPONSE_CODES: [NegativeResponseCode; 4] = [
 #[derive(Clone, Copy, Debug, Eq, PartialEq)]
 #[non_exhaustive]
 pub struct EcuResetRequest {
-    reset_type: SuppressablePositiveResponse<ResetType>,
+    /// Whether the server should suppress a positive response (SPRMIB).
+    pub suppress_positive_response: bool,
+    /// The type of reset being requested.
+    pub reset_type: ResetType,
 }
 
 impl EcuResetRequest {
     /// Create a new '`EcuResetRequest`'
     #[must_use]
     pub const fn new(suppress_positive_response: bool, reset_type: ResetType) -> Self {
         Self {
-            reset_type: SuppressablePositiveResponse::new(suppress_positive_response, reset_type),
+            suppress_positive_response,
+            reset_type,
         }
     }
 
-    /// Getter for whether a positive response should be suppressed
-    #[must_use]
-    pub fn suppress_positive_response(&self) -> bool {
-        self.reset_type.suppress_positive_response()
-    }
-
-    /// Getter for the requested [`ResetType`]
-    #[must_use]
-    pub fn reset_type(&self) -> ResetType {
-        self.reset_type.value()
-    }
-
     /// Get the allowed [`NegativeResponseCode`] variants for this request
     #[must_use]
     pub fn allowed_nack_codes() -> &'static [NegativeResponseCode] {
@@ -212,8 +216,11 @@ impl Encode for EcuResetRequest {
     }
 
     fn encode(&self, writer: &mut impl embedded_io::Write) -> Result<usize, Error> {
+        // Fuse the SPRMIB bit into the sub-function byte only at the wire boundary.
+        let sub_function =
+            SuppressablePositiveResponse::new(self.suppress_positive_response, self.reset_type);
         writer
-            .write_all(&[u8::from(self.reset_type)])
+            .write_all(&[u8::from(sub_function)])
             .map_err(Error::io)?;
         Ok(1)
     }
@@ -224,8 +231,14 @@ impl<'a> Decode<'a> for EcuResetRequest {
         if buf.is_empty() {
             return Err(Error::InsufficientData(1));
         }
-        let reset_type = SuppressablePositiveResponse::try_from(buf[0])?;
-        Ok((Self { reset_type }, &buf[1..]))
+        let sub_function = SuppressablePositiveResponse::<ResetType>::try_from(buf[0])?;
+        Ok((
+            Self {
+                suppress_positive_response: sub_function.suppress_positive_response(),
+                reset_type: sub_function.value(),
+            },
+            &buf[1..],
+        ))
     }
 }
 

src/services/mod.rs

@@ -47,7 +47,9 @@ pub use routine_control::{
 };
 
 mod security_access;
-pub use security_access::{SecurityAccessRequest, SecurityAccessResponse, SecurityAccessType};
+pub use security_access::{
+    SecurityAccessLevel, SecurityAccessRequest, SecurityAccessResponse, SecurityAccessType,
+};
 
 mod tester_present;
 pub use tester_present::{TesterPresentRequest, TesterPresentResponse};