pw_thread_freertos: apply pw_thread_embos style changes
Change-Id: Ic85ef57b089e4f83d73e994c613b3b01d2be4107
Reviewed-on: https://pigweed-review.googlesource.com/c/pigweed/pigweed/+/45423
Reviewed-by: Keir Mierle <keir@google.com>
Pigweed-Auto-Submit: Ewout van Bekkum <ewout@google.com>
Commit-Queue: Auto-Submit <auto-submit@pigweed.google.com.iam.gserviceaccount.com>
diff --git a/pw_thread_freertos/BUILD b/pw_thread_freertos/BUILD
index 5b0895d..c36ba6d 100644
--- a/pw_thread_freertos/BUILD
+++ b/pw_thread_freertos/BUILD
@@ -96,6 +96,7 @@
deps = [
":id",
"//pw_assert",
+ "//pw_string",
"//pw_sync:binary_semaphore",
"//pw_thread:thread_headers",
],
diff --git a/pw_thread_freertos/BUILD.gn b/pw_thread_freertos/BUILD.gn
index 7071804..742a1e2 100644
--- a/pw_thread_freertos/BUILD.gn
+++ b/pw_thread_freertos/BUILD.gn
@@ -104,6 +104,7 @@
public_deps = [
":config",
"$dir_pw_assert",
+ "$dir_pw_string",
"$dir_pw_third_party/freertos",
"$dir_pw_thread:id",
"$dir_pw_thread:thread.facade",
diff --git a/pw_thread_freertos/public/pw_thread_freertos/config.h b/pw_thread_freertos/public/pw_thread_freertos/config.h
index 90c8c7a..b0494d4 100644
--- a/pw_thread_freertos/public/pw_thread_freertos/config.h
+++ b/pw_thread_freertos/public/pw_thread_freertos/config.h
@@ -18,8 +18,16 @@
#include "task.h"
// Whether thread joining is enabled. By default this is disabled.
-// When enabled this adds a StaticEventGroup_t & EventGroupHandle_t to
-// every pw::thread::Thread's context.
+//
+// We suggest only enabling this when thread joining is required to minimize
+// the RAM and ROM cost of threads.
+//
+// Enabling this grows the RAM footprint of every pw::thread::Thread as it adds
+// a StaticEventGroup_t to every thread's pw::thread::freertos::Context. In
+// addition, there is a minute ROM cost to construct and destroy this added
+// object.
+//
+// PW_THREAD_JOINING_ENABLED gets set to this value.
#ifndef PW_THREAD_FREERTOS_CONFIG_JOINING_ENABLED
#define PW_THREAD_FREERTOS_CONFIG_JOINING_ENABLED 0
#endif // PW_THREAD_FREERTOS_CONFIG_JOINING_ENABLED
@@ -47,7 +55,7 @@
// The default stack size in words. By default this uses the minimal FreeRTOS
// priority level above the idle priority.
#ifndef PW_THREAD_FREERTOS_CONFIG_DEFAULT_PRIORITY
-#define PW_THREAD_FREERTOS_CONFIG_DEFAULT_PRIORITY tskIDLE_PRIORITY + 1
+#define PW_THREAD_FREERTOS_CONFIG_DEFAULT_PRIORITY (tskIDLE_PRIORITY + 1)
#endif // PW_THREAD_FREERTOS_CONFIG_DEFAULT_PRIORITY
namespace pw::thread::freertos::config {
diff --git a/pw_thread_freertos/public/pw_thread_freertos/context.h b/pw_thread_freertos/public/pw_thread_freertos/context.h
index 71bf1d5..a1c0bc4 100644
--- a/pw_thread_freertos/public/pw_thread_freertos/context.h
+++ b/pw_thread_freertos/public/pw_thread_freertos/context.h
@@ -56,8 +56,8 @@
using ThreadRoutine = void (*)(void* arg);
void set_thread_routine(ThreadRoutine entry, void* arg) {
- entry_ = entry;
- arg_ = arg;
+ user_thread_entry_function_ = entry;
+ user_thread_entry_arg_ = arg;
}
bool detached() const { return detached_; }
@@ -75,12 +75,12 @@
StaticEventGroup_t& join_event_group() { return event_group_; }
#endif // PW_THREAD_JOINING_ENABLED
- static void RunThread(void* void_context_ptr);
+ static void ThreadEntryPoint(void* void_context_ptr);
static void TerminateThread(Context& context);
TaskHandle_t task_handle_ = nullptr;
- ThreadRoutine entry_ = nullptr;
- void* arg_ = nullptr;
+ ThreadRoutine user_thread_entry_function_ = nullptr;
+ void* user_thread_entry_arg_ = nullptr;
#if PW_THREAD_JOINING_ENABLED
// Note that the FreeRTOS life cycle of this event group is managed together
// with the task life cycle, not this object's life cycle.
diff --git a/pw_thread_freertos/thread.cc b/pw_thread_freertos/thread.cc
index 5f63291..7854e19 100644
--- a/pw_thread_freertos/thread.cc
+++ b/pw_thread_freertos/thread.cc
@@ -31,9 +31,11 @@
#endif // PW_THREAD_JOINING_ENABLED
} // namespace
-void Context::RunThread(void* void_context_ptr) {
+void Context::ThreadEntryPoint(void* void_context_ptr) {
Context& context = *static_cast<Context*>(void_context_ptr);
- context.entry_(context.arg_);
+
+ // Invoke the user's thread function. This may never return.
+ context.user_thread_entry_function_(context.user_thread_entry_arg_);
// Use a task only critical section to guard against join() and detach().
vTaskSuspendAll();
@@ -44,8 +46,9 @@
context.set_task_handle(nullptr);
#if PW_THREAD_JOINING_ENABLED
- // Just in case someone abused our API, ensure their use of the event group
- // is properly handled by the kernel regardless.
+ // If the thread handle was detached before the thread finished execution,
+ // i.e. got here, then we are responsible for cleaning up the join event
+ // group.
vEventGroupDelete(&context.join_event_group());
#endif // PW_THREAD_JOINING_ENABLED
@@ -133,7 +136,7 @@
// invoke the task with its arg.
native_type_->set_thread_routine(entry, arg);
const TaskHandle_t task_handle =
- xTaskCreateStatic(Context::RunThread,
+ xTaskCreateStatic(Context::ThreadEntryPoint,
options.name(),
options.static_context()->stack().size(),
native_type_,
@@ -164,7 +167,7 @@
// invoke the task with its arg.
native_type_->set_thread_routine(entry, arg);
TaskHandle_t task_handle;
- const BaseType_t result = xTaskCreate(Context::RunThread,
+ const BaseType_t result = xTaskCreate(Context::ThreadEntryPoint,
options.name(),
options.stack_size_words(),
native_type_,
@@ -197,12 +200,12 @@
#endif // INCLUDE_vTaskSuspend == 1
if (thread_done) {
- // The task finished (hit end of Context::RunThread) before we invoked
- // detach, clean up the thread.
+ // The task finished (hit end of Context::ThreadEntryPoint) before we
+ // invoked detach, clean up the thread.
Context::TerminateThread(*native_type_);
} else {
// We're detaching before the task finished, defer cleanup to the task at
- // the end of Context::RunThread.
+ // the end of Context::ThreadEntryPoint.
}
// Update to no longer represent a thread of execution.
