Skip to content

Commit

Permalink
pw_thread: Migrate to Thread::id
Browse files Browse the repository at this point in the history
Update docs and code to the new pw::Thread::id alias instead of the
legacy thread::Id alias.

Bug: b/373524945
Change-Id: I9a7b938cf5135a5a380dcf108193c6b407902e65
Reviewed-on: https://pigweed-review.googlesource.com/c/pigweed/pigweed/+/238432
Commit-Queue: Auto-Submit <[email protected]>
Presubmit-Verified: CQ Bot Account <[email protected]>
Pigweed-Auto-Submit: Wyatt Hepler <[email protected]>
Reviewed-by: Dave Roth <[email protected]>
  • Loading branch information
255 authored and CQ Bot Account committed Oct 18, 2024
1 parent 9c37b72 commit e5db91d
Show file tree
Hide file tree
Showing 36 changed files with 142 additions and 155 deletions.
11 changes: 5 additions & 6 deletions pw_thread/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ pw_facade(
],
backend = ":sleep_backend",
implementation_deps = [
":id",
":thread",
],
includes = ["public"],
deps = [
Expand Down Expand Up @@ -214,7 +214,7 @@ pw_facade(
],
backend = ":yield_backend",
implementation_deps = [
":id",
":thread",
],
includes = ["public"],
deps = [
Expand Down Expand Up @@ -271,7 +271,6 @@ cc_library(
"thread_facade_test.cc",
],
deps = [
":id",
":non_portable_test_thread_options",
":thread",
"//pw_chrono:system_clock",
Expand Down Expand Up @@ -310,7 +309,7 @@ pw_cc_test(
"id_facade_test.cc",
],
deps = [
":id",
":thread",
"//pw_unit_test",
],
)
Expand All @@ -331,8 +330,8 @@ pw_cc_test(
"sleep_facade_test_c.c",
],
deps = [
":id",
":sleep",
":thread",
"//pw_chrono:system_clock",
"//pw_preprocessor",
"//pw_unit_test",
Expand Down Expand Up @@ -377,7 +376,7 @@ pw_cc_test(
"yield_facade_test_c.c",
],
deps = [
":id",
":thread",
":yield",
"//pw_preprocessor",
"//pw_unit_test",
Expand Down
7 changes: 3 additions & 4 deletions pw_thread/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ pw_test("test_thread_context_facade_test") {
pw_test("id_facade_test") {
enable_if = pw_thread_ID_BACKEND != ""
sources = [ "id_facade_test.cc" ]
deps = [ ":id" ]
deps = [ ":thread" ]
}

pw_test("thread_snapshot_service_test") {
Expand All @@ -213,8 +213,8 @@ pw_test("sleep_facade_test") {
"sleep_facade_test_c.c",
]
deps = [
":id",
":sleep",
":thread",
"$dir_pw_chrono:system_clock",
]
}
Expand All @@ -233,7 +233,6 @@ pw_source_set("thread_facade_test") {
testonly = pw_unit_test_TESTONLY
sources = [ "thread_facade_test.cc" ]
deps = [
":id",
":non_portable_test_thread_options",
":sleep",
":thread",
Expand Down Expand Up @@ -263,7 +262,7 @@ pw_test("yield_facade_test") {
"yield_facade_test_c.c",
]
deps = [
":id",
":thread",
":yield",
]
}
Expand Down
8 changes: 4 additions & 4 deletions pw_thread/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ pw_add_library(pw_thread.config INTERFACE
${pw_thread_CONFIG}
)

# pw_thread.id is deprecated; depend on pw_thread.thread instead.
pw_add_facade(pw_thread.id INTERFACE
BACKEND
pw_thread.id_BACKEND
Expand Down Expand Up @@ -183,7 +184,7 @@ if(NOT "${pw_thread.id_BACKEND}" STREQUAL "")
SOURCES
id_facade_test.cc
PRIVATE_DEPS
pw_thread.id
pw_thread.thread
GROUPS
modules
pw_thread
Expand All @@ -198,8 +199,8 @@ if((NOT "${pw_thread.id_BACKEND}" STREQUAL "") AND
sleep_facade_test_c.c
PRIVATE_DEPS
pw_chrono.system_clock
pw_thread.id
pw_thread.sleep
pw_thread.thread
GROUPS
modules
pw_thread
Expand All @@ -223,7 +224,6 @@ pw_add_library(pw_thread.thread_facade_test STATIC
SOURCES
thread_facade_test.cc
PRIVATE_DEPS
pw_thread.id
pw_thread.sleep
pw_thread.non_portable_test_thread_options
pw_thread.thread
Expand All @@ -238,7 +238,7 @@ if((NOT "${pw_thread.id_BACKEND}" STREQUAL "") AND
yield_facade_test.cc
yield_facade_test_c.c
PRIVATE_DEPS
pw_thread.id
pw_thread.thread
pw_thread.yield
GROUPS
modules
Expand Down
2 changes: 1 addition & 1 deletion pw_thread/backend.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ include_guard(GLOBAL)

include($ENV{PW_ROOT}/pw_build/pigweed.cmake)

# Backend for the pw_thread module's pw::thread::Id.
# Backend for the pw_thread module's pw::Thread::id.
pw_add_backend_variable(pw_thread.id_BACKEND)

# Backend for the pw_thread module's pw::thread::sleep_{for,until}.
Expand Down
2 changes: 1 addition & 1 deletion pw_thread/backend.gni
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
# the License.

declare_args() {
# Backend for the pw_thread module's pw::thread::Id.
# Backend for the pw_thread module's pw::Thread::id.
pw_thread_ID_BACKEND = ""

# Backend for the pw_thread module's pw::thread::sleep_{for,until}.
Expand Down
26 changes: 12 additions & 14 deletions pw_thread/docs.rst
Original file line number Diff line number Diff line change
Expand Up @@ -103,8 +103,8 @@ C
---------------------
Thread Identification
---------------------
The class ``pw::thread::Id`` is a lightweight, trivially copyable class that
serves as a unique identifier of Thread objects.
The class :cpp:type:`pw::Thread::id` is a lightweight, trivially copyable class
that serves as a unique identifier of Thread objects.

Instances of this class may also hold the special distinct value that does
not represent any thread. Once a thread has finished, the value of its
Expand All @@ -113,20 +113,19 @@ Thread::id may be reused by another thread.
This class is designed for use as key in associative containers, both ordered
and unordered.

Although the current API is similar to C++11 STL
`std::thread::id <https://en.cppreference.com/w/cpp/thread/thread/id>`_, it is
missing the required hashing and streaming operators and may diverge further in
the future.
Although the current API is similar to C++11 STL `std::thread::id
<https://en.cppreference.com/w/cpp/thread/thread/id>`_, it is missing the
required hashing and streaming operators and may diverge further in the future.

A thread's identification (``pw::thread::Id``) can be acquired only in C++ in
one of two ways:
A thread's identification (:cpp:type:`pw::Thread::id`) can be acquired only in
C++ in one of two ways:

1) Using the ``pw::thread::Thread`` handle's ``pw::thread::Id get_id() const``
1) Using the :cpp:type:`pw::Thread` handle's ``pw::Thread::id get_id() const``
method.
2) While executing the thread using
``pw::thread::Id pw::this_thread::get_id() noexcept``.
``pw::Thread::id pw::this_thread::get_id() noexcept``.

.. cpp:function:: pw::thread::Id pw::this_thread::get_id() noexcept
.. cpp:function:: pw::Thread::id pw::this_thread::get_id() noexcept

This is thread safe, not IRQ safe. It is implementation defined whether this
is safe before the scheduler has started.
Expand All @@ -136,13 +135,12 @@ Example
=======
.. code-block:: cpp
#include "pw_thread/id.h"
#include "pw_thread/thread.h"
void FunctionInvokedByThread() {
const pw::thread::Id my_id = pw::this_thread::get_id();
const pw::Thread::id my_id = pw::this_thread::get_id();
}
.. _module-pw_thread-thread-creation:

---------------
Expand Down
4 changes: 2 additions & 2 deletions pw_thread/id_facade_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
// License for the specific language governing permissions and limitations under
// the License.

#include "pw_thread/id.h"
#include "pw_thread/thread.h"
#include "pw_unit_test/framework.h"

namespace pw::this_thread {
Expand All @@ -22,7 +22,7 @@ TEST(Id, GetId) {
// We expect unit tests to run in a thread context.
// Unfortunately beyond this we need the ability to create and destroy threads
// to test more Id functionality.
EXPECT_NE(get_id(), thread::Id());
EXPECT_NE(get_id(), Thread::id());
}

} // namespace
Expand Down
6 changes: 4 additions & 2 deletions pw_thread/public/pw_thread/id.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,11 @@

#include "pw_thread_backend/id_native.h"

// This header is deprecated. Include pw_thread/thread.h instead.

namespace pw::thread {

// Legacy alias. Use pw::Thread::id instead.
// Legacy alias. Use pw::Thread::id in pw_thread/thread.h instead.
using Id = backend::NativeId;

} // namespace pw::thread
Expand All @@ -26,7 +28,7 @@ namespace pw::this_thread {

// This is thread safe, not IRQ safe. It is implementation defined whether this
// is safe before the scheduler has started.
thread::Id get_id() noexcept;
thread::backend::NativeId get_id() noexcept;

} // namespace pw::this_thread

Expand Down
2 changes: 1 addition & 1 deletion pw_thread/public/pw_thread/thread.h
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
// the License.
#pragma once

#include <tuple>
#include <type_traits>

#include "pw_function/function.h"
#include "pw_thread/id.h"
Expand Down
26 changes: 13 additions & 13 deletions pw_thread/sleep_facade_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@
#include <chrono>

#include "pw_chrono/system_clock.h"
#include "pw_thread/id.h"
#include "pw_thread/sleep.h"
#include "pw_thread/thread.h"
#include "pw_unit_test/framework.h"

using pw::chrono::SystemClock;
Expand Down Expand Up @@ -47,7 +47,7 @@ constexpr pw_chrono_SystemClock_Duration kRoundedArbitraryLongDurationInC =

TEST(Sleep, SleepForPositiveDuration) {
// Ensure we are in a thread context, meaning we are permitted to sleep.
ASSERT_NE(get_id(), thread::Id());
ASSERT_NE(get_id(), Thread::id());

SystemClock::time_point before = SystemClock::now();
sleep_for(kRoundedArbitraryShortDuration);
Expand All @@ -57,7 +57,7 @@ TEST(Sleep, SleepForPositiveDuration) {

TEST(Sleep, SleepForZeroLengthDuration) {
// Ensure we are in a thread context, meaning we are permitted to sleep.
ASSERT_NE(get_id(), thread::Id());
ASSERT_NE(get_id(), Thread::id());

// Ensure it doesn't sleep when a zero length duration is used.
SystemClock::time_point before = SystemClock::now();
Expand All @@ -68,7 +68,7 @@ TEST(Sleep, SleepForZeroLengthDuration) {

TEST(Sleep, SleepForNegativeDuration) {
// Ensure we are in a thread context, meaning we are permitted to sleep.
ASSERT_NE(get_id(), thread::Id());
ASSERT_NE(get_id(), Thread::id());

// Ensure it doesn't sleep when a negative duration is used.
SystemClock::time_point before = SystemClock::now();
Expand All @@ -79,7 +79,7 @@ TEST(Sleep, SleepForNegativeDuration) {

TEST(Sleep, SleepUntilFutureWakeupTime) {
// Ensure we are in a thread context, meaning we are permitted to sleep.
ASSERT_NE(get_id(), thread::Id());
ASSERT_NE(get_id(), Thread::id());

SystemClock::time_point deadline =
SystemClock::now() + kRoundedArbitraryShortDuration;
Expand All @@ -89,7 +89,7 @@ TEST(Sleep, SleepUntilFutureWakeupTime) {

TEST(Sleep, SleepUntilCurrentWakeupTime) {
// Ensure we are in a thread context, meaning we are permitted to sleep.
ASSERT_NE(get_id(), thread::Id());
ASSERT_NE(get_id(), Thread::id());

// Ensure it doesn't sleep when now is used.
SystemClock::time_point deadline =
Expand All @@ -100,7 +100,7 @@ TEST(Sleep, SleepUntilCurrentWakeupTime) {

TEST(Sleep, SleepUntilPastWakeupTime) {
// Ensure we are in a thread context, meaning we are permitted to sleep.
ASSERT_NE(get_id(), thread::Id());
ASSERT_NE(get_id(), Thread::id());

// Ensure it doesn't sleep when a timestamp in the past is used.
SystemClock::time_point deadline =
Expand All @@ -111,7 +111,7 @@ TEST(Sleep, SleepUntilPastWakeupTime) {

TEST(Sleep, SleepForPositiveDurationInC) {
// Ensure we are in a thread context, meaning we are permitted to sleep.
ASSERT_NE(get_id(), thread::Id());
ASSERT_NE(get_id(), Thread::id());

pw_chrono_SystemClock_TimePoint before = pw_chrono_SystemClock_Now();
pw_this_thread_SleepFor(kRoundedArbitraryShortDurationInC);
Expand All @@ -122,7 +122,7 @@ TEST(Sleep, SleepForPositiveDurationInC) {

TEST(Sleep, SleepForZeroLengthDurationInC) {
// Ensure we are in a thread context, meaning we are permitted to sleep.
ASSERT_NE(get_id(), thread::Id());
ASSERT_NE(get_id(), Thread::id());

// Ensure it doesn't sleep when a zero length duration is used.
pw_chrono_SystemClock_TimePoint before = pw_chrono_SystemClock_Now();
Expand All @@ -134,7 +134,7 @@ TEST(Sleep, SleepForZeroLengthDurationInC) {

TEST(Sleep, SleepForNegativeDurationInC) {
// Ensure we are in a thread context, meaning we are permitted to sleep.
ASSERT_NE(get_id(), thread::Id());
ASSERT_NE(get_id(), Thread::id());

// Ensure it doesn't sleep when a negative duration is used.
pw_chrono_SystemClock_TimePoint before = pw_chrono_SystemClock_Now();
Expand All @@ -147,7 +147,7 @@ TEST(Sleep, SleepForNegativeDurationInC) {

TEST(Sleep, SleepUntilFutureWakeupTimeInC) {
// Ensure we are in a thread context, meaning we are permitted to sleep.
ASSERT_NE(get_id(), thread::Id());
ASSERT_NE(get_id(), Thread::id());

pw_chrono_SystemClock_TimePoint deadline;
deadline.duration_since_epoch.ticks =
Expand All @@ -160,7 +160,7 @@ TEST(Sleep, SleepUntilFutureWakeupTimeInC) {

TEST(Sleep, SleepUntilCurrentWakeupTimeInC) {
// Ensure we are in a thread context, meaning we are permitted to sleep.
ASSERT_NE(get_id(), thread::Id());
ASSERT_NE(get_id(), Thread::id());

// Ensure it doesn't sleep when now is used.
pw_chrono_SystemClock_TimePoint deadline;
Expand All @@ -174,7 +174,7 @@ TEST(Sleep, SleepUntilCurrentWakeupTimeInC) {

TEST(Sleep, SleepUntilPastWakeupTimeInC) {
// Ensure we are in a thread context, meaning we are permitted to sleep.
ASSERT_NE(get_id(), thread::Id());
ASSERT_NE(get_id(), Thread::id());

// Ensure it doesn't sleep when a timestamp in the past is used.
pw_chrono_SystemClock_TimePoint deadline;
Expand Down
8 changes: 4 additions & 4 deletions pw_thread/test_thread_context_facade_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -35,19 +35,19 @@ TEST(Thread, TestThreadContext) {
thread_1 = Thread(context_1.options(),
[&thread_ran_sem_1] { thread_ran_sem_1.release(); });

EXPECT_NE(thread_0.get_id(), Id());
EXPECT_NE(thread_0.get_id(), Thread::id());
EXPECT_TRUE(thread_0.joinable());

EXPECT_NE(thread_1.get_id(), Id());
EXPECT_NE(thread_1.get_id(), Thread::id());
EXPECT_TRUE(thread_1.joinable());

thread_0.detach();
thread_1.detach();

EXPECT_EQ(thread_0.get_id(), Id());
EXPECT_EQ(thread_0.get_id(), Thread::id());
EXPECT_FALSE(thread_0.joinable());

EXPECT_EQ(thread_1.get_id(), Id());
EXPECT_EQ(thread_1.get_id(), Thread::id());
EXPECT_FALSE(thread_1.joinable());

thread_ran_sem_0.acquire();
Expand Down
Loading

0 comments on commit e5db91d

Please sign in to comment.