From d68f377ab1e3b4d017a52997d37dcf9062c837df Mon Sep 17 00:00:00 2001 From: Junnan Wu Date: Thu, 29 Feb 2024 10:24:45 +0800 Subject: [PATCH] scmi: set name only when missing name If vhost-device-scmi running with name argument, do not set name again. If not, use value from fs, otherwise use default name. It can avoid that program exit unexpectedly when the length of native sensor name is too long. Correspondingly, modify related unit test function. Signed-off-by: Junnan Wu --- vhost-device-scmi/src/devices/common.rs | 10 ++++----- vhost-device-scmi/src/devices/fake.rs | 2 +- vhost-device-scmi/src/devices/iio.rs | 28 ++++++++++++++++--------- 3 files changed, 24 insertions(+), 16 deletions(-) diff --git a/vhost-device-scmi/src/devices/common.rs b/vhost-device-scmi/src/devices/common.rs index 67adf2a..4b259ba 100644 --- a/vhost-device-scmi/src/devices/common.rs +++ b/vhost-device-scmi/src/devices/common.rs @@ -224,7 +224,7 @@ pub fn devices_help() -> String { #[derive(Debug)] pub struct Sensor { /// The sensor name (possibly truncated) as reported to the guest. - pub name: String, + pub name: Option, /// Whether the sensor is enabled. /// /// Sensors can be enabled and disabled using SCMI. [Sensor]s created @@ -233,10 +233,9 @@ pub struct Sensor { } impl Sensor { - pub fn new(properties: &DeviceProperties, default_name: &str) -> Self { - let name = properties.get("name").unwrap_or(default_name); + pub fn new(properties: &DeviceProperties) -> Self { Self { - name: name.to_owned(), + name: properties.get("name").map(|s| (*s).to_owned()), enabled: false, } } @@ -323,7 +322,8 @@ pub trait SensorT: Send { } else { self.format_unit(0) }; - let name = self.sensor().name.clone(); + // During initialization, sensor name has be set. + let name = self.sensor().name.clone().unwrap(); let values: MessageValues = vec![ // attributes low MessageValue::Unsigned(low), diff --git a/vhost-device-scmi/src/devices/fake.rs b/vhost-device-scmi/src/devices/fake.rs index c0936b4..de786d3 100644 --- a/vhost-device-scmi/src/devices/fake.rs +++ b/vhost-device-scmi/src/devices/fake.rs @@ -59,7 +59,7 @@ impl SensorT for FakeSensor { impl FakeSensor { pub fn new_device(properties: &DeviceProperties) -> MaybeDevice { properties.check(&[], &["name"])?; - let sensor = Sensor::new(properties, "fake"); + let sensor = Sensor::new(properties); let fake_sensor = Self { sensor, value: 0 }; let sensor_device = SensorDevice(Box::new(fake_sensor)); Ok(Box::new(sensor_device)) diff --git a/vhost-device-scmi/src/devices/iio.rs b/vhost-device-scmi/src/devices/iio.rs index 1c300af..bfe4a05 100644 --- a/vhost-device-scmi/src/devices/iio.rs +++ b/vhost-device-scmi/src/devices/iio.rs @@ -159,6 +159,8 @@ const UNIT_MAPPING: &[UnitMapping] = &[ }, ]; +const IIO_DEFAULT_NAME: &str = "iio"; + /// Representation of an IIO channel axis. /// /// Used also for scalar values. @@ -224,6 +226,10 @@ impl SensorT for IIOSensor { } Err(error) => return Err(DeviceError::IOError(self.path.clone(), error)), } + // If both no name param and no name sysfs exist, use default name + if self.sensor.name.is_none() { + self.sensor.name = Some(IIO_DEFAULT_NAME.to_owned()); + } if axes.is_empty() { return Err(DeviceError::GenericError(format!( "No {:?} channel found in {:?}", @@ -329,7 +335,7 @@ impl IIOSensor { #[allow(clippy::new_ret_no_self)] pub fn new(properties: &DeviceProperties) -> Result { properties.check(&["path", "channel"], &["name"])?; - let sensor = Sensor::new(properties, "iio"); + let sensor = Sensor::new(properties); Ok(Self { sensor, path: OsString::from(properties.get("path").unwrap()), @@ -347,7 +353,7 @@ impl IIOSensor { fn set_sensor_name_from_file(&mut self, path: &PathBuf) { match fs::read_to_string(path) { - Ok(name) => self.sensor_mut().name = name, + Ok(name) => self.sensor_mut().name = Some(name), Err(error) => warn!( "Error reading IIO device name from {}: {}", path.display(), @@ -396,7 +402,9 @@ impl IIOSensor { let os_file_name = file.file_name(); let file_name = os_file_name.to_str().unwrap_or_default(); let raw_suffix = "_raw"; - if file_name == "name" { + // Only override name when no defined by parameter. + // If both fs and parameter exist, use parameter first and skip from file. + if file_name == "name" && self.sensor.name.is_none() { self.set_sensor_name_from_file(&file.path()); } else if file_name.starts_with(channel) && file_name.ends_with(raw_suffix) { let infix = &file_name[channel.len()..file_name.len() - raw_suffix.len()]; @@ -634,18 +642,18 @@ mod tests { #[test] fn test_sensor_name_from_fs() { let directory = IIODirectory::new(&[("in_accel_raw", "123"), ("name", "foo")]); - let mut sensor = - make_iio_sensor(&directory, "in_accel".to_owned(), Some("accel".to_owned())); + let mut sensor = make_iio_sensor(&directory, "in_accel".to_owned(), None); sensor.initialize().unwrap(); - assert_eq!(sensor.sensor.name, "foo"); + assert_eq!(sensor.sensor.name, Some("foo".to_owned())); } #[test] fn test_sensor_name_from_params() { - let directory = IIODirectory::new(&[("in_accel_raw", "123")]); - let mut sensor = make_iio_sensor(&directory, "in_accel".to_owned(), Some("foo".to_owned())); + let directory = IIODirectory::new(&[("in_accel_raw", "123"), ("name", "foo")]); + let mut sensor = + make_iio_sensor(&directory, "in_accel".to_owned(), Some("accel".to_owned())); sensor.initialize().unwrap(); - assert_eq!(sensor.sensor.name, "foo"); + assert_eq!(sensor.sensor.name, Some("accel".to_owned())); } #[test] @@ -653,7 +661,7 @@ mod tests { let directory = IIODirectory::new(&[("in_accel_raw", "123")]); let mut sensor = make_iio_sensor(&directory, "in_accel".to_owned(), None); sensor.initialize().unwrap(); - assert_eq!(sensor.sensor.name, "iio"); + assert_eq!(sensor.sensor.name, Some("iio".to_owned())); } #[test]