8f49f3a29406 — flub@powell.devork.be 5 years ago
Refactor brightctl to use bright::dbus::Client

Now that we have this client, we should use it whereever we need a client.
2 files changed, 52 insertions(+), 119 deletions(-)

M src/bin/brightctl.rs
M src/dbus.rs
M src/bin/brightctl.rs +21 -117
@@ 9,7 9,6 @@ extern crate bright;
 use std::collections::HashMap;
 
 use bright::*;
-use dbus::stdintf::org_freedesktop_dbus::Properties;
 
 // Application configuration
 #[derive(Debug)]

          
@@ 134,19 133,15 @@ fn get_brightness(cfg: AppConfig) -> Res
 }
 
 fn get_brightness_dbus(cfg: &AppConfig) -> Result<(), failure::Error> {
-    let conn = dbus_connection(&cfg.bustype)?;
-    let devices = dbus_devices(&conn)?;
-    for objpath in devices {
-        let obj = conn.with_path(bright::dbus::BUSNAME, objpath, 500);
-        let name: String = obj
-            .get(bright::dbus::DEVICE_IFACE, "Name")
-            .map_err(|e| format_err!("{:?} {:?}", e.name(), e.message()))?;
+    // let client = bright::dbus::Client::new(cfg.bustype, bright::dbus::BUSNAME);
+
+    let client = dbus_client(&cfg.bustype)?;
+    for objpath in client.devices()?.iter() {
+        let name = client.name(objpath.to_owned())?;
         if cfg.devices.len() > 0 && !cfg.devices.contains(&name) {
             continue;
         }
-        let val: u32 = obj
-            .get(bright::dbus::DEVICE_IFACE, "Brightness")
-            .map_err(|e| format_err!("{:?} {:?}", e.name(), e.message()))?;
+        let val = client.get(objpath.to_owned())?;
         println!("{}/{}", name, val);
     }
     Ok(())

          
@@ 172,13 167,9 @@ fn set_brightness(cfg: AppConfig) -> Res
 }
 
 fn set_brightness_dbus(cfg: &AppConfig) -> Result<(), failure::Error> {
-    let conn = dbus_connection(&cfg.bustype)?;
-    let devices = dbus_devices(&conn)?;
-    for objpath in devices {
-        let obj = conn.with_path(bright::dbus::BUSNAME, objpath, 500);
-        let name: String = obj
-            .get(bright::dbus::DEVICE_IFACE, "Name")
-            .map_err(|e| format_err!("{:?} {:?}", e.name(), e.message()))?;
+    let client = dbus_client(&cfg.bustype)?;
+    for objpath in client.devices()?.iter() {
+        let name = client.name(objpath.to_owned())?;
         if cfg.devices.len() > 0 && !cfg.devices.contains(&name) {
             continue;
         }

          
@@ 188,65 179,19 @@ fn set_brightness_dbus(cfg: &AppConfig) 
                     Some(min) => std::cmp::max(n, min),
                     None => n,
                 };
-                obj.set(bright::dbus::DEVICE_IFACE, "Brightness", val)
-                    .map_err(|e| {
-                        format_err!(
-                            "Failed to set property {}.Brightness on {}: {}",
-                            bright::dbus::DEVICE_IFACE,
-                            obj.path,
-                            e
-                        )
-                    })?;
+                client.set(objpath.to_owned(), val)?;
             }
             Action::Incr(n) => {
-                let cur: u32 = obj
-                    .get(bright::dbus::DEVICE_IFACE, "Brightness")
-                    .map_err(|e| {
-                        format_err!(
-                            "Failed to get property {}.Brightness from {}: {}",
-                            bright::dbus::DEVICE_IFACE,
-                            obj.path,
-                            e
-                        )
-                    })?;
+                let cur = client.get(objpath.to_owned())?;
                 let val = match cfg.minimum {
                     Some(min) => std::cmp::max(cur + n, min),
                     None => cur + n,
                 };
-                obj.set(bright::dbus::DEVICE_IFACE, "Brightness", val)
-                    .map_err(|e| {
-                        format_err!(
-                            "Failed to set property {}.Brightness on {}: {}",
-                            bright::dbus::DEVICE_IFACE,
-                            obj.path,
-                            e
-                        )
-                    })?;
+                client.set(objpath.to_owned(), val)?;
             }
             Action::Decr(n) => {
-                let cur: u32 = obj
-                    .get(bright::dbus::DEVICE_IFACE, "Brightness")
-                    .map_err(|e| {
-                        format_err!(
-                            "Failed to get property {}.Brightness from {}: {}",
-                            bright::dbus::DEVICE_IFACE,
-                            obj.path,
-                            e
-                        )
-                    })?;
-                let val = match cfg.minimum {
-                    Some(min) => std::cmp::max(cur as i32 - n as i32, min as i32) as u32,
-                    None => std::cmp::max(cur as i32 - n as i32, 0) as u32,
-                };
-                obj.set(bright::dbus::DEVICE_IFACE, "Brightness", val)
-                    .map_err(|e| {
-                        format_err!(
-                            "Failed to set property {}.Brightness on {}: {}",
-                            bright::dbus::DEVICE_IFACE,
-                            obj.path,
-                            e
-                        )
-                    })?;
+                let min = cfg.minimum.unwrap_or(0);
+                client.dec(objpath.to_owned(), n, min)?;
             }
             _ => panic!("Action other then set/incr/decr"),
         }

          
@@ 315,25 260,18 @@ fn watch(_cfg: AppConfig) -> Result<(), 
 // be.devork.dbus.bright busname is registered, respecting the
 // DBusType to connct to the correct bus.  This must not be called
 // with DBusType::None as that does not make sense.
-fn dbus_connection(bustype: &DBusType) -> Result<dbus::Connection, failure::Error> {
+fn dbus_client(bustype: &DBusType) -> Result<bright::dbus::Client, failure::Error> {
     if *bustype == DBusType::None {
         panic!("Should not call dbus_connection with DBusType::None");
     }
-    let ping = dbus::Message::new_method_call(
-        bright::dbus::BUSNAME,
-        "/",
-        "org.freedesktop.DBus.Peer",
-        "Ping",
-    )
-    .map_err(|e| failure::err_msg(e))?;
     let bus = match *bustype {
         DBusType::Auto | DBusType::System => dbus::BusType::System,
         DBusType::Session => dbus::BusType::Session,
         _ => panic!("Wrong bustype"),
     };
-    match dbus::Connection::get_private(bus) {
-        Ok(conn) => match conn.send_with_reply_and_block(ping, 200) {
-            Ok(_msg) => return Ok(conn),
+    match bright::dbus::Client::new(bus, bright::dbus::BUSNAME) {
+        Ok(client) => match client.ping() {
+            Ok(_) => return Ok(client),
             Err(e) => match *bustype {
                 DBusType::System | DBusType::Session => {
                     return Err(format_err!(

          
@@ 355,16 293,9 @@ fn dbus_connection(bustype: &DBusType) -
             DBusType::None => panic!("oops"),
         },
     }
-    let ping = dbus::Message::new_method_call(
-        bright::dbus::BUSNAME,
-        "/",
-        "org.freedesktop.DBus.Peer",
-        "Ping",
-    )
-    .map_err(|e| failure::err_msg(e))?;
-    match dbus::Connection::get_private(dbus::BusType::System) {
-        Ok(conn) => match conn.send_with_reply_and_block(ping, 200) {
-            Ok(_msg) => Ok(conn),
+    match bright::dbus::Client::new(dbus::BusType::System, bright::dbus::BUSNAME) {
+        Ok(client) => match client.ping() {
+            Ok(_) => Ok(client),
             Err(e) => Err(format_err!(
                 "{} not found on {:?}: {}",
                 bright::dbus::BUSNAME,

          
@@ 375,30 306,3 @@ fn dbus_connection(bustype: &DBusType) -
         Err(e) => Err(format_err!("Failed to connect to any D-Bus bus: {}", e)),
     }
 }
-
-// Return the object paths for the devices.
-fn dbus_devices(conn: &dbus::Connection) -> Result<(Vec<dbus::Path>), failure::Error> {
-    let meth = dbus::Message::new_method_call(
-        bright::dbus::BUSNAME,
-        bright::dbus::MANAGER_PATH,
-        bright::dbus::MANAGER_IFACE,
-        "GetDevices",
-    )
-    .map_err(|e| failure::err_msg(e))?;
-    let ret = conn.send_with_reply_and_block(meth, 200).map_err(|e| {
-        format_err!(
-            "Failed D-Bus call {}.GetDevices(): {}",
-            bright::dbus::MANAGER_IFACE,
-            e
-        )
-    })?;
-    match ret.get1::<Vec<String>>() {
-        Some(objs) => Ok(objs
-            .into_iter()
-            .map(|s| dbus::Path::new(s).unwrap())
-            .collect()),
-        None => Err(failure::err_msg(
-            "Bad return value fromm be.devork.dbus.bright.Manager.GetDevices()",
-        )),
-    }
-}

          
M src/dbus.rs +31 -2
@@ 530,6 530,27 @@ impl<'a> Client<'a> {
             .or_else(|_| Err(failure::err_msg("Failed sending Term() message")))
     }
 
+    /// Ping the peer.
+    ///
+    /// # Errors
+    ///
+    /// Returns a `ClientError` with a cause which is a
+    /// `failure::Context` which has a `ClientErrorKind::CallFailed`
+    /// context if the peer did not respond.
+    pub fn ping(&self) -> Result<(), ClientError> {
+        let meth = dbus::Message::new_method_call(
+            self.peer.clone(),
+            "/",
+            "org.freedesktop.DBus.Peer",
+            "Ping",
+        )
+        .unwrap();
+        self.conn
+            .send_with_reply_and_block(meth, self.timeout)
+            .context(ClientErrorKind::CallFailed)?;
+        Ok(())
+    }
+
     pub fn devices(&self) -> Result<Vec<dbus::Path>, ClientError> {
         let meth = dbus::Message::new_method_call(
             self.peer.clone(),

          
@@ 551,6 572,14 @@ impl<'a> Client<'a> {
         }
     }
 
+    pub fn name(&self, dev: dbus::Path) -> Result<String, ClientError> {
+        let obj = self.conn.with_path(self.peer.clone(), dev, self.timeout);
+        let name: String = obj
+            .get(DEVICE_IFACE, "Name")
+            .context(ClientErrorKind::CallFailed)?;
+        Ok(name)
+    }
+
     pub fn get(&self, dev: dbus::Path) -> Result<u32, ClientError> {
         let obj = self.conn.with_path(self.peer.clone(), dev, self.timeout);
         let val: u32 = obj

          
@@ 586,8 615,8 @@ impl<'a> Client<'a> {
             return Err(ClientErrorKind::BrightnessRange)?;
         }
         let cur = self.get(dev.clone())?;
-        let new = match cur - step {
-            v if v >= min => v,
+        let new: u32 = match cur as i32 - step as i32 {
+            v if v >= min as i32 => v as u32,
             _ => min,
         };
         self.set(dev, new)?;