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;
@@ -53,6 +67,8 @@ use owo_colors::OwoColorize;
 use pyo3::prelude::*;
 use rand::Rng;
 use std::time::Duration;
+use tokio::select;
+use tokio_util::sync::CancellationToken;
 
 #[pyo3_asyncio::tokio::main]
 async fn main() -> PyResult<()> {
@@ -116,22 +132,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,
@@ -146,10 +162,28 @@ async fn run_server(mut device: Device) -> anyhow::Result<()> {
         .characteristic_handle(battery_level_uuid)
         .expect("Battery level should be present");
 
+    let cancellation_token = CancellationToken::new();
+
+    let ct = cancellation_token.clone();
+    tokio::spawn(async move {
+        let _ = tokio::signal::ctrl_c().await;
+        println!("Ctrl-C caught");
+        ct.cancel();
+    });
+
     loop {
-        tokio::time::sleep(Duration::from_secs(3)).await;
+        select! {
+            _ = tokio::time::sleep(Duration::from_secs(3)) => {}
+            _ = cancellation_token.cancelled() => { break }
+        }
+
         device.notify_subscribers(char_handle).await?;
     }
+
+    println!("Stopping advertising");
+    device.stop_advertising().await?;
+
+    Ok(())
 }
 
 struct BatteryRead;

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")
+}