Merge pull request #18 from datdenkikniet/fru

Add more SDR record types

CHANGELOG.md

@@ -1,5 +1,4 @@
 # Unreleased
-* Rudimentary RMCP+ support ([#13])
 * Support for sending bridged IPMB messages for sensors that are not available on the system
   interface for File-based connections. `GetSensorReading::new()` and `GetSensorReading::for_sensor()`
   have been replaced with`GetSensorReading::for_sensor_key()` which now takes a `&Sensorkey`. ([#6])
@@ -8,14 +7,18 @@
 * Breaking change: rename `Loggable::into_log` to `Loggable::as_log` as part of clippy cleanup. ([#12])
 * Breaking change: use newtype & typesafe variants for `Channel` and `ChannelNumber` in relevant places. ([#14])
 * Breaking change: remove duplicate `get_channel_authentication_capabilities` file. ([#14])
-
+* Rudimentary RMCP+ support. ([#13])
+* Add more SDR record types. ([#10], [#18])
+* Breaking change: improve error reporting in SDR records. ([#10], [#18])
 
 [#6]: https://github.com/datdenkikniet/ipmi-rs/pull/6
 [#7]: https://github.com/datdenkikniet/ipmi-rs/pull/7
+[#10]: https://github.com/datdenkikniet/ipmi-rs/pull/10
 [#11]: https://github.com/datdenkikniet/ipmi-rs/pull/11
 [#12]: https://github.com/datdenkikniet/ipmi-rs/pull/12
 [#13]: https://github.com/datdenkikniet/ipmi-rs/pull/13
 [#14]: https://github.com/datdenkikniet/ipmi-rs/pull/14
+[#18]: https://github.com/datdenkikniet/ipmi-rs/pull/18
 
 # [0.2.1](https://github.com/datdenkikniet/ipmi-rs/tree/v0.2.1)
 

README.md

@@ -26,13 +26,13 @@ This example will, using the `/dev/ipmi0` file:
 - [ ] FRU information lookup
 - [x] `ioctl`-based IPMI device file interface support
     - Supports bridging IPMI requests to other IPMBs.
-- [ ] RMCP
+- [x] RMCP
     Supported auth types:
     - [x] Unauthenticated
-    - [ ] MD5
-    - [ ] MD2
+    - [x] MD5
+    - [x] MD2
     - [ ] Key
-- [ ] RMCP+
+- [x] RMCP+
 
 ## License
 

examples/get-info.rs

@@ -5,12 +5,14 @@ use indicatif::{ProgressBar, ProgressStyle};
 use ipmi_rs::{
     app::GetDeviceId,
     sensor_event::{GetSensorReading, ThresholdReading},
-    storage::sdr::{
-        record::RecordContents, GetDeviceSdrInfo, GetSdrAllocInfo, GetSdrRepositoryInfo, SdrCount,
-        SdrOperation,
+    storage::{
+        sdr::{
+            record::{IdentifiableSensor, InstancedSensor, RecordContents},
+            GetDeviceSdrInfo, GetSdrAllocInfo, GetSdrRepositoryInfo, SdrCount, SdrOperation,
+        },
+        sel::{GetSelAllocInfo, GetSelEntry, GetSelInfo, RecordId as SelRecordId, SelCommand},
     },
-    storage::sel::{GetSelAllocInfo, GetSelEntry, GetSelInfo, RecordId as SelRecordId, SelCommand},
-    LogOutput, SensorRecord,
+    LogOutput,
 };
 
 fn main() -> std::io::Result<()> {
@@ -86,28 +88,45 @@ fn main() -> std::io::Result<()> {
     progress_bar.finish();
 
     for sensor in &sensors {
-        if let RecordContents::FullSensor(full) = &sensor.contents {
-            let value = ipmi
-                .send_recv(GetSensorReading::for_sensor_key(full.key_data()))
-                .unwrap();
-
-            let reading: ThresholdReading = (&value).into();
-
-            if let Some(reading) = reading.reading {
-                if let Some(display) = full.display_reading(reading) {
-                    log::info!("{}: {}", full.id_string(), display);
-                    ipmi_rs::Logger::log(debug_log_output, sensor);
+        match &sensor.contents {
+            RecordContents::FullSensor(full) => {
+                log_id("Full Sensor Record", full);
+                let value = ipmi
+                    .send_recv(GetSensorReading::for_sensor_key(full.key_data()))
+                    .unwrap();
+
+                let reading: ThresholdReading = (&value).into();
+
+                if let Some(reading) = reading.reading {
+                    if let Some(display) = full.display_reading(reading) {
+                        log::info!("  {}: {}", full.id_string(), display);
+                        ipmi_rs::Logger::log(debug_log_output, sensor);
+                    }
+                } else {
+                    log::warn!("  No reading for {}", full.id_string());
                 }
-            } else {
-                log::warn!("No reading for {}", full.id_string());
             }
-        } else if let RecordContents::CompactSensor(compact) = &sensor.contents {
-            log::info!("Compact sensor {}", compact.id_string(),);
-            log::info!("  Sensor type: {:?}", compact.common().ty,);
-        } else if let RecordContents::Unknown { ty, .. } = &sensor.contents {
-            log::info!("Unknown record type. Type: 0x{ty:02X}");
+            RecordContents::CompactSensor(compact) => log_sensor("Compact Sensor", compact),
+            RecordContents::EventOnlySensor(event) => log_sensor("Event-only Sensor", event),
+            RecordContents::FruDeviceLocator(fru) => {
+                log_id("FRU Device Locator", fru);
+                log::info!("  Device type: {}", fru.device_type);
+            }
+            RecordContents::McDeviceLocator(mc) => log_id("MC Device Locator", mc),
+            RecordContents::Unknown { ty, .. } => {
+                log::info!("Unknown record type. Type: 0x{ty:02X}");
+            }
         }
     }
 
     Ok(())
 }
+
+fn log_id<T: IdentifiableSensor>(ty: &str, sensor: &T) {
+    log::info!("{ty} {}", sensor.id_string());
+}
+
+fn log_sensor<T: InstancedSensor>(ty: &str, sensor: &T) {
+    log_id(ty, sensor);
+    log::info!("  Sensor type: {:?}", sensor.ty());
+}

examples/sensors-of-type.rs

@@ -1,8 +1,10 @@
 use clap::Parser;
 use ipmi_rs::{
     sensor_event::{GetSensorReading, ThresholdReading},
-    storage::sdr::{record::RecordContents, Record, SensorType},
-    SensorRecord,
+    storage::sdr::{
+        record::{IdentifiableSensor, InstancedSensor, RecordContents},
+        Record, SensorType,
+    },
 };
 use log::Level;
 

src/connection/mod.rs

@@ -101,18 +101,31 @@ pub enum LogicalUnit {
     Three,
 }
 
+impl LogicalUnit {
+    /// Construct a `LogicalUnit` from the two lowest bits of `value`,
+    /// ignoring all other bits.
+    pub fn from_low_bits(value: u8) -> Self {
+        let value = value & 0b11;
+
+        match value {
+            0b00 => Self::Zero,
+            0b01 => Self::One,
+            0b10 => Self::Two,
+            0b11 => Self::Three,
+            _ => unreachable!("Value bitmasked with 0b11 has value greater than 3"),
+        }
+    }
+}
+
 impl TryFrom<u8> for LogicalUnit {
     type Error = ();
 
     fn try_from(value: u8) -> Result<Self, Self::Error> {
-        let val = match value {
-            0 => Self::Zero,
-            1 => Self::One,
-            2 => Self::Two,
-            3 => Self::Three,
-            _ => return Err(()),
-        };
-        Ok(val)
+        if value <= 0b11 {
+            Ok(Self::from_low_bits(value))
+        } else {
+            Err(())
+        }
     }
 }
 

src/lib.rs

@@ -6,7 +6,7 @@ mod error;
 pub use error::IpmiError;
 
 pub mod storage;
-pub use storage::sdr::record::SensorRecord;
+pub use storage::sdr::record::WithSensorRecordCommon;
 
 pub mod sensor_event;
 
@@ -113,21 +113,49 @@ where
     type Item = SdrRecord;
 
     fn next(&mut self) -> Option<Self::Item> {
-        let next_id = self.next_id?;
+        let current_id = self.next_id?;
         let next_record = self
             .ipmi
-            .send_recv(sdr::GetDeviceSdr::new(None, next_id))
-            .map_err(|e| {
-                log::error!("Error occured while iterating SDR records: {e:?}");
-            })
-            .ok()?;
-
-        if !next_record.next_entry.is_last() {
-            self.next_id = Some(next_record.next_entry);
-        } else {
+            .send_recv(sdr::GetDeviceSdr::new(None, current_id));
+
+        let (value, next_record_id) = match next_record {
+            Ok(record) => {
+                let next_record_id = record.next_entry;
+
+                (Some(record.record), next_record_id)
+            }
+            Err(IpmiError::ParsingFailed {
+                error: ParseResponseError::Parse((e, next_id)),
+                ..
+            }) => {
+                log::warn!(
+                    "Recoverable error while parsing SDR record 0x{:04X}: {e:?}",
+                    current_id.value()
+                );
+                (None, next_id)
+            }
+            Err(e) => {
+                log::error!(
+                    "Unrecoverable error while parsing SDR record 0x{:04X}: {e:?}",
+                    current_id.value()
+                );
+                self.next_id.take();
+                return None;
+            }
+        };
+
+        if current_id.is_last() {
             self.next_id.take();
+        } else {
+            if next_record_id == current_id {
+                log::error!("Got duplicate SDR record IDs! Stopping iteration.");
+                self.next_id.take();
+                return None;
+            } else {
+                self.next_id = Some(next_record_id);
+            }
         }
 
-        Some(next_record.record)
+        value
     }
 }

src/storage/sdr/get_sdr.rs

@@ -4,7 +4,7 @@ use nonmax::NonMaxU8;
 
 use crate::connection::{IpmiCommand, Message, NetFn, ParseResponseError};
 
-use super::{record::Record, RecordId};
+use super::{Record, RecordId, RecordParseError};
 
 /// Get a device SDR.
 ///
@@ -56,15 +56,23 @@ impl From<GetDeviceSdr> for Message {
 impl IpmiCommand for GetDeviceSdr {
     type Output = RecordInfo;
 
-    type Error = ();
+    type Error = (RecordParseError, RecordId);
 
     fn parse_response(
         completion_code: crate::connection::CompletionCode,
         data: &[u8],
     ) -> Result<Self::Output, ParseResponseError<Self::Error>> {
         Self::check_cc_success(completion_code)?;
 
-        RecordInfo::parse(data).ok_or(ParseResponseError::NotEnoughData)
+        if data.len() < 9 {
+            return Err(ParseResponseError::NotEnoughData);
+        }
+
+        let next_id = RecordId::new_raw(u16::from_le_bytes([data[0], data[1]]));
+
+        let res = RecordInfo::parse(data).map_err(|e| (e, next_id))?;
+
+        Ok(res)
     }
 }
 
@@ -75,14 +83,9 @@ pub struct RecordInfo {
 }
 
 impl RecordInfo {
-    pub fn parse(data: &[u8]) -> Option<Self> {
-        if data.len() < 9 {
-            return None;
-        }
-
+    pub fn parse(data: &[u8]) -> Result<Self, RecordParseError> {
         let next_entry = RecordId::new_raw(u16::from_le_bytes([data[0], data[1]]));
         let data = &data[2..];
-
         Record::parse(data).map(|record| Self { next_entry, record })
     }
 }

src/storage/sdr/mod.rs

@@ -5,7 +5,7 @@ mod get_sdr;
 pub use get_sdr::{GetDeviceSdr, RecordInfo as SdrRecordInfo, *};
 
 pub mod record;
-pub use record::Record;
+pub use record::{ParseError as RecordParseError, Record};
 
 mod get_info;
 pub use get_info::{

src/storage/sdr/record/compact_sensor_record.rs

@@ -24,28 +24,30 @@ pub struct CompactSensorRecord {
     pub oem_data: u8,
 }
 
-impl SensorRecord for CompactSensorRecord {
+impl WithSensorRecordCommon for CompactSensorRecord {
     fn common(&self) -> &SensorRecordCommon {
         &self.common
     }
+}
 
-    fn direction(&self) -> Direction {
-        self.direction
+impl DirectionalSensor for CompactSensorRecord {
+    fn direction(&self) -> &Direction {
+        &self.direction
     }
 }
 
 impl CompactSensorRecord {
-    pub fn parse(record_data: &[u8]) -> Option<Self> {
+    pub fn parse(record_data: &[u8]) -> Result<Self, ParseError> {
         if record_data.len() < 26 {
-            return None;
+            return Err(ParseError::NotEnoughData);
         }
 
         let (mut common, record_data) = SensorRecordCommon::parse_without_id(record_data)?;
 
         let direction_sharing_1 = record_data[0];
         let direction_sharing_2 = record_data[1];
 
-        let direction = Direction::try_from((direction_sharing_1 & 0xC) >> 6).unwrap();
+        let direction = Direction::try_from((direction_sharing_1 & 0xC) >> 6)?;
         let id_string_instance_modifier = match (direction_sharing_1 & 0x30) >> 4 {
             0b00 => IdStringModifier::Numeric,
             0b01 => IdStringModifier::Alpha,
@@ -72,11 +74,11 @@ impl CompactSensorRecord {
 
         let id_string_type_len = record_data[8];
         let id_string_bytes = &record_data[9..];
-        let id_string = TypeLengthRaw::new(id_string_type_len, id_string_bytes).into();
+        let id_string = TypeLengthRaw::new(id_string_type_len, id_string_bytes).try_into()?;
 
         common.set_id(id_string);
 
-        Some(Self {
+        Ok(Self {
             common,
             direction,
             record_sharing,