Split out ProbeDriver (probe-rs#1996)

winksaville · Jan 4, 2024 · 852d92a · 852d92a
1 parent 342ec2c
commit 852d92a
Showing 19 changed files with 374 additions and 372 deletions.
diff --git a/changelog/changed-debug-probe-traits.md b/changelog/changed-debug-probe-traits.md
@@ -0,0 +1 @@
+Extract ProbeDriver from DebugProbe
diff --git a/changelog/removed-debugprobetype.md b/changelog/removed-debugprobetype.md
@@ -0,0 +1 @@
+Removed `DebugProbeType`
diff --git a/probe-rs/src/bin/probe-rs/cmd/dap_server/server/debugger.rs b/probe-rs/src/bin/probe-rs/cmd/dap_server/server/debugger.rs
@@ -916,30 +916,49 @@ mod test {
     use std::path::PathBuf;
 
     use probe_rs::architecture::arm::ApAddress;
-    use probe_rs::DebugProbeInfo;
     use probe_rs::{
         integration::{FakeProbe, Operation},
         Lister,
     };
+    use probe_rs::{DebugProbeInfo, ProbeDriver};
     use serde_json::json;
     use time::UtcOffset;
 
-    use crate::cmd::dap_server::debug_adapter::dap::dap_types::{
-        DisconnectArguments, ErrorResponseBody, Message, Response, Thread, ThreadsResponseBody,
-    };
-    use crate::cmd::dap_server::server::configuration::{ConsoleLog, CoreConfig, FlashingConfig};
     use crate::cmd::dap_server::{
         debug_adapter::{
             dap::{
                 adapter::DebugAdapter,
-                dap_types::{Capabilities, InitializeRequestArguments, Request},
+                dap_types::{
+                    Capabilities, DisconnectArguments, ErrorResponseBody,
+                    InitializeRequestArguments, Message, Request, Response, Thread,
+                    ThreadsResponseBody,
+                },
             },
             protocol::ProtocolAdapter,
         },
-        server::{configuration::SessionConfig, debugger::DebugSessionStatus},
+        server::{
+            configuration::{ConsoleLog, CoreConfig, FlashingConfig, SessionConfig},
+            debugger::DebugSessionStatus,
+        },
         test::TestLister,
     };
 
+    #[derive(Debug)]
+    struct MockProbeSource;
+
+    impl ProbeDriver for MockProbeSource {
+        fn open(
+            &self,
+            _selector: &probe_rs::DebugProbeSelector,
+        ) -> Result<Box<dyn probe_rs::DebugProbe>, probe_rs::DebugProbeError> {
+            todo!()
+        }
+
+        fn list_probes(&self) -> Vec<DebugProbeInfo> {
+            todo!()
+        }
+    }
+
     /// Helper function to get the expected capabilities for the debugger
     ///
     /// `Capabilities::default()` is not const, so this can't just be a constant.
@@ -1344,7 +1363,7 @@ mod test {
             0x12,
             0x23,
             Some("mock_serial".to_owned()),
-            probe_rs::DebugProbeType::CmsisDap,
+            &MockProbeSource,
             None,
         );
 
@@ -1422,7 +1441,7 @@ mod test {
             0x12,
             0x23,
             Some("mock_serial".to_owned()),
-            probe_rs::DebugProbeType::CmsisDap,
+            &MockProbeSource,
             None,
         );
 
@@ -1478,7 +1497,7 @@ mod test {
             0x12,
             0x23,
             Some("mock_serial".to_owned()),
-            probe_rs::DebugProbeType::CmsisDap,
+            &MockProbeSource,
             None,
         );
 
@@ -1552,7 +1571,7 @@ mod test {
             0x12,
             0x23,
             Some("mock_serial".to_owned()),
-            probe_rs::DebugProbeType::CmsisDap,
+            &MockProbeSource,
             None,
         );
 
@@ -1626,7 +1645,7 @@ mod test {
             0x12,
             0x23,
             Some("mock_serial".to_owned()),
-            probe_rs::DebugProbeType::CmsisDap,
+            &MockProbeSource,
             None,
         );
 
@@ -1718,7 +1737,7 @@ mod test {
             0x12,
             0x23,
             Some("mock_serial".to_owned()),
-            probe_rs::DebugProbeType::CmsisDap,
+            &MockProbeSource,
             None,
         );
 

diff --git a/probe-rs/src/lib.rs b/probe-rs/src/lib.rs
@@ -104,6 +104,6 @@ pub use crate::error::Error;
 pub use crate::memory::MemoryInterface;
 pub use crate::probe::{
     list::Lister, AttachMethod, DebugProbe, DebugProbeError, DebugProbeInfo, DebugProbeSelector,
-    DebugProbeType, Probe, ProbeCreationError, WireProtocol,
+    Probe, ProbeCreationError, ProbeDriver, WireProtocol,
 };
 pub use crate::session::{Permissions, Session};
diff --git a/probe-rs/src/probe.rs b/probe-rs/src/probe.rs
@@ -482,19 +482,23 @@ impl Probe {
     }
 }
 
-/// An abstraction over general debug probe functionality.
+/// An abstraction over a probe driver type.
 ///
 /// This trait has to be implemented by ever debug probe driver.
-pub trait DebugProbe: Send + fmt::Debug {
+pub trait ProbeDriver: std::any::Any + std::fmt::Debug {
     /// Creates a new boxed [`DebugProbe`] from a given [`DebugProbeSelector`].
     /// This will be called for all available debug drivers when discovering probes.
     /// When opening, it will open the first probe which succeeds during this call.
-    fn new_from_selector(
-        selector: impl Into<DebugProbeSelector>,
-    ) -> Result<Box<Self>, DebugProbeError>
-    where
-        Self: Sized;
+    fn open(&self, selector: &DebugProbeSelector) -> Result<Box<dyn DebugProbe>, DebugProbeError>;
+
+    /// Returns a list of all available debug probes of the current type.
+    fn list_probes(&self) -> Vec<DebugProbeInfo>;
+}
 
+/// An abstraction over general debug probe.
+///
+/// This trait has to be implemented by ever debug probe driver.
+pub trait DebugProbe: Send + fmt::Debug {
     /// Get human readable name for the probe.
     fn get_name(&self) -> &str;
 
@@ -644,25 +648,21 @@ pub trait DebugProbe: Send + fmt::Debug {
     }
 }
 
-/// Denotes the type of a given [`DebugProbe`].
-#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Hash)]
-pub enum DebugProbeType {
-    /// CMSIS-DAP
-    CmsisDap,
-    /// FTDI based debug probe
-    Ftdi,
-    /// ST-Link
-    StLink,
-    /// J-Link
-    JLink,
-    /// Built in RISC-V ESP JTAG debug probe
-    EspJtag,
-    /// WCH-Link
-    WchLink,
+impl PartialEq for dyn ProbeDriver {
+    fn eq(&self, other: &Self) -> bool {
+        // Consider ProbeDriver objects equal when their types and data pointers are equal.
+        // Pointer equality is insufficient, because ZST objects may have the same dangling pointer
+        // as their address.
+        self.type_id() == other.type_id()
+            && std::ptr::eq(
+                self as *const _ as *const (),
+                other as *const _ as *const (),
+            )
+    }
 }
 
 /// Gathers some information about a debug probe which was found during a scan.
-#[derive(Clone, PartialEq, Eq)]
+#[derive(Clone, PartialEq)]
 pub struct DebugProbeInfo {
     /// The name of the debug probe.
     pub identifier: String,
@@ -673,7 +673,7 @@ pub struct DebugProbeInfo {
     /// The serial number of the debug probe.
     pub serial_number: Option<String>,
     /// The probe type of the debug probe.
-    pub probe_type: DebugProbeType,
+    pub probe_type: &'static dyn ProbeDriver,
 
     /// The USB HID interface which should be used.
     /// This is necessary for composite HID devices.
@@ -689,7 +689,7 @@ impl std::fmt::Debug for DebugProbeInfo {
             self.vendor_id,
             self.product_id,
             self.serial_number
-                .clone()
+                .as_ref()
                 .map_or("".to_owned(), |v| format!("Serial: {v}, ")),
             self.probe_type
         )
@@ -703,16 +703,16 @@ impl DebugProbeInfo {
         vendor_id: u16,
         product_id: u16,
         serial_number: Option<String>,
-        probe_type: DebugProbeType,
-        usb_hid_interface: Option<u8>,
+        probe_type: &'static dyn ProbeDriver,
+        hid_interface: Option<u8>,
     ) -> Self {
         Self {
             identifier: identifier.into(),
             vendor_id,
             product_id,
             serial_number,
             probe_type,
-            hid_interface: usb_hid_interface,
+            hid_interface,
         }
     }
 

diff --git a/probe-rs/src/probe/arm_jtag.rs b/probe-rs/src/probe/arm_jtag.rs
@@ -1671,15 +1671,6 @@ mod test {
     /// This is just a blanket impl that will crash if used (only relevant in tests,
     /// so no problem as we do not use it) to fulfill the marker requirement.
     impl DebugProbe for MockJaylink {
-        fn new_from_selector(
-            _selector: impl Into<crate::DebugProbeSelector>,
-        ) -> Result<Box<Self>, crate::DebugProbeError>
-        where
-            Self: Sized,
-        {
-            todo!()
-        }
-
         fn get_name(&self) -> &str {
             todo!()
         }

diff --git a/probe-rs/src/probe/cmsisdap/mod.rs b/probe-rs/src/probe/cmsisdap/mod.rs
@@ -15,7 +15,7 @@ use crate::{
             general::info::{CapabilitiesCommand, PacketCountCommand, SWOTraceBufferSizeCommand},
             CmsisDapError,
         },
-        BatchCommand,
+        BatchCommand, ProbeDriver,
     },
     CoreStatus, DebugProbe, DebugProbeError, DebugProbeSelector, WireProtocol,
 };
@@ -59,6 +59,26 @@ use bitvec::prelude::*;
 
 use super::common::{extract_idcodes, extract_ir_lengths, ScanChainError};
 
+pub struct CmsisDapSource;
+
+impl std::fmt::Debug for CmsisDapSource {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        f.debug_struct("CmsisDap").finish()
+    }
+}
+
+impl ProbeDriver for CmsisDapSource {
+    fn open(&self, selector: &DebugProbeSelector) -> Result<Box<dyn DebugProbe>, DebugProbeError> {
+        Ok(Box::new(CmsisDap::new_from_device(
+            tools::open_device_from_selector(selector)?,
+        )?))
+    }
+
+    fn list_probes(&self) -> Vec<crate::DebugProbeInfo> {
+        tools::list_cmsisdap_devices()
+    }
+}
+
 pub struct CmsisDap {
     pub device: CmsisDapDevice,
     _hw_version: u8,
@@ -700,17 +720,6 @@ impl CmsisDap {
 }
 
 impl DebugProbe for CmsisDap {
-    fn new_from_selector(
-        selector: impl Into<DebugProbeSelector>,
-    ) -> Result<Box<Self>, DebugProbeError>
-    where
-        Self: Sized,
-    {
-        Ok(Box::new(Self::new_from_device(
-            tools::open_device_from_selector(selector)?,
-        )?))
-    }
-
     fn get_name(&self) -> &str {
         "CMSIS-DAP"
     }

diff --git a/probe-rs/src/probe/cmsisdap/tools.rs b/probe-rs/src/probe/cmsisdap/tools.rs
@@ -1,6 +1,6 @@
 use super::CmsisDapDevice;
 use crate::{
-    probe::{DebugProbeInfo, DebugProbeType, ProbeCreationError},
+    probe::{cmsisdap::CmsisDapSource, DebugProbeInfo, ProbeCreationError},
     DebugProbeSelector,
 };
 use hidapi::HidApi;
@@ -116,7 +116,7 @@ fn get_cmsisdap_info(device: &Device<rusb::Context>) -> Option<DebugProbeInfo> {
             d_desc.vendor_id(),
             d_desc.product_id(),
             sn_str,
-            DebugProbeType::CmsisDap,
+            &CmsisDapSource,
             hid_interface,
         ))
     } else {
@@ -141,7 +141,7 @@ fn get_cmsisdap_hid_info(device: &hidapi::DeviceInfo) -> Option<DebugProbeInfo>
             device.vendor_id(),
             device.product_id(),
             device.serial_number().map(|s| s.to_owned()),
-            DebugProbeType::CmsisDap,
+            &CmsisDapSource,
             Some(device.interface_number() as u8),
         ))
     } else {
@@ -254,10 +254,8 @@ fn device_matches(
 /// Attempt to open the given DebugProbeInfo in CMSIS-DAP v2 mode if possible,
 /// otherwise in v1 mode.
 pub fn open_device_from_selector(
-    selector: impl Into<DebugProbeSelector>,
+    selector: &DebugProbeSelector,
 ) -> Result<CmsisDapDevice, ProbeCreationError> {
-    let selector = selector.into();
-
     tracing::trace!("Attempting to open device matching {}", selector);
 
     // We need to use rusb to detect the proper HID interface to use
@@ -309,7 +307,7 @@ pub fn open_device_from_selector(
             // multiple open handles are not allowed on Windows.
             drop(handle);
 
-            if device_matches(d_desc, &selector, sn_str) {
+            if device_matches(d_desc, selector, sn_str) {
                 hid_device_info = get_cmsisdap_info(&device);
 
                 if hid_device_info.is_some() {
@@ -329,7 +327,7 @@ pub fn open_device_from_selector(
     // If rusb failed or the device didn't support v2, try using hidapi to open in v1 mode.
     let vid = selector.vendor_id;
     let pid = selector.product_id;
-    let sn = &selector.serial_number;
+    let sn = selector.serial_number.as_deref();
 
     tracing::debug!(
         "Attempting to open {:04x}:{:04x} in CMSIS-DAP v1 mode",
@@ -351,7 +349,7 @@ pub fn open_device_from_selector(
             let mut device_match = info.vendor_id() == vid && info.product_id() == pid;
 
             if let Some(sn) = sn {
-                device_match &= Some(sn.as_ref()) == info.serial_number();
+                device_match &= Some(sn) == info.serial_number();
             }
 
             if let Some(hid_interface) =