Various fixes: NRC/DID block additions

src/common/diagnostic_identifier.rs

@@ -71,6 +71,14 @@ impl TryFrom<u16> for UDSIdentifier {
     fn try_from(value: u16) -> Result<Self, Self::Error> {
         Ok(match value {
             0x0000..=0x00FF => Self::ISOSAEReserved(value),
+            0x0100..=0xA5FF => Self::VehicleManufacturerSpecific(value),
+            //0xA600..=0xA7FF => Self::ISOSAEReserved(value),
+            0xA800..=0xACFF => Self::VehicleManufacturerSpecific(value),
+            0xB000..=0xB1FF => Self::VehicleManufacturerSpecific(value),
+            0xC000..=0xC2FF => Self::VehicleManufacturerSpecific(value),
+            0xCF00..=0xEFFF => Self::VehicleManufacturerSpecific(value),
+            // 0xF000..=0xF00F => Self::TractorTrailer(value),
+            0xF010..=0xF0FF => Self::VehicleManufacturerSpecific(value),
             0xF100..=0xF17F => Self::VehicleManufacturerSpecific(value),
             // 0x0100..0xA5FF => Manufacturer Specific,
             0xF180 => Self::BootSoftwareIdentification,
@@ -105,6 +113,16 @@ impl TryFrom<u16> for UDSIdentifier {
             0xF19D => Self::ECUInstallationDate,
             0xF19E => Self::ODXFile,
             0xF19F => Self::Entity,
+            0xF1A0..=0xF1EF => Self::VehicleManufacturerSpecific(value),
+            0xF1F0..=0xF1FF => Self::SystemSupplierSpecific(value),
+            // 0xF200..=0xFDFF => Self::PeriodicDataIdentifier(value),
+            // 0xF300..=0xF3FF => Self::DynamicallyDefined(value),
+            // 0xF400..=0xF5FF => Self::OBD(value),
+            // 0xF600..=0xF6FF => Self::OBDMonitor(value),
+            // 0xF700..=0xF7FF => Self::OBD(value),
+            // 0xF800..=0xF8FF => Self::OBDInfoType(value),
+            // 0xF900..=0xF9FF => Self::Tachograph(value),
+            // 0xFA00..=0xFA0F => Self::AirbagDeployment(value),
             0xFD00..=0xFEFF => Self::SystemSupplierSpecific(value),
             0xFF02..=0xFFFF => Self::ISOSAEReserved(value),
 

src/error.rs

@@ -6,9 +6,9 @@ pub enum Error {
     IoError(#[from] std::io::Error),
     #[error("Insufficient data. Expected {0} bytes.")]
     InsufficientData(usize),
-    #[error("Invalid Diagnostic Identifier: {0:X}")]
+    #[error("Invalid Diagnostic Identifier: {0:#X}")]
     InvalidDiagnosticIdentifier(u16),
-    #[error("Invalid Diagnostic Identifier: {0:X} with payload {1:?}")]
+    #[error("Invalid Diagnostic Identifier: {0:#X} with payload {1:?}")]
     InvalidDiagnosticIdentifierPayload(u16, Vec<u8>),
     #[error("Invalid diagnostic session type: {0}")]
     InvalidDiagnosticSessionType(u8),
@@ -40,4 +40,6 @@ pub enum Error {
     InvalidDtcFormatIdentifier(u8),
     #[error("Reserved for legislative use: {0} ({1})")]
     ReservedForLegislativeUse(String, u8),
+    #[error("Serialization error: {0}")]
+    SerializationError(String),
 }

src/lib.rs

@@ -133,13 +133,14 @@ impl From<DtcSettings> for u8 {
         }
     }
 }
+impl TryFrom<u8> for DtcSettings {
+    type Error = Error;
 
-impl From<u8> for DtcSettings {
-    fn from(value: u8) -> Self {
+    fn try_from(value: u8) -> Result<Self, Self::Error> {
         match value {
-            0x01 => Self::On,
-            0x02 => Self::Off,
-            _ => panic!("Invalid DTC setting: {value}"),
+            0x01 => Ok(Self::On),
+            0x02 => Ok(Self::Off),
+            _ => Err(Error::InvalidDtcSubfunctionType(value)),
         }
     }
 }

src/request.rs

@@ -65,10 +65,10 @@ impl<D: DiagnosticDefinition> Request<D> {
     pub fn communication_control(
         communication_enable: CommunicationControlType,
         communication_type: CommunicationType,
-        suppress_response: bool,
+        suppress_positive_response: bool,
     ) -> Self {
         Request::CommunicationControl(CommunicationControlRequest::new(
-            suppress_response,
+            suppress_positive_response,
             communication_enable,
             communication_type,
         ))
@@ -86,10 +86,10 @@ impl<D: DiagnosticDefinition> Request<D> {
         communication_enable: CommunicationControlType,
         communication_type: CommunicationType,
         node_id: u16,
-        suppress_response: bool,
+        suppress_positive_response: bool,
     ) -> Self {
         Request::CommunicationControl(CommunicationControlRequest::new_with_node_id(
-            suppress_response,
+            suppress_positive_response,
             communication_enable,
             communication_type,
             node_id,
@@ -98,8 +98,11 @@ impl<D: DiagnosticDefinition> Request<D> {
 
     /// Create a new `ControlDTCSettings` request
     #[must_use]
-    pub fn control_dtc_settings(setting: DtcSettings, suppress_response: bool) -> Self {
-        Request::ControlDTCSettings(ControlDTCSettingsRequest::new(setting, suppress_response))
+    pub fn control_dtc_settings(suppress_positive_response: bool, setting: DtcSettings) -> Self {
+        Request::ControlDTCSettings(ControlDTCSettingsRequest::new(
+            suppress_positive_response,
+            setting,
+        ))
     }
 
     /// Create a new `DiagnosticSessionControl` request
@@ -271,6 +274,7 @@ impl<T: DiagnosticDefinition> WireFormat for Request<T> {
     /// or the deserialization could read unexpected data
     fn decode<R: Read>(reader: &mut R) -> Result<Option<Self>, Error> {
         let service = UdsServiceType::service_from_request_byte(reader.read_u8()?);
+        tracing::debug!("Deserializing UDS Request for service: {:?}", service);
         Ok(Some(match service {
             UdsServiceType::CommunicationControl => Self::CommunicationControl(
                 CommunicationControlRequest::decode_single_value(reader)?,
@@ -404,7 +408,7 @@ mod tests {
         assert!(communication_control_request.is_positive_response_suppressed());
 
         let control_dtc_settings_request =
-            ProtocolRequest::control_dtc_settings(DtcSettings::On, true);
+            ProtocolRequest::control_dtc_settings(true, DtcSettings::On);
         assert!(control_dtc_settings_request.is_positive_response_suppressed());
 
         let diagnostic_session_control_request = ProtocolRequest::diagnostic_session_control(

src/response.rs

@@ -82,7 +82,7 @@ impl<D: DiagnosticDefinition> Response<D> {
     }
 
     #[must_use]
-    pub fn ecu_reset(reset_type: ResetType, power_down_time: u8) -> Self {
+    pub fn ecu_reset(reset_type: ResetType, power_down_time: Option<u8>) -> Self {
         Response::EcuReset(EcuResetResponse::new(reset_type, power_down_time))
     }
 

src/services/control_dtc_settings.rs

@@ -1,4 +1,4 @@
-use crate::{DtcSettings, Error, SUCCESS, SingleValueWireFormat, WireFormat};
+use crate::{DtcSettings, Error, SingleValueWireFormat, SuppressablePositiveResponse, WireFormat};
 use byteorder::{ReadBytesExt, WriteBytesExt};
 
 /// The `ControlDTCSettings` service is used to control the DTC settings of the ECU.
@@ -8,44 +8,45 @@ use byteorder::{ReadBytesExt, WriteBytesExt};
 #[non_exhaustive]
 pub struct ControlDTCSettingsRequest {
     /// The requested DTC logging setting
-    pub setting: DtcSettings,
-    /// Whether the ECU should suppress a response
-    pub suppress_response: bool,
+    setting: SuppressablePositiveResponse<DtcSettings>,
 }
 
 impl ControlDTCSettingsRequest {
-    pub(crate) fn new(setting: DtcSettings, suppress_response: bool) -> Self {
+    pub(crate) fn new(suppress_positive_response: bool, setting: DtcSettings) -> Self {
         Self {
-            setting,
-            suppress_response,
+            setting: SuppressablePositiveResponse::new(suppress_positive_response, setting),
         }
     }
+    /// Getter for whether a positive response should be suppressed
+    #[must_use]
+    pub fn suppress_positive_response(&self) -> bool {
+        self.setting.suppress_positive_response()
+    }
+
+    /// Getter for the setting
+    #[must_use]
+    pub fn setting(&self) -> DtcSettings {
+        self.setting.value()
+    }
 }
 
 impl WireFormat for ControlDTCSettingsRequest {
     fn decode<T: std::io::Read>(reader: &mut T) -> Result<Option<Self>, Error> {
-        let request_byte = reader.read_u8()?;
-        let setting = DtcSettings::from(request_byte & !SUCCESS);
-        let suppress_response = request_byte & SUCCESS != 0;
-        Ok(Some(Self {
-            setting,
-            suppress_response,
-        }))
+        let setting = SuppressablePositiveResponse::try_from(reader.read_u8()?)?;
+        Ok(Some(Self { setting }))
     }
 
     fn required_size(&self) -> usize {
         1
     }
 
     fn encode<T: std::io::Write>(&self, writer: &mut T) -> Result<usize, Error> {
-        let request_byte =
-            u8::from(self.setting) | if self.suppress_response { SUCCESS } else { 0 };
-        writer.write_u8(request_byte)?;
+        writer.write_u8(u8::from(self.setting))?;
         Ok(1)
     }
 
     fn is_positive_response_suppressed(&self) -> bool {
-        self.suppress_response
+        self.suppress_positive_response()
     }
 }
 
@@ -71,7 +72,7 @@ impl ControlDTCSettingsResponse {
 
 impl WireFormat for ControlDTCSettingsResponse {
     fn decode<T: std::io::Read>(reader: &mut T) -> Result<Option<Self>, Error> {
-        let setting = DtcSettings::from(reader.read_u8()?);
+        let setting = DtcSettings::try_from(reader.read_u8()?)?;
         Ok(Some(Self { setting }))
     }
 
@@ -94,7 +95,7 @@ mod request {
 
     #[test]
     fn simple_request() {
-        let req = ControlDTCSettingsRequest::new(DtcSettings::On, true);
+        let req = ControlDTCSettingsRequest::new(true, DtcSettings::On);
         let mut buffer = Vec::new();
         let written = req.encode(&mut buffer).unwrap();
         assert_eq!(buffer, vec![0x81]);
@@ -103,8 +104,8 @@ mod request {
 
         let parsed =
             ControlDTCSettingsRequest::decode_single_value(&mut buffer.as_slice()).unwrap();
-        assert_eq!(parsed.setting, DtcSettings::On);
-        assert!(parsed.suppress_response);
+        assert_eq!(parsed.setting(), DtcSettings::On);
+        assert!(parsed.suppress_positive_response());
     }
 }
 

src/services/ecu_reset.rs

@@ -77,12 +77,20 @@ impl SingleValueWireFormat for EcuResetRequest {}
 #[non_exhaustive]
 pub struct EcuResetResponse {
     pub reset_type: ResetType,
-    pub power_down_time: u8,
+    /// Optional power down time in seconds for [`ResetType::EnableRapidPowerShutDown`]
+    pub power_down_time: Option<u8>,
 }
 
 impl EcuResetResponse {
     /// Create a new '`EcuResetResponse`'
-    pub(crate) fn new(reset_type: ResetType, power_down_time: u8) -> Self {
+    ///
+    /// `power_down_time` is only valid for [`ResetType::EnableRapidPowerShutDown`], will be set to None otherwise
+    pub(crate) fn new(reset_type: ResetType, power_down_time: Option<u8>) -> Self {
+        let power_down_time = if reset_type == ResetType::EnableRapidPowerShutDown {
+            power_down_time
+        } else {
+            None
+        };
         Self {
             reset_type,
             power_down_time,
@@ -94,22 +102,38 @@ impl WireFormat for EcuResetResponse {
     /// Deserialization function to read a [`EcuResetResponse`] from a `Reader`
     fn decode<T: Read>(reader: &mut T) -> Result<Option<Self>, Error> {
         let reset_type = ResetType::try_from(reader.read_u8()?)?;
-        let power_down_time = reader.read_u8()?;
+        let mut power_down_time = None;
+        if reset_type == ResetType::EnableRapidPowerShutDown {
+            power_down_time = Some(reader.read_u8()?);
+        }
         Ok(Some(Self {
             reset_type,
             power_down_time,
         }))
     }
 
     fn required_size(&self) -> usize {
-        2
+        if self.reset_type == ResetType::EnableRapidPowerShutDown {
+            2
+        } else {
+            1
+        }
     }
 
     /// Serialization function to write a [`EcuResetResponse`] to a `Writer`
     fn encode<T: Write>(&self, buffer: &mut T) -> Result<usize, Error> {
         buffer.write_u8(u8::from(self.reset_type))?;
-        buffer.write_u8(self.power_down_time)?;
-        Ok(2)
+        if self.reset_type == ResetType::EnableRapidPowerShutDown {
+            if let Some(power_down_time) = self.power_down_time {
+                buffer.write_u8(power_down_time)?;
+            } else {
+                return Err(Error::SerializationError(
+                    "ECUReset: Power down time must be set for EnableRapidPowerShutDown"
+                        .to_string(),
+                ));
+            }
+        }
+        Ok(self.required_size())
     }
 }
 
@@ -139,8 +163,8 @@ mod response {
 
     #[test]
     fn ecu_reset_response() {
-        let bytes: [u8; 2] = [0x01, 0x20];
-        let resp = EcuResetResponse::new(ResetType::HardReset, 0x20);
+        let bytes: [u8; 2] = [0x04, 0x20];
+        let resp = EcuResetResponse::new(ResetType::EnableRapidPowerShutDown, Some(0x20));
         let mut buffer = Vec::new();
         let written = resp.encode(&mut buffer).unwrap();
         let result = EcuResetResponse::decode_single_value(&mut bytes.as_slice()).unwrap();
@@ -149,4 +173,16 @@ mod response {
         assert_eq!(written, 2);
         assert_eq!(written, resp.required_size());
     }
+
+    #[test]
+    // Test that power down time is ignored for other reset types
+    fn ecu_reset_response_no_power_down_time() {
+        let bytes: [u8; 1] = [0x01];
+        let resp = EcuResetResponse::new(ResetType::HardReset, Some(0x20));
+        let mut buffer = Vec::new();
+        let written = resp.encode(&mut buffer).unwrap();
+        assert_eq!(written, 1);
+        let result = EcuResetResponse::decode_single_value(&mut bytes.as_slice()).unwrap();
+        assert_eq!(result, EcuResetResponse::new(ResetType::HardReset, None));
+    }
 }

src/traits.rs

@@ -169,11 +169,16 @@ where
             _ => unreachable!("Impossible to read more than 2 bytes into 2 byte array"),
         }
 
-        match Self::try_from(u16::from_be_bytes(identifier_data)) {
-            Ok(identifier) => Ok(Some(identifier)),
-            Err(_) => Err(Error::InvalidDiagnosticIdentifier(u16::from_be_bytes(
+        if let Ok(identifier) = Self::try_from(u16::from_be_bytes(identifier_data)) {
+            Ok(Some(identifier))
+        } else {
+            tracing::error!(
+                "Invalid identifier value: {:#X}",
+                u16::from_be_bytes(identifier_data)
+            );
+            Err(Error::InvalidDiagnosticIdentifier(u16::from_be_bytes(
                 identifier_data,
-            ))),
+            )))
         }
     }