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.