pw_multisink: Add option to inject a user defined lock

Change-Id: I0a80b675fdd117973803e209f51d0a2d52018056
Reviewed-on: https://pigweed-review.googlesource.com/c/pigweed/pigweed/+/213211
Pigweed-Auto-Submit: Kevin Zeng <[email protected]>
Commit-Queue: Auto-Submit <[email protected]>
Lint: Lint 🤖 <[email protected]>
Reviewed-by: Carlos Chinchilla <[email protected]>
Reviewed-by: Wyatt Hepler <[email protected]>
Presubmit-Verified: CQ Bot Account <[email protected]>

pw_multisink/BUILD.gn

@@ -46,6 +46,8 @@ pw_source_set("pw_multisink") {
     "$dir_pw_sync:interrupt_spin_lock",
     "$dir_pw_sync:lock_annotations",
     "$dir_pw_sync:mutex",
+    "$dir_pw_sync:virtual_basic_lockable",
+    dir_pw_assert,
     dir_pw_bytes,
     dir_pw_containers,
     dir_pw_function,

pw_multisink/docs.rst

@@ -14,16 +14,18 @@ of this module, see the
 :ref:`module documentation <module-structure-compile-time-configuration>` for
 more details.
 
-.. c:macro:: PW_MULTISINK_CONFIG_LOCK_INTERRUPT_SAFE
+.. c:macro:: PW_MULTISINK_CONFIG_LOCK_TYPE
 
-  Whether an interrupt-safe lock is used to guard multisink read/write
-  operations.
+  Set to configure the underlying lock used to guard multisink read/write
+  operations.  By default, this is set to uses an interrupt spin-lock to guard
+  the multisink transactions. Should be set to one of the following options:
 
-  By default, this option is enabled and the multisink uses an interrupt
-  spin-lock to guard its transactions. If disabled, a mutex is used instead.
+  - PW_MULTISINK_INTERRUPT_SPIN_LOCK: Use an interrupt spin lock
+  - PW_MULTISINK_MUTEX: Use a mutex. Not safe to use multisink in interrupt
+    context
+  - PW_MULTISINK_VIRTUAL_LOCK: User provided locking implementation. Interrupt
+    support will be left to the user to manage.
 
-  Disabling this will alter the entry precondition of the multisink,
-  requiring that it not be called from an interrupt context.
 
 .. _module-pw_multisink-late_drain_attach:
 

pw_multisink/public/pw_multisink/config.h

@@ -13,27 +13,54 @@
 // the License.
 #pragma once
 
-// PW_MULTISINK_LOCK_INTERRUPT_SAFE controls whether an interrupt-safe lock is
-// used when reading and writing from the underlying ring-buffer. This is
-// enabled by default, using an interrupt spin-lock instead of a mutex.
-// Disabling this alters the entry precondition of the multisink, requiring that
-// it not be invoked from an interrupt context.
-#if !defined(PW_MULTISINK_CONFIG_LOCK_INTERRUPT_SAFE)
-#define PW_MULTISINK_CONFIG_LOCK_INTERRUPT_SAFE 1
-#endif  // !defined(PW_MULTISINK_CONFIG_LOCK_INTERRUPT_SAFE)
+// Enum for different lock configurations.
+// Use non-obvious numbers so users use the macro instead of an integer
+#define PW_MULTISINK_INTERRUPT_SPIN_LOCK 100
+#define PW_MULTISINK_MUTEX 200
+#define PW_MULTISINK_VIRTUAL_LOCK 300
 
-#if PW_MULTISINK_CONFIG_LOCK_INTERRUPT_SAFE
+// PW_MULTISINK_CONFIG_LOCK_TYPE controls which lock is used when reading and
+// writing from the underlying ring-buffer. The interrupt spin lock is
+// enabled by default if PW_MULTISINK_CONFIG_LOCK_TYPE is not set. Otherwise,
+// one of the following locking implementations can be set using the enums
+// above. PW_MULTISINK_VIRTUAL_LOCK can be used if the user wants to provide
+// their own locking implementation.
+#ifndef PW_MULTISINK_CONFIG_LOCK_TYPE
+// Backwards compatibility
+#ifdef PW_MULTISINK_LOCK_INTERRUPT_SAFE
+#warning "Multisink PW_MULTISINK_INTERRUPT_SPIN_LOCK is deprecated!!!"
+#if PW_MULTISINK_LOCK_INTERRUPT_SAFE
+#define PW_MULTISINK_CONFIG_LOCK_TYPE PW_MULTISINK_INTERRUPT_SPIN_LOCK
+#else
+#define PW_MULTISINK_CONFIG_LOCK_TYPE PW_MULTISINK_MUTEX
+#endif
+#else
+// Default to use interrupt spin lock.
+#define PW_MULTISINK_CONFIG_LOCK_TYPE PW_MULTISINK_INTERRUPT_SPIN_LOCK
+#endif
+#endif  // PW_MULTISINK_CONFIG_LOCK_TYPE
+
+static_assert(PW_MULTISINK_CONFIG_LOCK_TYPE ==
+                  PW_MULTISINK_INTERRUPT_SPIN_LOCK ||
+              PW_MULTISINK_CONFIG_LOCK_TYPE == PW_MULTISINK_MUTEX ||
+              PW_MULTISINK_CONFIG_LOCK_TYPE == PW_MULTISINK_VIRTUAL_LOCK);
+
+#if PW_MULTISINK_CONFIG_LOCK_TYPE == PW_MULTISINK_INTERRUPT_SPIN_LOCK
 #include "pw_sync/interrupt_spin_lock.h"
-#else  // !PW_MULTISINK_CONFIG_LOCK_INTERRUPT_SAFE
+#elif PW_MULTISINK_CONFIG_LOCK_TYPE == PW_MULTISINK_VIRTUAL_LOCK
+#include "pw_sync/virtual_basic_lockable.h"
+#elif PW_MULTISINK_CONFIG_LOCK_TYPE == PW_MULTISINK_MUTEX
 #include "pw_sync/mutex.h"
 #endif  // PW_MULTISINK_CONFIG_LOCK_INTERRUPT_SAFE
 
 namespace pw {
 namespace multisink {
 
-#if PW_MULTISINK_CONFIG_LOCK_INTERRUPT_SAFE
+#if PW_MULTISINK_CONFIG_LOCK_TYPE == PW_MULTISINK_INTERRUPT_SPIN_LOCK
 using LockType = pw::sync::InterruptSpinLock;
-#else   // !PW_MULTISINK_CONFIG_LOCK_INTERRUPT_SAFE
+#elif PW_MULTISINK_CONFIG_LOCK_TYPE == PW_MULTISINK_VIRTUAL_LOCK
+using LockType = pw::sync::VirtualBasicLockable&;
+#elif PW_MULTISINK_CONFIG_LOCK_TYPE == PW_MULTISINK_MUTEX
 using LockType = pw::sync::Mutex;
 #endif  // PW_MULTISINK_CONFIG_LOCK_INTERRUPT_SAFE
 

pw_multisink/public/pw_multisink/multisink.h

@@ -16,6 +16,7 @@
 #include <limits>
 #include <mutex>
 
+#include "pw_assert/assert.h"
 #include "pw_bytes/span.h"
 #include "pw_function/function.h"
 #include "pw_multisink/config.h"
@@ -310,10 +311,18 @@ class MultiSink {
   }
 
   // Constructs a multisink using a ring buffer backed by the provided buffer.
+  // If we're using a virtual lock, then the lock needs to be passed in.
+#if PW_MULTISINK_CONFIG_LOCK_TYPE == PW_MULTISINK_VIRTUAL_LOCK
+  MultiSink(ByteSpan buffer, LockType lock)
+      : lock_(lock),
+#else
   MultiSink(ByteSpan buffer)
-      : ring_buffer_(true), sequence_id_(0), total_ingress_drops_(0) {
-    ring_buffer_.SetBuffer(buffer)
-        .IgnoreError();  // TODO: b/242598609 - Handle Status properly
+      :
+#endif
+        ring_buffer_(true),
+        sequence_id_(0),
+        total_ingress_drops_(0) {
+    PW_ASSERT(ring_buffer_.SetBuffer(buffer).ok());
     AttachDrain(oldest_entry_drain_);
   }
 
@@ -415,12 +424,12 @@ class MultiSink {
   // Notifies attached listeners of new entries or an updated drop count.
   void NotifyListeners() PW_EXCLUSIVE_LOCKS_REQUIRED(lock_);
 
+  LockType lock_;
   IntrusiveList<Listener> listeners_ PW_GUARDED_BY(lock_);
   ring_buffer::PrefixedEntryRingBufferMulti ring_buffer_ PW_GUARDED_BY(lock_);
   Drain oldest_entry_drain_ PW_GUARDED_BY(lock_);
   uint32_t sequence_id_ PW_GUARDED_BY(lock_);
   uint32_t total_ingress_drops_ PW_GUARDED_BY(lock_);
-  LockType lock_;
 };
 
 }  // namespace multisink