chore: refactor ptp handling

Signed-off-by: Nikolaos Karaolidis <nick@karaolidis.com>

src/cli/backup/mod.rs

@@ -1,4 +1,4 @@
-use std::{error::Error, fmt};
+use std::error::Error;
 
 use crate::usb;
 

src/cli/device/mod.rs

@@ -3,7 +3,10 @@ use std::{error::Error, fmt};
 use clap::Subcommand;
 use serde::Serialize;
 
-use crate::{hardware::FujiUsbMode, usb};
+use crate::{
+    hardware::{CameraImpl, FujiUsbMode},
+    usb,
+};
 
 #[derive(Subcommand, Debug)]
 pub enum DeviceCmd {
@@ -24,13 +27,13 @@ pub struct CameraItemRepr {
     pub product_id: String,
 }
 
-impl From<&usb::Camera> for CameraItemRepr {
+impl From<&Box<dyn CameraImpl>> for CameraItemRepr {
-    fn from(device: &usb::Camera) -> Self {
+    fn from(camera: &Box<dyn CameraImpl>) -> Self {
         CameraItemRepr {
-            id: device.id(),
+            id: camera.usb_id(),
-            name: device.name(),
+            name: camera.id().name.to_string(),
-            vendor_id: format!("0x{:04x}", device.vendor_id()),
+            vendor_id: format!("0x{:04x}", camera.id().vendor),
-            product_id: format!("0x{:04x}", device.product_id()),
+            product_id: format!("0x{:04x}", camera.id().product),
         }
     }
 }
@@ -101,7 +104,7 @@ impl fmt::Display for CameraRepr {
 }
 
 fn handle_info(json: bool, device_id: Option<&str>) -> Result<(), Box<dyn Error + Send + Sync>> {
-    let camera = usb::get_camera(device_id)?;
+    let mut camera = usb::get_camera(device_id)?;
 
     let info = camera.get_info()?;
     let mode = camera.get_fuji_usb_mode()?;

src/hardware/mod.rs

@@ -7,13 +7,37 @@ use serde::Serialize;
 
 mod xt5;
 
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+pub struct CameraId {
+    pub name: &'static str,
+    pub vendor: u16,
+    pub product: u16,
+}
+
+pub struct SupportedCamera {
+    pub id: CameraId,
+    pub factory: fn(
+        rusb::Device<GlobalContext>,
+    ) -> Result<Box<dyn CameraImpl>, Box<dyn Error + Send + Sync>>,
+}
+
+pub const SUPPORTED_CAMERAS: &[SupportedCamera] = &[SupportedCamera {
+    id: xt5::FUJIFILM_XT5,
+    factory: |d| xt5::FujifilmXT5::new_boxed(d),
+}];
+
+impl SupportedCamera {
+    pub fn matches_descriptor(&self, descriptor: &DeviceDescriptor) -> bool {
+        descriptor.vendor_id() == self.id.vendor && descriptor.product_id() == self.id.product
+    }
+}
+
 pub const TIMEOUT: Duration = Duration::from_millis(500);
 
 #[repr(u32)]
 #[derive(Debug, Clone, Copy)]
 pub enum DevicePropCode {
     FujiUsbMode = 0xd16e,
-    FujiBatteryInfo1 = 0xD36A,
     FujiBatteryInfo2 = 0xD36B,
 }
 
@@ -43,17 +67,30 @@ impl fmt::Display for FujiUsbMode {
 }
 
 pub trait CameraImpl {
-    fn name(&self) -> &'static str;
+    fn id(&self) -> &'static CameraId;
+
+    fn usb_id(&self) -> String;
+
+    fn ptp(&self) -> libptp::Camera<GlobalContext>;
+
+    fn get_info(&self) -> Result<DeviceInfo, Box<dyn Error + Send + Sync>> {
+        debug!("Sending GetDeviceInfo command");
+        let response =
+            self.ptp()
+                .command(StandardCommandCode::GetDeviceInfo, &[], None, Some(TIMEOUT))?;
+        debug!("Received response with {} bytes", response.len());
+
+        let info = DeviceInfo::decode(&response)?;
+        Ok(info)
+    }
 
     fn next_session_id(&self) -> u32;
 
-    fn open_session(
+    fn open_session(&self) -> Result<(), Box<dyn Error + Send + Sync>> {
-        &self,
+        let session_id = self.next_session_id();
-        ptp: &mut libptp::Camera<GlobalContext>,
+
-        session_id: u32,
-    ) -> Result<(), Box<dyn Error + Send + Sync>> {
         debug!("Opening new session with id {}", session_id);
-        ptp.command(
+        self.ptp().command(
             StandardCommandCode::OpenSession,
             &[session_id],
             None,
@@ -63,34 +100,30 @@ pub trait CameraImpl {
         Ok(())
     }
 
-    fn close_session(
+    fn close_session(&self) -> Result<(), Box<dyn Error + Send + Sync>> {
-        &self,
-        ptp: &mut libptp::Camera<GlobalContext>,
-    ) -> Result<(), Box<dyn Error + Send + Sync>> {
         debug!("Closing session");
-        ptp.command(StandardCommandCode::CloseSession, &[], None, Some(TIMEOUT))?;
+        self.ptp()
+            .command(StandardCommandCode::CloseSession, &[], None, Some(TIMEOUT))?;
 
         Ok(())
     }
 
     fn get_prop_value_raw(
         &self,
-        ptp: &mut libptp::Camera<GlobalContext>,
         prop: DevicePropCode,
     ) -> Result<Vec<u8>, Box<dyn Error + Send + Sync>> {
-        let session_id = self.next_session_id();
+        self.open_session()?;
-        self.open_session(ptp, session_id)?;
 
         debug!("Getting property {:?}", prop);
 
-        let response = ptp.command(
+        let response = self.ptp().command(
             StandardCommandCode::GetDevicePropValue,
             &[prop as u32],
             None,
             Some(TIMEOUT),
         );
 
-        self.close_session(ptp)?;
+        self.close_session()?;
 
         let response = response?;
         debug!("Received response with {} bytes", response.len());
@@ -100,10 +133,9 @@ pub trait CameraImpl {
 
     fn get_prop_value_scalar(
         &self,
-        ptp: &mut libptp::Camera<GlobalContext>,
         prop: DevicePropCode,
     ) -> Result<u32, Box<dyn Error + Send + Sync>> {
-        let data = self.get_prop_value_raw(ptp, prop)?;
+        let data = self.get_prop_value_raw(prop)?;
 
         match data.len() {
             1 => Ok(data[0] as u32),
@@ -113,19 +145,13 @@ pub trait CameraImpl {
         }
     }
 
-    fn get_fuji_usb_mode(
+    fn get_fuji_usb_mode(&self) -> Result<FujiUsbMode, Box<dyn Error + Send + Sync>> {
-        &self,
+        let result = self.get_prop_value_scalar(DevicePropCode::FujiUsbMode)?;
-        ptp: &mut libptp::Camera<GlobalContext>,
-    ) -> Result<FujiUsbMode, Box<dyn Error + Send + Sync>> {
-        let result = self.get_prop_value_scalar(ptp, DevicePropCode::FujiUsbMode)?;
         Ok(result.into())
     }
 
-    fn get_fuji_battery_info(
+    fn get_fuji_battery_info(&self) -> Result<u32, Box<dyn Error + Send + Sync>> {
-        &self,
+        let data = self.get_prop_value_raw(DevicePropCode::FujiBatteryInfo2)?;
-        ptp: &mut libptp::Camera<GlobalContext>,
-    ) -> Result<u32, Box<dyn Error + Send + Sync>> {
-        let data = self.get_prop_value_raw(ptp, DevicePropCode::FujiBatteryInfo2)?;
         debug!("Raw battery data: {:?}", data);
 
         if data.len() < 3 {
@@ -151,44 +177,4 @@ pub trait CameraImpl {
 
         Ok(percentage)
     }
-
-    fn get_info(
-        &self,
-        ptp: &mut libptp::Camera<GlobalContext>,
-    ) -> Result<DeviceInfo, Box<dyn Error + Send + Sync>> {
-        debug!("Sending GetDeviceInfo command");
-        let response = ptp.command(StandardCommandCode::GetDeviceInfo, &[], None, Some(TIMEOUT))?;
-        debug!("Received response with {} bytes", response.len());
-
-        let info = DeviceInfo::decode(&response)?;
-        Ok(info)
-    }
-}
-
-#[derive(Debug, Clone, Copy, PartialEq, Eq)]
-pub struct CameraId {
-    pub vendor: u16,
-    pub product: u16,
-}
-
-pub struct SupportedCamera {
-    pub id: CameraId,
-    pub factory: fn() -> Box<dyn CameraImpl>,
-}
-
-pub const SUPPORTED_CAMERAS: &[SupportedCamera] = &[SupportedCamera {
-    id: xt5::FUJIFILM_XT5,
-    factory: || Box::new(xt5::FujifilmXT5::new()),
-}];
-
-impl From<&SupportedCamera> for Box<dyn CameraImpl> {
-    fn from(camera: &SupportedCamera) -> Self {
-        (camera.factory)()
-    }
-}
-
-impl SupportedCamera {
-    pub fn matches_descriptor(&self, descriptor: &DeviceDescriptor) -> bool {
-        descriptor.vendor_id() == self.id.vendor && descriptor.product_id() == self.id.product
-    }
 }

src/hardware/xt5.rs

@@ -1,28 +1,52 @@
-use std::sync::atomic::{AtomicU32, Ordering};
+use std::{
+    error::Error,
+    sync::atomic::{AtomicU32, Ordering},
+};
+
+use rusb::GlobalContext;
 
 use super::{CameraId, CameraImpl};
 
 pub const FUJIFILM_XT5: CameraId = CameraId {
+    name: "FUJIFILM XT-5",
     vendor: 0x04cb,
     product: 0x02fc,
 };
 
-#[derive(Debug)]
 pub struct FujifilmXT5 {
+    device: rusb::Device<GlobalContext>,
     session_counter: AtomicU32,
 }
 
 impl FujifilmXT5 {
-    pub fn new() -> Self {
+    pub fn new_boxed(
-        Self {
+        rusb_device: rusb::Device<GlobalContext>,
-            session_counter: AtomicU32::new(1),
+    ) -> Result<Box<dyn CameraImpl>, Box<dyn Error + Send + Sync>> {
-        }
+        let session_counter = AtomicU32::new(1);
+
+        let handle = rusb_device.open()?;
+        let device = handle.device();
+
+        Ok(Box::new(Self {
+            session_counter,
+            device,
+        }))
     }
 }
 
 impl CameraImpl for FujifilmXT5 {
-    fn name(&self) -> &'static str {
+    fn id(&self) -> &'static CameraId {
-        "FUJIFILM X-T5"
+        &FUJIFILM_XT5
+    }
+
+    fn usb_id(&self) -> String {
+        let bus = self.device.bus_number();
+        let address = self.device.address();
+        format!("{}.{}", bus, address)
+    }
+
+    fn ptp(&self) -> libptp::Camera<rusb::GlobalContext> {
+        libptp::Camera::new(&self.device).unwrap()
     }
 
     fn next_session_id(&self) -> u32 {

src/usb/mod.rs

@@ -1,60 +1,9 @@
 use std::error::Error;
 
-use libptp::DeviceInfo;
+use crate::hardware::{CameraImpl, SUPPORTED_CAMERAS};
-use rusb::GlobalContext;
 
-use crate::hardware::{FujiUsbMode, SUPPORTED_CAMERAS};
+pub fn get_connected_camers()
-
+-> Result<Vec<Box<dyn crate::hardware::CameraImpl>>, Box<dyn Error + Send + Sync>> {
-pub struct Camera {
-    camera_impl: Box<dyn crate::hardware::CameraImpl>,
-    rusb_device: rusb::Device<GlobalContext>,
-}
-
-impl Camera {
-    pub fn id(&self) -> String {
-        let bus = self.rusb_device.bus_number();
-        let address = self.rusb_device.address();
-        format!("{}.{}", bus, address)
-    }
-
-    pub fn name(&self) -> String {
-        self.camera_impl.name().to_string()
-    }
-
-    pub fn vendor_id(&self) -> u16 {
-        let descriptor = self.rusb_device.device_descriptor().unwrap();
-        descriptor.vendor_id()
-    }
-
-    pub fn product_id(&self) -> u16 {
-        let descriptor = self.rusb_device.device_descriptor().unwrap();
-        descriptor.product_id()
-    }
-
-    pub fn ptp(&self) -> Result<libptp::Camera<GlobalContext>, Box<dyn Error + Send + Sync>> {
-        let handle = self.rusb_device.open()?;
-        let device = handle.device();
-        let ptp = libptp::Camera::new(&device)?;
-        Ok(ptp)
-    }
-
-    pub fn get_info(&self) -> Result<DeviceInfo, Box<dyn Error + Send + Sync>> {
-        let mut ptp = self.ptp()?;
-        self.camera_impl.get_info(&mut ptp)
-    }
-
-    pub fn get_fuji_usb_mode(&self) -> Result<FujiUsbMode, Box<dyn Error + Send + Sync>> {
-        let mut ptp = self.ptp()?;
-        self.camera_impl.get_fuji_usb_mode(&mut ptp)
-    }
-
-    pub fn get_fuji_battery_info(&self) -> Result<u32, Box<dyn Error + Send + Sync>> {
-        let mut ptp = self.ptp()?;
-        self.camera_impl.get_fuji_battery_info(&mut ptp)
-    }
-}
-
-pub fn get_connected_camers() -> Result<Vec<Camera>, Box<dyn Error + Send + Sync>> {
     let mut connected_cameras = Vec::new();
 
     for device in rusb::devices()?.iter() {
@@ -65,11 +14,7 @@ pub fn get_connected_camers() -> Result<Vec<Camera>, Box<dyn Error + Send + Sync
 
         for camera in SUPPORTED_CAMERAS.iter() {
             if camera.matches_descriptor(&descriptor) {
-                let camera = Camera {
+                let camera = (camera.factory)(device)?;
-                    camera_impl: camera.into(),
-                    rusb_device: device,
-                };
-
                 connected_cameras.push(camera);
                 break;
             }
@@ -79,7 +24,9 @@ pub fn get_connected_camers() -> Result<Vec<Camera>, Box<dyn Error + Send + Sync
     Ok(connected_cameras)
 }
 
-pub fn get_connected_camera_by_id(id: &str) -> Result<Camera, Box<dyn Error + Send + Sync>> {
+pub fn get_connected_camera_by_id(
+    id: &str,
+) -> Result<Box<dyn CameraImpl>, Box<dyn Error + Send + Sync>> {
     let parts: Vec<&str> = id.split('.').collect();
     if parts.len() != 2 {
         return Err(format!("Invalid device id format: {}", id).into());
@@ -94,10 +41,8 @@ pub fn get_connected_camera_by_id(id: &str) -> Result<Camera, Box<dyn Error + Se
 
             for camera in SUPPORTED_CAMERAS.iter() {
                 if camera.matches_descriptor(&descriptor) {
-                    return Ok(Camera {
+                    let camera = (camera.factory)(device)?;
-                        camera_impl: camera.into(),
+                    return Ok(camera);
-                        rusb_device: device,
-                    });
                 }
             }
 
@@ -108,7 +53,9 @@ pub fn get_connected_camera_by_id(id: &str) -> Result<Camera, Box<dyn Error + Se
     Err(format!("No device found with id: {}", id).into())
 }
 
-pub fn get_camera(device_id: Option<&str>) -> Result<Camera, Box<dyn Error + Send + Sync>> {
+pub fn get_camera(
+    device_id: Option<&str>,
+) -> Result<Box<dyn CameraImpl>, Box<dyn Error + Send + Sync>> {
     match device_id {
         Some(id) => get_connected_camera_by_id(id),
         None => get_connected_camers()?