Re: [PATCH V6 3/8] libgpiod: Add rust wrapper crate

[Viresh Kumar <viresh.kumar@xxxxxxxxxx>]

On 15-10-22, 00:06, Kent Gibson wrote:
> If the mutators for each field still exist they may as well be pub.
> 
> And they should return Result<&mut Self> so they can be chained, as you
> suggest.
> 
> Wrt the values param (which I would prefer was called props)

Is this fine now ? Rebased over v7.

diff --git a/bindings/rust/libgpiod/src/line_settings.rs b/bindings/rust/libgpiod/src/line_settings.rs
index 2c3090132ea5..be50b5b41c5a 100644
--- a/bindings/rust/libgpiod/src/line_settings.rs
+++ b/bindings/rust/libgpiod/src/line_settings.rs
@@ -70,18 +70,18 @@ impl Settings {
     }

     /// Set line prop setting.
-    pub fn set_prop(&mut self, values: &[SettingVal]) -> Result<()> {
-        for value in values {
-            match value {
-                SettingVal::Direction(val) => self.set_direction(*val)?,
-                SettingVal::EdgeDetection(val) => self.set_edge_detection(*val)?,
-                SettingVal::Bias(val) => self.set_bias(*val)?,
-                SettingVal::Drive(val) => self.set_drive(*val)?,
-                SettingVal::ActiveLow(val) => self.set_active_low(*val),
-                SettingVal::DebouncePeriod(val) => self.set_debounce_period(*val),
-                SettingVal::EventClock(val) => self.set_event_clock(*val)?,
-                SettingVal::OutputValue(val) => self.set_output_value(*val)?,
-            }
+    pub fn set_prop(&mut self, props: &[SettingVal]) -> Result<()> {
+        for property in props {
+            match property {
+                SettingVal::Direction(prop) => self.set_direction(*prop)?,
+                SettingVal::EdgeDetection(prop) => self.set_edge_detection(*prop)?,
+                SettingVal::Bias(prop) => self.set_bias(*prop)?,
+                SettingVal::Drive(prop) => self.set_drive(*prop)?,
+                SettingVal::ActiveLow(prop) => self.set_active_low(*prop),
+                SettingVal::DebouncePeriod(prop) => self.set_debounce_period(*prop),
+                SettingVal::EventClock(prop) => self.set_event_clock(*prop)?,
+                SettingVal::OutputValue(prop) => self.set_output_value(*prop)?,
+            };
         }

         Ok(())
@@ -102,7 +102,7 @@ impl Settings {
     }

     /// Set the line direction.
-    fn set_direction(&mut self, direction: Direction) -> Result<()> {
+    pub fn set_direction(&mut self, direction: Direction) -> Result<&mut Self> {
         let ret = unsafe {
             gpiod::gpiod_line_settings_set_direction(
                 self.settings,
@@ -116,17 +116,17 @@ impl Settings {
                 Errno::last(),
             ))
         } else {
-            Ok(())
+            Ok(self)
         }
     }

     /// Get the direction setting.
-    fn direction(&self) -> Result<Direction> {
+    pub fn direction(&self) -> Result<Direction> {
         Direction::new(unsafe { gpiod::gpiod_line_settings_get_direction(self.settings) } as u32)
     }

     /// Set the edge event detection setting.
-    fn set_edge_detection(&mut self, edge: Option<Edge>) -> Result<()> {
+    pub fn set_edge_detection(&mut self, edge: Option<Edge>) -> Result<&mut Self> {
         let ret = unsafe {
             gpiod::gpiod_line_settings_set_edge_detection(
                 self.settings,
@@ -140,17 +140,17 @@ impl Settings {
                 Errno::last(),
             ))
         } else {
-            Ok(())
+            Ok(self)
         }
     }

     /// Get the edge event detection setting.
-    fn edge_detection(&self) -> Result<Option<Edge>> {
+    pub fn edge_detection(&self) -> Result<Option<Edge>> {
         Edge::new(unsafe { gpiod::gpiod_line_settings_get_edge_detection(self.settings) } as u32)
     }

     /// Set the bias setting.
-    fn set_bias(&mut self, bias: Option<Bias>) -> Result<()> {
+    pub fn set_bias(&mut self, bias: Option<Bias>) -> Result<&mut Self> {
         let ret = unsafe {
             gpiod::gpiod_line_settings_set_bias(self.settings, Bias::gpiod_bias(bias) as i32)
         };
@@ -161,17 +161,17 @@ impl Settings {
                 Errno::last(),
             ))
         } else {
-            Ok(())
+            Ok(self)
         }
     }

     /// Get the bias setting.
-    fn bias(&self) -> Result<Option<Bias>> {
+    pub fn bias(&self) -> Result<Option<Bias>> {
         Bias::new(unsafe { gpiod::gpiod_line_settings_get_bias(self.settings) } as u32)
     }

     /// Set the drive setting.
-    fn set_drive(&mut self, drive: Drive) -> Result<()> {
+    pub fn set_drive(&mut self, drive: Drive) -> Result<&mut Self> {
         let ret = unsafe {
             gpiod::gpiod_line_settings_set_drive(self.settings, drive.gpiod_drive() as i32)
         };
@@ -182,44 +182,49 @@ impl Settings {
                 Errno::last(),
             ))
         } else {
-            Ok(())
+            Ok(self)
         }
     }

     /// Get the drive setting.
-    fn drive(&self) -> Result<Drive> {
+    pub fn drive(&self) -> Result<Drive> {
         Drive::new(unsafe { gpiod::gpiod_line_settings_get_drive(self.settings) } as u32)
     }

     /// Set active-low setting.
-    fn set_active_low(&mut self, active_low: bool) {
-        unsafe { gpiod::gpiod_line_settings_set_active_low(self.settings, active_low) }
+    pub fn set_active_low(&mut self, active_low: bool) -> &mut Self {
+        unsafe {
+            gpiod::gpiod_line_settings_set_active_low(self.settings, active_low);
+        }
+        self
     }

     /// Check the active-low setting.
-    fn active_low(&self) -> bool {
+    pub fn active_low(&self) -> bool {
         unsafe { gpiod::gpiod_line_settings_get_active_low(self.settings) }
     }

     /// Set the debounce period setting.
-    fn set_debounce_period(&mut self, period: Duration) {
+    pub fn set_debounce_period(&mut self, period: Duration) -> &mut Self {
         unsafe {
             gpiod::gpiod_line_settings_set_debounce_period_us(
                 self.settings,
                 period.as_micros() as u64,
-            )
+            );
         }
+
+        self
     }

     /// Get the debounce period.
-    fn debounce_period(&self) -> Result<Duration> {
+    pub fn debounce_period(&self) -> Result<Duration> {
         Ok(Duration::from_micros(unsafe {
             gpiod::gpiod_line_settings_get_debounce_period_us(self.settings)
         }))
     }

     /// Set the event clock setting.
-    fn set_event_clock(&mut self, clock: EventClock) -> Result<()> {
+    pub fn set_event_clock(&mut self, clock: EventClock) -> Result<&mut Self> {
         let ret = unsafe {
             gpiod::gpiod_line_settings_set_event_clock(self.settings, clock.gpiod_clock() as i32)
         };
@@ -230,17 +235,17 @@ impl Settings {
                 Errno::last(),
             ))
         } else {
-            Ok(())
+            Ok(self)
         }
     }

     /// Get the event clock setting.
-    fn event_clock(&self) -> Result<EventClock> {
+    pub fn event_clock(&self) -> Result<EventClock> {
         EventClock::new(unsafe { gpiod::gpiod_line_settings_get_event_clock(self.settings) } as u32)
     }

     /// Set the output value setting.
-    fn set_output_value(&mut self, value: Value) -> Result<()> {
+    pub fn set_output_value(&mut self, value: Value) -> Result<&mut Self> {
         let ret =
             unsafe { gpiod::gpiod_line_settings_set_output_value(self.settings, value.value()) };

@@ -250,12 +255,12 @@ impl Settings {
                 Errno::last(),
             ))
         } else {
-            Ok(())
+            Ok(self)
         }
     }

     /// Get the output value, 0 or 1.
-    fn output_value(&self) -> Result<Value> {
+    pub fn output_value(&self) -> Result<Value> {
         let value = unsafe { gpiod::gpiod_line_settings_get_output_value(self.settings) };

         if value != 0 && value != 1 {

-- 
viresh