pw_sync: Split TimedBorrowable from Borrowable

This CL separates time-related methods of Borrowable into a new, derived type, TimedBorrowable. Much like TimedMutex and TimedThreadNotification, this allows downstream consumers to use Borrowable without depending on pw_chrono. Change-Id: I1614dc6eadf8a70382ea8755e31aeedd1fc9c579 Reviewed-on: https://pigweed-review.googlesource.com/c/pigweed/pigweed/+/260313 Lint: Lint 🤖 <[email protected]> Commit-Queue: Aaron Green <[email protected]> Reviewed-by: Keir Mierle <[email protected]> Presubmit-Verified: CQ Bot Account <[email protected]>
google · Jan 14, 2025 · 86cb968 · 86cb968
1 parent b3361cf
commit 86cb968
Show file tree

Hide file tree

Showing 14 changed files with 336 additions and 207 deletions.
diff --git a/docs/BUILD.gn b/docs/BUILD.gn
@@ -356,6 +356,7 @@ _doxygen_input_files = [  # keep-sorted: start
   "$dir_pw_sync/public/pw_sync/lock_annotations.h",
   "$dir_pw_sync/public/pw_sync/mutex.h",
   "$dir_pw_sync/public/pw_sync/thread_notification.h",
+  "$dir_pw_sync/public/pw_sync/timed_borrow.h",
   "$dir_pw_sync/public/pw_sync/timed_mutex.h",
   "$dir_pw_sync/public/pw_sync/timed_thread_notification.h",
   "$dir_pw_sync/public/pw_sync/virtual_basic_lockable.h",

diff --git a/pw_sync/BUILD.bazel b/pw_sync/BUILD.bazel
@@ -110,6 +110,21 @@ cc_library(
     ],
 )
 
+cc_library(
+    name = "timed_borrow",
+    hdrs = [
+        "public/pw_sync/timed_borrow.h",
+    ],
+    strip_include_prefix = "public",
+    deps = [
+        ":borrow",
+        ":lock_annotations",
+        ":lock_traits",
+        ":virtual_basic_lockable",
+        "//pw_chrono:system_clock",
+    ],
+)
+
 cc_library(
     name = "inline_borrowable",
     hdrs = [
@@ -375,6 +390,7 @@ cc_library(
     deps = [
         ":virtual_basic_lockable",
         "//pw_assert",
+        "//pw_chrono:system_clock",
     ],
 )
 
@@ -388,6 +404,16 @@ cc_library(
     ],
 )
 
+cc_library(
+    name = "timed_borrow_testing",
+    hdrs = ["public/pw_sync/timed_borrow_testing.h"],
+    strip_include_prefix = "public",
+    deps = [
+        ":borrow_testing",
+        ":timed_borrow",
+    ],
+)
+
 pw_cc_test(
     name = "lock_traits_test",
     srcs = ["lock_traits_test.cc"],
@@ -402,8 +428,8 @@ pw_cc_test(
     srcs = ["borrow_test.cc"],
     deps = [
         ":borrow",
-        ":borrow_testing",
         ":lock_testing",
+        ":timed_borrow_testing",
         "//pw_unit_test",
     ],
 )
@@ -469,7 +495,7 @@ pw_cc_test(
         "timed_mutex_facade_test_c.c",
     ],
     deps = [
-        ":borrow_testing",
+        ":timed_borrow_testing",
         ":timed_mutex",
         "//pw_chrono:system_clock",
         "//pw_preprocessor",
@@ -544,6 +570,7 @@ filegroup(
         "public/pw_sync/lock_annotations.h",
         "public/pw_sync/mutex.h",
         "public/pw_sync/thread_notification.h",
+        "public/pw_sync/timed_borrow.h",
         "public/pw_sync/timed_mutex.h",
         "public/pw_sync/timed_thread_notification.h",
         "public/pw_sync/virtual_basic_lockable.h",

diff --git a/pw_sync/BUILD.gn b/pw_sync/BUILD.gn
@@ -75,6 +75,18 @@ pw_source_set("borrow") {
   ]
 }
 
+pw_source_set("timed_borrow") {
+  public_configs = [ ":public_include_path" ]
+  public = [ "public/pw_sync/timed_borrow.h" ]
+  public_deps = [
+    ":borrow",
+    ":lock_annotations",
+    ":lock_traits",
+    ":virtual_basic_lockable",
+    "$dir_pw_chrono:system_clock",
+  ]
+}
+
 pw_source_set("inline_borrowable") {
   public = [
     "public/pw_sync/inline_borrowable.h",
@@ -238,7 +250,10 @@ pw_source_set("lock_testing") {
   public_configs = [ ":public_include_path" ]
   public = [ "public/pw_sync/lock_testing.h" ]
   sources = [ "lock_testing.cc" ]
-  public_deps = [ ":virtual_basic_lockable" ]
+  public_deps = [
+    ":virtual_basic_lockable",
+    "$dir_pw_chrono:system_clock",
+  ]
   deps = [ dir_pw_assert ]
 }
 
@@ -251,7 +266,17 @@ pw_source_set("borrow_testing") {
   ]
 }
 
+pw_source_set("timed_borrow_testing") {
+  public_configs = [ ":public_include_path" ]
+  public = [ "public/pw_sync/timed_borrow_testing.h" ]
+  public_deps = [
+    ":borrow_testing",
+    ":timed_borrow",
+  ]
+}
+
 pw_test("lock_traits_test") {
+  enable_if = pw_chrono_SYSTEM_CLOCK_BACKEND != ""
   sources = [ "lock_traits_test.cc" ]
   deps = [
     ":lock_testing",
@@ -260,11 +285,12 @@ pw_test("lock_traits_test") {
 }
 
 pw_test("borrow_test") {
+  enable_if = pw_chrono_SYSTEM_CLOCK_BACKEND != ""
   sources = [ "borrow_test.cc" ]
   deps = [
     ":borrow",
-    ":borrow_testing",
     ":lock_testing",
+    ":timed_borrow_testing",
   ]
 }
 
@@ -325,7 +351,7 @@ pw_test("timed_mutex_facade_test") {
     "timed_mutex_facade_test_c.c",
   ]
   deps = [
-    ":borrow_testing",
+    ":timed_borrow_testing",
     ":timed_mutex",
     "$dir_pw_preprocessor",
     pw_sync_TIMED_MUTEX_BACKEND,

diff --git a/pw_sync/CMakeLists.txt b/pw_sync/CMakeLists.txt
@@ -71,6 +71,19 @@ pw_add_library(pw_sync.borrow INTERFACE
     pw_sync.virtual_basic_lockable
 )
 
+pw_add_library(pw_sync.timed_borrow INTERFACE
+  HEADERS
+    public/pw_sync/timed_borrow.h
+  PUBLIC_INCLUDES
+    public
+  PUBLIC_DEPS
+    pw_chrono.system_clock
+    pw_sync.borrow
+    pw_sync.lock_annotations
+    pw_sync.lock_traits
+    pw_sync.virtual_basic_lockable
+)
+
 pw_add_library(pw_sync.inline_borrowable INTERFACE
   HEADERS
     public/pw_sync/inline_borrowable.h
@@ -230,6 +243,7 @@ pw_add_library(pw_sync.lock_testing STATIC
   PUBLIC_INCLUDES
     public
   PUBLIC_DEPS
+    pw_chrono.system_clock
     pw_sync.virtual_basic_lockable
   PRIVATE_DEPS
     pw_assert
@@ -243,26 +257,38 @@ pw_add_library(pw_sync.borrow_testing INTERFACE
     pw_sync.lock_traits
 )
 
-pw_add_test(pw_sync.lock_traits_test
-  SOURCES
-    lock_traits_test.cc
-  PRIVATE_DEPS
-    pw_sync.lock_testing
-    pw_sync.lock_traits
-)
-
-pw_add_test(pw_sync.borrow_test
-  SOURCES
-    borrow_test.cc
-  PRIVATE_DEPS
-    pw_sync.borrow
+pw_add_library(pw_sync.timed_borrow_testing INTERFACE
+  HEADERS
+    public/pw_sync/timed_borrow_testing.h
+  PUBLIC_DEPS
     pw_sync.borrow_testing
-    pw_sync.lock_testing
-  GROUPS
-    modules
-    pw_sync
+    pw_sync.timed_borrow
 )
 
+if(NOT "${pw_chrono.system_clock_BACKEND}" STREQUAL "")
+  pw_add_test(pw_sync.lock_traits_test
+    SOURCES
+      lock_traits_test.cc
+    PRIVATE_DEPS
+      pw_sync.lock_testing
+      pw_sync.lock_traits
+  )
+endif()
+
+if(NOT "${pw_chrono.system_clock_BACKEND}" STREQUAL "")
+  pw_add_test(pw_sync.borrow_test
+    SOURCES
+      borrow_test.cc
+    PRIVATE_DEPS
+      pw_sync.borrow
+      pw_sync.timed_borrow_testing
+      pw_sync.lock_testing
+    GROUPS
+      modules
+      pw_sync
+  )
+endif()
+
 pw_add_test(pw_sync.inline_borrowable_test
   SOURCES
     inline_borrowable_test.cc
@@ -327,7 +353,7 @@ if(NOT "${pw_sync.timed_mutex_BACKEND}" STREQUAL "")
     PRIVATE_DEPS
       pw_preprocessor
       pw_sync.timed_mutex
-      pw_sync.borrow_testing
+      pw_sync.timed_borrow_testing
     GROUPS
       modules
       pw_sync

diff --git a/pw_sync/borrow_test.cc b/pw_sync/borrow_test.cc
@@ -14,13 +14,21 @@
 
 #include "pw_sync/borrow.h"
 
-#include "pw_sync/borrow_testing.h"
 #include "pw_sync/lock_testing.h"
+#include "pw_sync/timed_borrow_testing.h"
 #include "pw_unit_test/framework.h"
 
+using namespace std::chrono_literals;
+
 namespace pw::sync::test {
 namespace {
 
+// We can't control the SystemClock's period configuration, so just in case
+// duration cannot be accurately expressed in integer ticks, round the
+// duration up.
+constexpr chrono::SystemClock::duration kRoundedArbitraryDuration =
+    chrono::SystemClock::for_at_least(42ms);
+
 TEST(BorrowedPointerTest, MoveConstruct) {
   Derived derived(1);
   FakeBasicLockable lock;
@@ -75,7 +83,7 @@ TEST_F(FakeLockableBorrowTest, TryAcquireSuccess) { TestTryAcquireSuccess(); }
 TEST_F(FakeLockableBorrowTest, TryAcquireFailure) { TestTryAcquireFailure(); }
 
 // Unit tests for a `Borrowable`that uses a `FakeTimedLockable` as its lock.
-using FakeTimedLockableBorrowTest = BorrowTest<FakeTimedLockable, FakeClock>;
+using FakeTimedLockableBorrowTest = TimedBorrowTest<FakeTimedLockable>;
 
 TEST_F(FakeTimedLockableBorrowTest, Acquire) { TestAcquire(); }
 
@@ -100,19 +108,19 @@ TEST_F(FakeTimedLockableBorrowTest, TryAcquireFailure) {
 }
 
 TEST_F(FakeTimedLockableBorrowTest, TryAcquireForSuccess) {
-  TestTryAcquireForSuccess();
+  TestTryAcquireForSuccess(kRoundedArbitraryDuration);
 }
 
 TEST_F(FakeTimedLockableBorrowTest, TryAcquireForFailure) {
-  TestTryAcquireForFailure();
+  TestTryAcquireForFailure(kRoundedArbitraryDuration);
 }
 
 TEST_F(FakeTimedLockableBorrowTest, TryAcquireUntilSuccess) {
-  TestTryAcquireUntilSuccess();
+  TestTryAcquireUntilSuccess(kRoundedArbitraryDuration);
 }
 
 TEST_F(FakeTimedLockableBorrowTest, TryAcquireUntilFailure) {
-  TestTryAcquireUntilFailure();
+  TestTryAcquireUntilFailure(kRoundedArbitraryDuration);
 }
 
 }  // namespace

diff --git a/pw_sync/docs.rst b/pw_sync/docs.rst
@@ -896,6 +896,9 @@ C++
 .. doxygenclass:: pw::sync::Borrowable
    :members:
 
+.. doxygenclass:: pw::sync::TimedBorrowable
+   :members:
+
 Example in C++
 ^^^^^^^^^^^^^^
 

diff --git a/pw_sync/interrupt_spin_lock_facade_test.cc b/pw_sync/interrupt_spin_lock_facade_test.cc
@@ -61,7 +61,7 @@ TEST(InterruptSpinLock, TryLockUnlock) {
 }
 
 // Unit tests for a `Borrowable`that uses a `InterruptSpinLock` as its lock.
-using InterruptSpinLockBorrowTest = BorrowTest<InterruptSpinLock>;
+using InterruptSpinLockBorrowTest = test::BorrowTest<InterruptSpinLock>;
 
 TEST_F(InterruptSpinLockBorrowTest, Acquire) { TestAcquire(); }
 
@@ -103,7 +103,8 @@ TEST(VirtualInterruptSpinLock, LockUnlockStatic) {
 
 // Unit tests for a `Borrowable`that uses a `VirtualInterruptSpinLock` as its
 // lock.
-using VirtualInterruptSpinLockBorrowTest = BorrowTest<VirtualInterruptSpinLock>;
+using VirtualInterruptSpinLockBorrowTest =
+    test::BorrowTest<VirtualInterruptSpinLock>;
 
 TEST_F(VirtualInterruptSpinLockBorrowTest, Acquire) { TestAcquire(); }
 

diff --git a/pw_sync/mutex_facade_test.cc b/pw_sync/mutex_facade_test.cc
@@ -60,7 +60,7 @@ TEST(Mutex, TryLockUnlock) {
 }
 
 // Unit tests for a `Borrowable`that uses a `Mutex` as its lock.
-using MutexBorrowTest = BorrowTest<Mutex>;
+using MutexBorrowTest = test::BorrowTest<Mutex>;
 
 TEST_F(MutexBorrowTest, Acquire) { TestAcquire(); }
 
@@ -104,7 +104,7 @@ TEST(VirtualMutex, LockUnlockExternal) {
 }
 
 // Unit tests for a `Borrowable`that uses a `VirtualMutex` as its lock.
-using VirtualMutexBorrowTest = BorrowTest<VirtualMutex>;
+using VirtualMutexBorrowTest = test::BorrowTest<VirtualMutex>;
 
 TEST_F(VirtualMutexBorrowTest, Acquire) { TestAcquire(); }