Address PR feedback

Author: marshallpierce

rust/CHANGELOG.md

@@ -1,3 +1,10 @@
+# Next
+
+- GATT server support
+- Tests w/ rootcanal
+- Battery service example
+- Address is now pure Rust that's convertible to its Python equivalent rather than just holding a PyObject
+
 # 0.2.0
 
 - Code-gen company ID table

rust/examples/battery_service.rs

@@ -31,6 +31,20 @@
 //!     client \
 //!     --target-addr F0:F1:F2:F3:F4:F5
 //! ```
+//!
+//! Any combo will work, e.g. a Rust server and Python client:
+//!
+//! ```
+//! PYTHONPATH=..:/path/to/virtualenv/site-packages/ \
+//!     cargo run --example battery_service -- \
+//!     --transport android-netsim \
+//!     server
+//! ```
+//!
+//! ```
+//! PYTHONPATH=. python examples/battery_client.py \
+//!     android-netsim F0:F1:F2:F3:F4:F5
+//! ```
 
 use anyhow::anyhow;
 use async_trait::async_trait;
@@ -116,22 +130,22 @@ async fn run_client(device: Device, target_addr: Address) -> anyhow::Result<()>
 }
 
 async fn run_server(mut device: Device) -> anyhow::Result<()> {
-    let uuid = services::BATTERY.uuid();
+    let battery_service_uuid = services::BATTERY.uuid();
     let battery_level_uuid = Uuid16::from(0x2A19).into();
     let battery_level = Characteristic::new(
         battery_level_uuid,
         CharacteristicProperty::Read | CharacteristicProperty::Notify,
         AttributePermission::Readable.into(),
         CharacteristicValueHandler::new(Box::new(BatteryRead), Box::new(NoOpWrite)),
     );
-    let service = Service::new(uuid.into(), vec![battery_level]);
+    let service = Service::new(battery_service_uuid.into(), vec![battery_level]);
     let service_handle = device.add_service(&service)?;
 
     let mut builder = AdvertisementDataBuilder::new();
     builder.append(CommonDataType::CompleteLocalName, "Bumble Battery")?;
     builder.append(
         CommonDataType::IncompleteListOf16BitServiceClassUuids,
-        &uuid,
+        &battery_service_uuid,
     )?;
     builder.append(
         CommonDataType::Appearance,

rust/pytests/rootcanal.rs

@@ -271,11 +271,16 @@ async fn with_dir_lock<T>(
     closure: impl Future<Output = anyhow::Result<T>>,
 ) -> anyhow::Result<T> {
     // wait until we can create the dir
+    let mut printed_contention_msg = false;
     loop {
         match fs::create_dir(dir) {
             Ok(_) => break,
             Err(e) => {
                 if e.kind() == io::ErrorKind::AlreadyExists {
+                    if !printed_contention_msg {
+                        printed_contention_msg = true;
+                        eprintln!("Dir lock contention; sleeping");
+                    }
                     tokio::time::sleep(std::time::Duration::from_millis(50)).await;
                 } else {
                     warn!(

rust/src/cli/firmware/rtk.rs

@@ -16,6 +16,7 @@
 
 use crate::{Download, Source};
 use anyhow::anyhow;
+use bumble::project_dir;
 use bumble::wrapper::{
     drivers::rtk::{Driver, DriverInfo, Firmware},
     host::{DriverFactory, Host},
@@ -28,10 +29,7 @@ use std::{fs, path};
 pub(crate) async fn download(dl: Download) -> PyResult<()> {
     let data_dir = dl
         .output_dir
-        .or_else(|| {
-            directories::ProjectDirs::from("com", "google", "bumble")
-                .map(|pd| pd.data_local_dir().join("firmware").join("realtek"))
-        })
+        .or_else(|| project_dir().map(|pd| pd.data_local_dir().join("firmware").join("realtek")))
         .unwrap_or_else(|| {
             eprintln!("Could not determine standard data directory");
             path::PathBuf::from(".")

rust/src/internal/hci/mod.rs

@@ -12,9 +12,11 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+use itertools::Itertools;
 pub use pdl_runtime::{Error, Packet};
+use std::fmt;
 
-use crate::internal::hci::packets::{Acl, Command, Event, Sco};
+use crate::internal::hci::packets::{Acl, AddressType, Command, Event, Sco};
 use pdl_derive::pdl;
 
 #[allow(missing_docs, warnings, clippy::all)]
@@ -23,6 +25,110 @@ pub mod packets {}
 #[cfg(test)]
 mod tests;
 
+/// A Bluetooth address
+#[derive(Clone, Copy, PartialEq, Eq, Hash)]
+pub struct Address {
+    /// Little-endian bytes
+    le_bytes: [u8; 6],
+    address_type: AddressType,
+}
+
+impl Address {
+    /// Creates a new address with the provided little-endian bytes.
+    pub fn from_le_bytes(le_bytes: [u8; 6], address_type: AddressType) -> Self {
+        Self {
+            le_bytes,
+            address_type,
+        }
+    }
+
+    /// Creates a new address with the provided big endian hex (with or without `:` separators).
+    ///
+    /// # Examples
+    ///
+    /// ```
+    /// use bumble::{wrapper::{hci::{Address, packets::AddressType}}};
+    /// let hex = "F0:F1:F2:F3:F4:F5";
+    /// assert_eq!(
+    ///     hex,
+    ///     Address::from_be_hex(hex, AddressType::PublicDeviceAddress).unwrap().as_be_hex()
+    /// );
+    /// ```
+    pub fn from_be_hex(
+        address: &str,
+        address_type: AddressType,
+    ) -> Result<Self, InvalidAddressHex> {
+        let filtered: String = address.chars().filter(|c| *c != ':').collect();
+        let mut bytes: [u8; 6] = hex::decode(filtered)
+            .map_err(|_| InvalidAddressHex { address })?
+            .try_into()
+            .map_err(|_| InvalidAddressHex { address })?;
+        bytes.reverse();
+
+        Ok(Self {
+            le_bytes: bytes,
+            address_type,
+        })
+    }
+
+    /// The type of address
+    pub fn address_type(&self) -> AddressType {
+        self.address_type
+    }
+
+    /// True if the address is static
+    pub fn is_static(&self) -> bool {
+        !self.is_public() && self.le_bytes[5] >> 6 == 3
+    }
+
+    /// True if the address type is [AddressType::PublicIdentityAddress] or
+    /// [AddressType::PublicDeviceAddress]
+    pub fn is_public(&self) -> bool {
+        matches!(
+            self.address_type,
+            AddressType::PublicDeviceAddress | AddressType::PublicIdentityAddress
+        )
+    }
+
+    /// True if the address is resolvable
+    pub fn is_resolvable(&self) -> bool {
+        self.address_type == AddressType::RandomDeviceAddress && self.le_bytes[5] >> 6 == 1
+    }
+
+    /// Address bytes in _little-endian_ format
+    pub fn as_le_bytes(&self) -> [u8; 6] {
+        self.le_bytes
+    }
+
+    /// Address bytes as big-endian colon-separated hex
+    pub fn as_be_hex(&self) -> String {
+        self.le_bytes
+            .into_iter()
+            .rev()
+            .map(|byte| hex::encode_upper([byte]))
+            .join(":")
+    }
+}
+
+// show a more readable form than default Debug for a byte array
+impl fmt::Debug for Address {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        write!(
+            f,
+            "Address {{ address: {}, type: {:?} }}",
+            self.as_be_hex(),
+            self.address_type
+        )
+    }
+}
+
+/// Error type for [Address::from_be_hex].
+#[derive(Debug, thiserror::Error)]
+#[error("Invalid address hex: {address}")]
+pub struct InvalidAddressHex<'a> {
+    address: &'a str,
+}
+
 /// HCI Packet type, prepended to the packet.
 /// Rootcanal's PDL declaration excludes this from ser/deser and instead is implemented in code.
 /// To maintain the ability to easily use future versions of their packet PDL, packet type is

rust/src/lib.rs

@@ -30,3 +30,10 @@ pub mod wrapper;
 
 pub use internal::adv;
 pub(crate) mod internal;
+
+/// Directory for Bumble local storage for the current user according to the OS's conventions,
+/// if a convention is known for the current OS
+#[cfg(any(feature = "bumble-tools", test))]
+pub fn project_dir() -> Option<directories::ProjectDirs> {
+    directories::ProjectDirs::from("com", "google", "bumble")
+}

rust/src/wrapper/hci.rs

@@ -14,15 +14,13 @@
 
 //! HCI
 
-use std::fmt;
 // re-export here, and internal usages of these imports should refer to this mod, not the internal
 // mod
 pub(crate) use crate::internal::hci::WithPacketType;
-pub use crate::internal::hci::{packets, Error, Packet};
+pub use crate::internal::hci::{packets, Address, Error, InvalidAddressHex, Packet};
 
 use crate::wrapper::core::{TryFromPy, TryToPy};
 use crate::wrapper::hci::packets::{AddressType, Command, ErrorCode};
-use itertools::Itertools as _;
 use pyo3::types::PyBytes;
 use pyo3::{
     exceptions::PyException, intern, types::PyModule, FromPyObject, IntoPy, PyAny, PyErr, PyObject,
@@ -72,111 +70,11 @@ impl IntoPy<PyObject> for HciCommand {
     }
 }
 
-/// A Bluetooth address
-#[derive(Clone, Copy, PartialEq, Eq, Hash)]
-pub struct Address {
-    /// Little-endian bytes
-    le_bytes: [u8; 6],
-    address_type: AddressType,
-}
-
-impl Address {
-    /// Creates a new address with the provided little-endian bytes.
-    pub fn from_le_bytes(le_bytes: [u8; 6], address_type: AddressType) -> Self {
-        Self {
-            le_bytes,
-            address_type,
-        }
-    }
-
-    /// Creates a new address with the provided big endian hex (with or without `:` separators).
-    ///
-    /// # Examples
-    ///
-    /// ```
-    /// use bumble::{wrapper::{hci::{Address, packets::AddressType}}};
-    /// let hex = "F0:F1:F2:F3:F4:F5";
-    /// assert_eq!(
-    ///     hex,
-    ///     Address::from_be_hex(hex, AddressType::PublicDeviceAddress).unwrap().as_be_hex()
-    /// );
-    /// ```
-    pub fn from_be_hex(
-        address: &str,
-        address_type: AddressType,
-    ) -> Result<Self, InvalidAddressHex> {
-        let filtered: String = address.chars().filter(|c| *c != ':').collect();
-        let mut bytes: [u8; 6] = hex::decode(filtered)
-            .map_err(|_| InvalidAddressHex { address })?
-            .try_into()
-            .map_err(|_| InvalidAddressHex { address })?;
-        bytes.reverse();
-
-        Ok(Self {
-            le_bytes: bytes,
-            address_type,
-        })
-    }
-
-    /// The type of address
-    pub fn address_type(&self) -> AddressType {
-        self.address_type
-    }
-
-    /// True if the address is static
-    pub fn is_static(&self) -> bool {
-        !self.is_public() && self.le_bytes[5] >> 6 == 3
-    }
-
-    /// True if the address type is [AddressType::PublicIdentityAddress] or
-    /// [AddressType::PublicDeviceAddress]
-    pub fn is_public(&self) -> bool {
-        matches!(
-            self.address_type,
-            AddressType::PublicDeviceAddress | AddressType::PublicIdentityAddress
-        )
-    }
-
-    /// True if the address is resolvable
-    pub fn is_resolvable(&self) -> bool {
-        matches!(
-            self.address_type,
-            AddressType::PublicIdentityAddress | AddressType::RandomIdentityAddress
-        )
-    }
-
-    /// Address bytes in _little-endian_ format
-    pub fn as_le_bytes(&self) -> [u8; 6] {
-        self.le_bytes
-    }
-
-    /// Address bytes as big-endian colon-separated hex
-    pub fn as_be_hex(&self) -> String {
-        self.le_bytes
-            .into_iter()
-            .rev()
-            .map(|byte| hex::encode_upper([byte]))
-            .join(":")
-    }
-}
-
-// show a more readable form than default Debug for a byte array
-impl fmt::Debug for Address {
-    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        write!(
-            f,
-            "Address {{ address: {}, type: {:?} }}",
-            self.as_be_hex(),
-            self.address_type
-        )
-    }
-}
-
 impl TryToPy for Address {
     fn try_to_py<'py>(&self, py: Python<'py>) -> PyResult<&'py PyAny> {
         PyModule::import(py, intern!(py, "bumble.device"))?
             .getattr(intern!(py, "Address"))?
-            .call1((PyBytes::new(py, &self.le_bytes), self.address_type))
+            .call1((PyBytes::new(py, &self.as_le_bytes()), self.address_type()))
     }
 }
 
@@ -194,20 +92,10 @@ impl TryFromPy for Address {
 
         let address = obj.call_method0(intern!(py, "to_bytes"))?.extract()?;
 
-        Ok(Self {
-            le_bytes: address,
-            address_type,
-        })
+        Ok(Self::from_le_bytes(address, address_type))
     }
 }
 
-/// Error type for [Address::from_be_hex].
-#[derive(Debug, thiserror::Error)]
-#[error("Invalid address hex: {address}")]
-pub struct InvalidAddressHex<'a> {
-    address: &'a str,
-}
-
 /// An error meaning that the internal u64 value used to convert to [packets::Address] did not
 /// represent a valid BT address.
 #[derive(Debug, thiserror::Error)]

rust/src/wrapper/mod.rs

@@ -31,13 +31,12 @@ use pyo3::{
 };
 
 pub mod assigned_numbers;
+pub mod att;
 pub mod common;
 pub mod controller;
 pub mod core;
 pub mod device;
 pub mod drivers;
-
-pub mod att;
 pub mod gatt;
 pub mod hci;
 pub mod host;