diff --git a/embassy/src/blocking_mutex/mod.rs b/embassy/src/blocking_mutex/mod.rs index 602ec8a0..c9db89f0 100644 --- a/embassy/src/blocking_mutex/mod.rs +++ b/embassy/src/blocking_mutex/mod.rs @@ -160,11 +160,20 @@ mod thread_mode_mutex { } impl ThreadModeMutex { + /// Lock the `ThreadModeMutex`, granting access to the data. + /// + /// # Panics + /// + /// This will panic if not currently running in thread mode. pub fn lock(&self, f: impl FnOnce(&T) -> R) -> R { f(self.borrow()) } /// Borrows the data + /// + /// # Panics + /// + /// This will panic if not currently running in thread mode. pub fn borrow(&self) -> &T { assert!( raw::in_thread_mode(), diff --git a/embassy/src/blocking_mutex/raw.rs b/embassy/src/blocking_mutex/raw.rs index bdb443e4..669a23e3 100644 --- a/embassy/src/blocking_mutex/raw.rs +++ b/embassy/src/blocking_mutex/raw.rs @@ -3,12 +3,32 @@ //! This module provides a trait for mutexes that can be used in different contexts. use core::marker::PhantomData; -/// Any object implementing this trait guarantees exclusive access to the data contained -/// within the mutex for the duration of the lock. -/// Adapted from . -pub trait RawMutex { +/// Raw mutex trait. +/// +/// This mutex is "raw", which means it does not actually contain the protected data, it +/// just implements the mutex mechanism. For most uses you should use [`super::Mutex`] instead, +/// which is generic over a RawMutex and contains the protected data. +/// +/// Note that, unlike other mutexes, implementations only guarantee no +/// concurrent access from other threads: concurrent access from the current +/// thread is allwed. For example, it's possible to lock the same mutex multiple times reentrantly. +/// +/// Therefore, locking a `RawMutex` is only enough to guarantee safe shared (`&`) access +/// to the data, it is not enough to guarantee exclusive (`&mut`) access. +/// +/// # Safety +/// +/// RawMutex implementations must ensure that, while locked, no other thread can lock +/// the RawMutex concurrently. +/// +/// Unsafe code is allowed to rely on this fact, so incorrect implementations will cause undefined behavior. +pub unsafe trait RawMutex { + /// Create a new `RawMutex` instance. + /// + /// This is a const instead of a method to allow creating instances in const context. const INIT: Self; + /// Lock this `RawMutex`. fn lock(&self, f: impl FnOnce() -> R) -> R; } @@ -24,12 +44,13 @@ unsafe impl Send for CriticalSectionRawMutex {} unsafe impl Sync for CriticalSectionRawMutex {} impl CriticalSectionRawMutex { + /// Create a new `CriticalSectionRawMutex`. pub const fn new() -> Self { Self { _phantom: PhantomData } } } -impl RawMutex for CriticalSectionRawMutex { +unsafe impl RawMutex for CriticalSectionRawMutex { const INIT: Self = Self::new(); fn lock(&self, f: impl FnOnce() -> R) -> R { @@ -51,12 +72,13 @@ pub struct NoopRawMutex { unsafe impl Send for NoopRawMutex {} impl NoopRawMutex { + /// Create a new `NoopRawMutex`. pub const fn new() -> Self { Self { _phantom: PhantomData } } } -impl RawMutex for NoopRawMutex { +unsafe impl RawMutex for NoopRawMutex { const INIT: Self = Self::new(); fn lock(&self, f: impl FnOnce() -> R) -> R { f() @@ -84,12 +106,13 @@ mod thread_mode { unsafe impl Sync for ThreadModeRawMutex {} impl ThreadModeRawMutex { + /// Create a new `ThreadModeRawMutex`. pub const fn new() -> Self { Self { _phantom: PhantomData } } } - impl RawMutex for ThreadModeRawMutex { + unsafe impl RawMutex for ThreadModeRawMutex { const INIT: Self = Self::new(); fn lock(&self, f: impl FnOnce() -> R) -> R { assert!(in_thread_mode(), "ThreadModeMutex can only be locked from thread mode."); diff --git a/embassy/src/channel/mpmc.rs b/embassy/src/channel/mpmc.rs index 1d03eef1..ca2214bb 100644 --- a/embassy/src/channel/mpmc.rs +++ b/embassy/src/channel/mpmc.rs @@ -180,6 +180,7 @@ where } } +/// Future returned by [`Channel::recv`] and [`Receiver::recv`]. pub struct RecvFuture<'ch, M, T, const N: usize> where M: RawMutex, @@ -201,6 +202,7 @@ where } } +/// Future returned by [`DynamicReceiver::recv`]. pub struct DynamicRecvFuture<'ch, T> { channel: &'ch dyn DynamicChannel, } @@ -216,6 +218,7 @@ impl<'ch, T> Future for DynamicRecvFuture<'ch, T> { } } +/// Future returned by [`Channel::send`] and [`Sender::send`]. pub struct SendFuture<'ch, M, T, const N: usize> where M: RawMutex, @@ -246,6 +249,7 @@ where impl<'ch, M, T, const N: usize> Unpin for SendFuture<'ch, M, T, N> where M: RawMutex {} +/// Future returned by [`DynamicSender::send`]. pub struct DynamicSendFuture<'ch, T> { channel: &'ch dyn DynamicChannel, message: Option, diff --git a/embassy/src/channel/pubsub/mod.rs b/embassy/src/channel/pubsub/mod.rs index 11c88936..64a72a52 100644 --- a/embassy/src/channel/pubsub/mod.rs +++ b/embassy/src/channel/pubsub/mod.rs @@ -104,7 +104,7 @@ impl(&'a self) -> Result, Error> { + pub fn dyn_subscriber(&self) -> Result, Error> { self.inner.lock(|inner| { let mut s = inner.borrow_mut(); @@ -136,7 +136,7 @@ impl(&'a self) -> Result, Error> { + pub fn dyn_publisher(&self) -> Result, Error> { self.inner.lock(|inner| { let mut s = inner.borrow_mut(); @@ -369,7 +369,7 @@ impl PubSubSta } /// Error type for the [PubSubChannel] -#[derive(Debug, PartialEq, Clone)] +#[derive(Debug, PartialEq, Eq, Clone)] #[cfg_attr(feature = "defmt", derive(defmt::Format))] pub enum Error { /// All subscriber slots are used. To add another subscriber, first another subscriber must be dropped or @@ -404,7 +404,7 @@ pub trait PubSubBehavior { } /// The result of the subscriber wait procedure -#[derive(Debug, Clone, PartialEq)] +#[derive(Debug, Clone, PartialEq, Eq)] #[cfg_attr(feature = "defmt", derive(defmt::Format))] pub enum WaitResult { /// The subscriber did not receive all messages and lagged by the given amount of messages. diff --git a/embassy/src/channel/signal.rs b/embassy/src/channel/signal.rs index cf78dad8..3f3c482f 100644 --- a/embassy/src/channel/signal.rs +++ b/embassy/src/channel/signal.rs @@ -4,11 +4,19 @@ use core::future::Future; use core::mem; use core::task::{Context, Poll, Waker}; -/// Synchronization primitive. Allows creating awaitable signals that may be passed between tasks. -/// For a simple use-case where the receiver is only ever interested in the latest value of -/// something, Signals work well. For more advanced use cases, you might want to use [`Channel`](crate::channel::mpmc::Channel) instead.. +/// Single-slot signaling primitive. /// -/// Signals are generally declared as being a static const and then borrowed as required. +/// This is similar to a [`Channel`](crate::channel::mpmc::Channel) with a buffer size of 1, except +/// "sending" to it (calling [`Signal::signal`]) when full will overwrite the previous value instead +/// of waiting for the receiver to pop the previous value. +/// +/// It is useful for sending data between tasks when the receiver only cares about +/// the latest data, and therefore it's fine to "lose" messages. This is often the case for "state" +/// updates. +/// +/// For more advanced use cases, you might want to use [`Channel`](crate::channel::mpmc::Channel) instead. +/// +/// Signals are generally declared as `static`s and then borrowed as required. /// /// ``` /// use embassy::channel::signal::Signal; @@ -34,6 +42,7 @@ unsafe impl Send for Signal {} unsafe impl Sync for Signal {} impl Signal { + /// Create a new `Signal`. pub const fn new() -> Self { Self { state: UnsafeCell::new(State::None), @@ -42,7 +51,7 @@ impl Signal { } impl Signal { - /// Mark this Signal as completed. + /// Mark this Signal as signaled. pub fn signal(&self, val: T) { critical_section::with(|_| unsafe { let state = &mut *self.state.get(); @@ -52,6 +61,7 @@ impl Signal { }) } + /// Remove the queued value in this `Signal`, if any. pub fn reset(&self) { critical_section::with(|_| unsafe { let state = &mut *self.state.get(); @@ -59,7 +69,7 @@ impl Signal { }) } - pub fn poll_wait(&self, cx: &mut Context<'_>) -> Poll { + fn poll_wait(&self, cx: &mut Context<'_>) -> Poll { critical_section::with(|_| unsafe { let state = &mut *self.state.get(); match state { diff --git a/embassy/src/executor/raw/mod.rs b/embassy/src/executor/raw/mod.rs index b8455442..7e855916 100644 --- a/embassy/src/executor/raw/mod.rs +++ b/embassy/src/executor/raw/mod.rs @@ -445,6 +445,10 @@ impl Executor { /// Wake a task by raw pointer. /// /// You can obtain task pointers from `Waker`s using [`task_from_waker`]. +/// +/// # Safety +/// +/// `task` must be a valid task pointer obtained from [`task_from_waker`]. pub unsafe fn wake_task(task: NonNull) { task.as_ref().enqueue(); } diff --git a/embassy/src/executor/raw/waker.rs b/embassy/src/executor/raw/waker.rs index ea9010ca..605cda4c 100644 --- a/embassy/src/executor/raw/waker.rs +++ b/embassy/src/executor/raw/waker.rs @@ -33,12 +33,18 @@ pub(crate) unsafe fn from_task(p: NonNull) -> Waker { /// # Panics /// /// Panics if the waker is not created by the Embassy executor. -pub unsafe fn task_from_waker(waker: &Waker) -> NonNull { - let hack: &WakerHack = mem::transmute(waker); +pub fn task_from_waker(waker: &Waker) -> NonNull { + // safety: OK because WakerHack has the same layout as Waker. + // This is not really guaranteed because the structs are `repr(Rust)`, it is + // indeed the case in the current implementation. + // TODO use waker_getters when stable. https://github.com/rust-lang/rust/issues/96992 + let hack: &WakerHack = unsafe { mem::transmute(waker) }; if hack.vtable != &VTABLE { panic!("Found waker not created by the embassy executor. Consider enabling the `executor-agnostic` feature on the `embassy` crate.") } - NonNull::new_unchecked(hack.data as *mut TaskHeader) + + // safety: we never create a waker with a null data pointer. + unsafe { NonNull::new_unchecked(hack.data as *mut TaskHeader) } } struct WakerHack { diff --git a/embassy/src/executor/spawner.rs b/embassy/src/executor/spawner.rs index 884db6b5..c8d036eb 100644 --- a/embassy/src/executor/spawner.rs +++ b/embassy/src/executor/spawner.rs @@ -93,7 +93,7 @@ impl Spawner { pub async fn for_current_executor() -> Self { poll_fn(|cx| unsafe { let task = raw::task_from_waker(cx.waker()); - let executor = (&*task.as_ptr()).executor.get(); + let executor = (*task.as_ptr()).executor.get(); Poll::Ready(Self::new(&*executor)) }) .await @@ -169,7 +169,7 @@ impl SendSpawner { pub async fn for_current_executor() -> Self { poll_fn(|cx| unsafe { let task = raw::task_from_waker(cx.waker()); - let executor = (&*task.as_ptr()).executor.get(); + let executor = (*task.as_ptr()).executor.get(); Poll::Ready(Self::new(&*executor)) }) .await diff --git a/embassy/src/lib.rs b/embassy/src/lib.rs index c0a0deab..b7be8b34 100644 --- a/embassy/src/lib.rs +++ b/embassy/src/lib.rs @@ -3,6 +3,7 @@ #![cfg_attr(all(feature = "nightly", target_arch = "xtensa"), feature(asm_experimental_arch))] #![allow(clippy::new_without_default)] #![doc = include_str!("../../README.md")] +#![warn(missing_docs)] // This mod MUST go first, so that the others see its macros. pub(crate) mod fmt; diff --git a/embassy/src/util/forever.rs b/embassy/src/util/forever.rs index e367d264..ba3c66a9 100644 --- a/embassy/src/util/forever.rs +++ b/embassy/src/util/forever.rs @@ -31,6 +31,7 @@ unsafe impl Send for Forever {} unsafe impl Sync for Forever {} impl Forever { + /// Create a new `Forever`. #[inline(always)] pub const fn new() -> Self { Self { @@ -39,11 +40,15 @@ impl Forever { } } - /// Gives this `Forever` a value. + /// Store a value in this `Forever`, returning a mutable reference to it. /// - /// Panics if this `Forever` already has a value. + /// Using this method, the compiler usually constructs `val` in the stack and then moves + /// it into the `Forever`. If `T` is big, this is likely to cause stack overflows. + /// Considering using [`Signal::put_with`] instead, which will construct it in-place inside the `Forever`. /// - /// Returns a mutable reference to the stored value. + /// # Panics + /// + /// Panics if this `Forever` already has a value stored in it. #[inline(always)] #[allow(clippy::mut_from_ref)] pub fn put(&'static self, val: T) -> &'static mut T { @@ -52,7 +57,7 @@ impl Forever { .compare_exchange(false, true, Ordering::Relaxed, Ordering::Relaxed) .is_err() { - panic!("Forever.put() called multiple times"); + panic!("Forever::put() called multiple times"); } unsafe { @@ -63,6 +68,14 @@ impl Forever { } } + /// Store the closure return value in this `Forever`, returning a mutable reference to it. + /// + /// The advantage over [`Forever::put`] is that this method allows the closure to construct + /// the `T` value in-place directly inside the `Forever`, saving stack space. + /// + /// # Panics + /// + /// Panics if this `Forever` already has a value stored in it. #[inline(always)] #[allow(clippy::mut_from_ref)] pub fn put_with(&'static self, val: impl FnOnce() -> T) -> &'static mut T { @@ -82,9 +95,17 @@ impl Forever { } } + /// Unsafely get a mutable reference to the contents of this Forever. + /// + /// # Safety + /// + /// This is undefined behavior if: + /// + /// - The `Forever` has not been initialized yet (with `put' or `put_with`), or + /// - A reference to the contents (mutable or not) already exists. #[inline(always)] #[allow(clippy::mut_from_ref)] - pub unsafe fn steal(&'static self) -> &'static mut T { + pub unsafe fn steal(&self) -> &mut T { let p = self.t.get(); let p = (&mut *p).as_mut_ptr(); &mut *p diff --git a/embassy/src/util/select.rs b/embassy/src/util/select.rs index ccc50f11..8cecb7fa 100644 --- a/embassy/src/util/select.rs +++ b/embassy/src/util/select.rs @@ -2,9 +2,12 @@ use core::future::Future; use core::pin::Pin; use core::task::{Context, Poll}; +/// Result for [`select`]. #[derive(Debug, Clone)] pub enum Either { + /// First future finished first. First(A), + /// Second future finished first. Second(B), } @@ -55,10 +58,14 @@ where // ==================================================================== +/// Result for [`select3`]. #[derive(Debug, Clone)] pub enum Either3 { + /// First future finished first. First(A), + /// Second future finished first. Second(B), + /// Third future finished first. Third(C), } @@ -109,11 +116,16 @@ where // ==================================================================== +/// Result for [`select4`]. #[derive(Debug, Clone)] pub enum Either4 { + /// First future finished first. First(A), + /// Second future finished first. Second(B), + /// Third future finished first. Third(C), + /// Fourth future finished first. Fourth(D), } diff --git a/embassy/src/waitqueue/waker.rs b/embassy/src/waitqueue/waker.rs index a90154cc..cdc96507 100644 --- a/embassy/src/waitqueue/waker.rs +++ b/embassy/src/waitqueue/waker.rs @@ -16,13 +16,14 @@ pub struct WakerRegistration { } impl WakerRegistration { + /// Create a new `WakerRegistration`. pub const fn new() -> Self { Self { waker: None } } /// Register a waker. Overwrites the previous waker, if any. pub fn register(&mut self, w: &Waker) { - let w = unsafe { task_from_waker(w) }; + let w = task_from_waker(w); match self.waker { // Optimization: If both the old and new Wakers wake the same task, do nothing. Some(w2) if w == w2 => {} @@ -72,6 +73,7 @@ pub struct AtomicWaker { } impl AtomicWaker { + /// Create a new `AtomicWaker`. pub const fn new() -> Self { Self { waker: AtomicPtr::new(ptr::null_mut()), @@ -80,7 +82,7 @@ impl AtomicWaker { /// Register a waker. Overwrites the previous waker, if any. pub fn register(&self, w: &Waker) { - let w = unsafe { task_from_waker(w) }; + let w = task_from_waker(w); self.waker.store(w.as_ptr(), Ordering::Relaxed); compiler_fence(Ordering::SeqCst); } diff --git a/embassy/src/waitqueue/waker_agnostic.rs b/embassy/src/waitqueue/waker_agnostic.rs index 62e3adb7..64e300eb 100644 --- a/embassy/src/waitqueue/waker_agnostic.rs +++ b/embassy/src/waitqueue/waker_agnostic.rs @@ -12,6 +12,7 @@ pub struct WakerRegistration { } impl WakerRegistration { + /// Create a new `WakerRegistration`. pub const fn new() -> Self { Self { waker: None } } @@ -60,6 +61,7 @@ pub struct AtomicWaker { } impl AtomicWaker { + /// Create a new `AtomicWaker`. pub const fn new() -> Self { Self { waker: Mutex::const_new(CriticalSectionRawMutex::new(), Cell::new(None)),