Merge pull request #3703 from loftyinclination/main

add documentation for the BLE feature of the embassy-stm32-wpan crate

7 files changed, 106 insertions, 18 deletions

diff --git a/embassy-stm32-wpan/Cargo.toml b/embassy-stm32-wpan/Cargo.toml
index 552d80982..74e7a67d9 100644
--- a/embassy-stm32-wpan/Cargo.toml
+++ b/embassy-stm32-wpan/Cargo.toml
@@ -13,10 +13,10 @@ documentation = "https://docs.embassy.dev/embassy-stm32-wpan"
 src_base = "https://github.com/embassy-rs/embassy/blob/embassy-stm32-wpan-v$VERSION/embassy-stm32-wpan/src/"
 src_base_git = "https://github.com/embassy-rs/embassy/blob/$COMMIT/embassy-stm32-wpan/src/"
 target = "thumbv7em-none-eabihf"
-features = ["stm32wb55rg"]
+features = ["stm32wb55rg", "ble", "mac"]
 
 [package.metadata.docs.rs]
-features = ["stm32wb55rg"]
+features = ["stm32wb55rg", "ble", "mac"]
 
 [dependencies]
 embassy-stm32 = { version = "0.1.0", path = "../embassy-stm32" }
diff --git a/embassy-stm32-wpan/src/consts.rs b/embassy-stm32-wpan/src/consts.rs
index 6aaef1d35..e2ae6ca86 100644
--- a/embassy-stm32-wpan/src/consts.rs
+++ b/embassy-stm32-wpan/src/consts.rs
@@ -69,7 +69,7 @@ pub const TL_CS_EVT_SIZE: usize = core::mem::size_of::<CsEvt>();
  * enough to store all asynchronous events received in between.
  * When CFG_TLBLE_MOST_EVENT_PAYLOAD_SIZE is set to 27, this allow to store three 255 bytes long asynchronous events
  * between the HCI command and its event.
- * This parameter depends on the value given to CFG_TLBLE_MOST_EVENT_PAYLOAD_SIZE. When the queue size is to small,
+ * This parameter depends on the value given to CFG_TLBLE_MOST_EVENT_PAYLOAD_SIZE. When the queue size is too small,
  * the system may hang if the queue is full with asynchronous events and the HCI layer is still waiting
  * for a CC/CS event, In that case, the notification TL_BLE_HCI_ToNot() is called to indicate
  * to the application a HCI command did not receive its command event within 30s (Default HCI Timeout).
diff --git a/embassy-stm32-wpan/src/lib.rs b/embassy-stm32-wpan/src/lib.rs
index fb34d4ba0..25e6d965a 100644
--- a/embassy-stm32-wpan/src/lib.rs
+++ b/embassy-stm32-wpan/src/lib.rs
@@ -1,3 +1,16 @@
+//! The embassy-stm32-wpan crate aims to provide safe use of the commands necessary to interface
+//! with the Cortex C0 CPU2 coprocessor of STM32WB microcontrollers. It implements safe wrappers
+//! around the Transport Layer, and in particular the system, memory, BLE and Mac channels.
+//!
+//! # Design
+//!
+//! This crate loosely follows the Application Note 5289 "How to build wireless applications with
+//! STM32WB MCUs"; several of the startup procedures laid out in Annex 14.1 are implemented using
+//! inline copies of the code contained within the `stm32wb_copro` C library.
+//!
+//! BLE commands are implemented via use of the [stm32wb_hci] crate, for which the
+//! [stm32wb_hci::Controller] trait has been implemented.
+
 #![no_std]
 #![allow(async_fn_in_trait)]
 #![doc = include_str!("../README.md")]
@@ -37,6 +50,7 @@ pub use crate::sub::ble::hci;
 
 type PacketHeader = LinkedListNode;
 
+/// Transport Layer for the Mailbox interface
 pub struct TlMbox<'d> {
     _ipcc: PeripheralRef<'d, IPCC>,
 
@@ -49,6 +63,34 @@ pub struct TlMbox<'d> {
 }
 
 impl<'d> TlMbox<'d> {
+    /// Initialise the Transport Layer, and creates and returns a wrapper around it.
+    ///
+    /// This method performs the initialisation laid out in AN5289 annex 14.1. However, it differs
+    /// from the implementation documented in Figure 64, to avoid needing to reference any C
+    /// function pointers.
+    ///
+    /// Annex 14.1 lays out the following methods that should be called:
+    ///     1. tl_mbox.c/TL_Init, which initialises the reference table that is shared between CPU1
+    ///        and CPU2.
+    ///     2. shci_tl.c/shci_init(), which initialises the system transport layer, and in turn
+    ///        calls tl_mbox.c/TL_SYS_Init, which initialises SYSTEM_EVT_QUEUE channel.
+    ///     3. tl_mbox.c/TL_MM_Init(), which initialises the channel used for sending memory
+    ///        manager commands.
+    ///     4. tl_mbox.c/TL_Enable(), which enables the IPCC, and starts CPU2.
+    /// This implementation initialises all of the shared refernce tables and all IPCC channel that
+    /// would be initialised by this process. The developer should therefore treat this method as
+    /// completing all steps in Figure 64.
+    ///
+    /// Once this method has been called, no system commands may be sent until the CPU2 ready
+    /// signal is received, via [sys_subsystem.read]; this completes the procedure laid out in
+    /// Figure 65.
+    ///
+    /// If the `ble` feature is enabled, at this point, the user should call
+    /// [sys_subsystem.shci_c2_ble_init], before any commands are written to the
+    /// [TlMbox.ble_subsystem] ([sub::ble::Ble::new()] completes the process that would otherwise
+    /// be handled by `TL_BLE_Init`; see Figure 66). This completes the procedure laid out in
+    /// Figure 66.
+    // TODO: document what the user should do after calling init to use the mac_802_15_4 subsystem
     pub fn init(
         ipcc: impl Peripheral<P = IPCC> + 'd,
         _irqs: impl interrupt::typelevel::Binding<interrupt::typelevel::IPCC_C1_RX, ReceiveInterruptHandler>
@@ -57,6 +99,9 @@ impl<'d> TlMbox<'d> {
     ) -> Self {
         into_ref!(ipcc);
 
+        // this is an inlined version of TL_Init from the STM32WB firmware as requested by AN5289.
+        // HW_IPCC_Init is not called, and its purpose is (presumably?) covered by this
+        // implementation
         unsafe {
             TL_REF_TABLE.as_mut_ptr().write_volatile(RefTable {
                 device_info_table: TL_DEVICE_INFO_TABLE.as_ptr(),
@@ -140,6 +185,7 @@ impl<'d> TlMbox<'d> {
 
         compiler_fence(Ordering::SeqCst);
 
+        // this is equivalent to `HW_IPCC_Enable`, which is called by `TL_Enable`
         Ipcc::enable(config);
 
         Self {
diff --git a/embassy-stm32-wpan/src/sub/ble.rs b/embassy-stm32-wpan/src/sub/ble.rs
index c5f2334f4..0f770d92c 100644
--- a/embassy-stm32-wpan/src/sub/ble.rs
+++ b/embassy-stm32-wpan/src/sub/ble.rs
@@ -11,11 +11,39 @@ use crate::tables::{BleTable, BLE_CMD_BUFFER, CS_BUFFER, EVT_QUEUE, HCI_ACL_DATA
 use crate::unsafe_linked_list::LinkedListNode;
 use crate::{channels, evt};
 
+/// A guard that, once constructed, may be used to send BLE commands to CPU2.
+///
+/// It is the responsibility of the caller to ensure that they have awaited an event via
+/// [crate::sub::sys::Sys::read] before sending any of these commands, and to call
+/// [crate::sub::sys::Sys::shci_c2_ble_init] and await the HCI_COMMAND_COMPLETE_EVENT before
+/// sending any other commands.
+///
+/// # Example
+///
+/// ```
+/// # embassy_stm32::bind_interrupts!(struct Irqs{
+/// #     IPCC_C1_RX => ReceiveInterruptHandler;
+/// #     IPCC_C1_TX => TransmitInterruptHandler;
+/// # });
+/// #
+/// # let p = embassy_stm32::init(embassy_stm32::Config::default());
+/// # let mut mbox = embassy_stm32_wpan::TlMbox::init(p.IPCC, Irqs, embassy_stm32::ipcc::Config::default());
+/// #
+/// # let sys_event = mbox.sys_subsystem.read().await;
+/// # let _command_status = mbox.sys_subsystem.shci_c2_ble_init(Default::default());
+/// # // BLE commands may now be sent
+/// #
+/// # mbox.ble_subsystem.reset().await;
+/// # let _reset_response = mbox.ble_subsystem.read().await;
+/// ```
 pub struct Ble {
     _private: (),
 }
 
 impl Ble {
+    /// Constructs a guard that allows for BLE commands to be sent to CPU2.
+    ///
+    /// This takes the place of `TL_BLE_Init`, completing that step as laid out in AN5289, Fig 66.
     pub(crate) fn new() -> Self {
         unsafe {
             LinkedListNode::init_head(EVT_QUEUE.as_mut_ptr());
@@ -30,6 +58,7 @@ impl Ble {
 
         Self { _private: () }
     }
+
     /// `HW_IPCC_BLE_EvtNot`
     pub async fn tl_read(&self) -> EvtBox<Self> {
         Ipcc::receive(channels::cpu2::IPCC_BLE_EVENT_CHANNEL, || unsafe {
diff --git a/embassy-stm32-wpan/src/sub/sys.rs b/embassy-stm32-wpan/src/sub/sys.rs
index bd2ea3f74..cf6df58bf 100644
--- a/embassy-stm32-wpan/src/sub/sys.rs
+++ b/embassy-stm32-wpan/src/sub/sys.rs
@@ -10,6 +10,7 @@ use crate::tables::{SysTable, WirelessFwInfoTable};
 use crate::unsafe_linked_list::LinkedListNode;
 use crate::{channels, Ipcc, SYSTEM_EVT_QUEUE, SYS_CMD_BUF, TL_DEVICE_INFO_TABLE, TL_SYS_TABLE};
 
+/// A guard that, once constructed, allows for sys commands to be sent to CPU2.
 pub struct Sys {
     _private: (),
 }
@@ -86,12 +87,22 @@ impl Sys {
         self.write_and_get_response(ShciOpcode::Mac802_15_4Init, &[]).await
     }
 
+    /// Send a request to CPU2 to initialise the BLE stack.
+    ///
+    /// This must be called before any BLE commands are sent via the BLE channel (according to
+    /// AN5289, Figures 65 and 66). It should only be called after CPU2 sends a system event, via
+    /// `HW_IPCC_SYS_EvtNot`, aka `IoBusCallBackUserEvt` (as detailed in Figure 65), aka
+    /// [crate::sub::ble::hci::host::uart::UartHci::read].
     #[cfg(feature = "ble")]
     pub async fn shci_c2_ble_init(&self, param: ShciBleInitCmdParam) -> Result<SchiCommandStatus, ()> {
         self.write_and_get_response(ShciOpcode::BleInit, param.payload()).await
     }
 
     /// `HW_IPCC_SYS_EvtNot`
+    ///
+    /// This method takes the place of the `HW_IPCC_SYS_EvtNot`/`SysUserEvtRx`/`APPE_SysUserEvtRx`,
+    /// as the embassy implementation avoids the need to call C public bindings, and instead
+    /// handles the event channels directly.
     pub async fn read(&self) -> EvtBox<mm::MemoryManager> {
         Ipcc::receive(channels::cpu2::IPCC_SYSTEM_EVENT_CHANNEL, || unsafe {
             if let Some(node_ptr) = LinkedListNode::remove_head(SYSTEM_EVT_QUEUE.as_mut_ptr()) {
diff --git a/embassy-stm32-wpan/src/tables.rs b/embassy-stm32-wpan/src/tables.rs
index f2c250527..fe6fc47a3 100644
--- a/embassy-stm32-wpan/src/tables.rs
+++ b/embassy-stm32-wpan/src/tables.rs
@@ -25,17 +25,17 @@ pub struct RssInfoTable {
 
 /**
  * Version
- * [0:3]   = Build - 0: Untracked - 15:Released - x: Tracked version
- * [4:7]   = branch - 0: Mass Market - x: ...
- * [8:15]  = Subversion
- * [16:23] = Version minor
- * [24:31] = Version major
+ * \[0:3\]   = Build - 0: Untracked - 15:Released - x: Tracked version
+ * \[4:7\]   = branch - 0: Mass Market - x: ...
+ * \[8:15\]  = Subversion
+ * \[16:23\] = Version minor
+ * \[24:31\] = Version major
  *
  * Memory Size
- * [0:7]   = Flash ( Number of 4k sector)
- * [8:15]  = Reserved ( Shall be set to 0 - may be used as flash extension )
- * [16:23] = SRAM2b ( Number of 1k sector)
- * [24:31] = SRAM2a ( Number of 1k sector)
+ * \[0:7\]   = Flash ( Number of 4k sector)
+ * \[8:15\]  = Reserved ( Shall be set to 0 - may be used as flash extension )
+ * \[16:23\] = SRAM2b ( Number of 1k sector)
+ * \[24:31\] = SRAM2a ( Number of 1k sector)
  */
 #[derive(Debug, Copy, Clone)]
 #[repr(C, packed)]
@@ -89,12 +89,19 @@ pub struct DeviceInfoTable {
     pub wireless_fw_info_table: WirelessFwInfoTable,
 }
 
+/// The bluetooth reference table, as defined in figure 67 of STM32WX AN5289.
 #[derive(Debug)]
 #[repr(C)]
 pub struct BleTable {
+    /// A pointer to the buffer that is used for sending BLE commands.
     pub pcmd_buffer: *mut CmdPacket,
+    /// A pointer to the buffer used for storing Command statuses.
     pub pcs_buffer: *const u8,
+    /// A pointer to the event queue, over which IPCC BLE events are sent. This may be accessed via
+    /// [crate::sub::ble::Ble::tl_read].
     pub pevt_queue: *const u8,
+    /// A pointer to the buffer that is used for sending HCI (Host-Controller Interface) ACL
+    /// (Asynchronous Connection-oriented Logical transport) commands (unused).
     pub phci_acl_data_buffer: *mut AclDataPacket,
 }
 
@@ -271,6 +278,6 @@ pub static mut BLE_SPARE_EVT_BUF: Aligned<A4, MaybeUninit<[u8; TL_PACKET_HEADER_
 
 #[cfg(feature = "ble")]
 #[link_section = "MB_MEM2"]
-//                                 fuck these "magic" numbers from ST ---v---v
+//                                                 fuck these "magic" numbers from ST ---v---v
 pub static mut HCI_ACL_DATA_BUFFER: Aligned<A4, MaybeUninit<[u8; TL_PACKET_HEADER_SIZE + 5 + 251]>> =
     Aligned(MaybeUninit::uninit());
diff --git a/examples/stm32wb/src/bin/gatt_server.rs b/examples/stm32wb/src/bin/gatt_server.rs
index 1cc50e134..041dc0cf5 100644
--- a/examples/stm32wb/src/bin/gatt_server.rs
+++ b/examples/stm32wb/src/bin/gatt_server.rs
@@ -151,11 +151,6 @@ async fn main(_spawner: Spawner) {
     let response = mbox.ble_subsystem.read().await;
     defmt::debug!("{}", response);
 
-    info!("set scan response data...");
-    mbox.ble_subsystem.le_set_scan_response_data(b"TXTX").await.unwrap();
-    let response = mbox.ble_subsystem.read().await;
-    defmt::debug!("{}", response);
-
     defmt::info!("initializing services and characteristics...");
     let mut ble_context = init_gatt_services(&mut mbox.ble_subsystem).await.unwrap();
     defmt::info!("{}", ble_context);