Skip to content

Add RPI Pico SDK mutex and locking to multithread #345

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 8 commits into
base: ros2
Choose a base branch
from
Open

Add RPI Pico SDK mutex and locking to multithread #345

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
bdaddba
Release v2.1.0
pablogs9 Nov 30, 2021
27b810a
Add Twitter and Readthedocs shields (#307) (#308)
mergify[bot] Jan 19, 2022
2195d43
Release v2.1.1
pablogs9 Mar 10, 2022
6f4edd2
Release v2.2.0
pablogs9 Jun 28, 2022
fda7242
Release v2.2.1
pablogs9 Sep 28, 2022
4b98d2d
Release v2.3.0
pablogs9 Dec 14, 2022
a0fd359
add pico sdk multi thread capability
DwayneGit Feb 22, 2023
8e44612
Merge remote-tracking branch 'eProsima/ros2' into ros2
DwayneGit Feb 22, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions include/uxr/client/profile/multithread/multithread.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ struct uxrSession;
#elif defined(PLATFORM_NAME_FREERTOS)
#include "FreeRTOS.h"
#include "semphr.h"
#elif defined(PLATFORM_NAME_RPIPICO)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where is PLATFORM_NAME_RPIPICO defined?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I gathered from here #233 that PLATFORM_NAME_FREERTOS was defined by the user so followed the same logic. I stuck add_compile_definitions(PLATFORM_NAME_RPIPICO) at the bottom of the toolchain.cmake file and it works however the its not detecting the "pico/mutex.h" so I am working on linking that.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You will need to add the include folders of Pico SDK on the CMake toolchain.

Copy link
Author

@DwayneGit DwayneGit Feb 23, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I am working on that. The problem is the sdk has all of its include directories scattered and one file is "version.h" is auto-generated and stored in the build folder. I gathered most of what i think is important and trying to compile now but running into some errors that i don't understand like:

/home/.../pico/pico-sdk/src/rp2040/hardware_structs/include/hardware/structs/timer.h:105:33: error: expected declaration specifiers or '...' before string constant
  105 | static_assert( NUM_TIMERS == 4, "");

include_directories(
    $ENV{PICO_SDK_PATH}/build/generated/pico_base
    $ENV{PICO_SDK_PATH}/src/rp2040/hardware_regs/include
    $ENV{PICO_SDK_PATH}/src/rp2040/hardware_structs/include
    $ENV{PICO_SDK_PATH}/src/rp2_common/hardware_timer/include
    $ENV{PICO_SDK_PATH}/src/rp2_common/hardware_base/include
    $ENV{PICO_SDK_PATH}/src/rp2_common/hardware_sync/include
    $ENV{PICO_SDK_PATH}/src/rp2_common/pico_platform/include
    $ENV{PICO_SDK_PATH}/src/common/pico_base/include
    $ENV{PICO_SDK_PATH}/src/common/pico_time/include
    $ENV{PICO_SDK_PATH}/src/common/pico_sync/include
)

If I comment that line out it works but I'm looking for a better solution than that

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like the problem is the code is being compiled with -std=c99 somewhere but the pico sdk needs c11 to compile with the static_assert function. I cloned the sdk repository branch and patched that and am able to build just fine.

Copy link
Author

@DwayneGit DwayneGit Feb 23, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually adding "-DUCLIENT_C_STANDARD=11" to the colcon.meta solved this problem for the microxrcedds_client but i get the same error in rmw_microxrcedds where it looks like the C_STANDARD is hard coded to 99. If this CMakeList file could change that static 99 to an argument with a default of 99 like UCLIENT_C_STANDARD here i think this would not be a problem.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, we will merge this asap

#include "pico/mutex.h"
#elif defined(UCLIENT_PLATFORM_ZEPHYR)
#elif defined(UCLIENT_PLATFORM_POSIX)
#include <pthread.h>
Expand All @@ -45,6 +47,8 @@ typedef struct uxrMutex
#elif defined(PLATFORM_NAME_FREERTOS)
SemaphoreHandle_t impl;
StaticSemaphore_t xMutexBuffer;
#elif defined(PLATFORM_NAME_RPIPICO)
recursive_mutex_t impl;
#elif defined(UCLIENT_PLATFORM_ZEPHYR)
struct k_mutex impl;
#elif defined(UCLIENT_PLATFORM_POSIX)
Expand Down
6 changes: 6 additions & 0 deletions src/c/profile/multithread/multithread.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ void uxr_init_lock(
{
#if defined(PLATFORM_NAME_FREERTOS)
mutex->impl = xSemaphoreCreateRecursiveMutexStatic( &mutex->xMutexBuffer );
#elif defined(PLATFORM_NAME_RPIPICO)
recursive_mutex_init(&mutex->impl);
#elif defined(UCLIENT_PLATFORM_ZEPHYR)
k_mutex_init(&mutex->impl);
#elif defined(UCLIENT_PLATFORM_POSIX)
Expand All @@ -29,6 +31,8 @@ void uxr_lock(
{
#if defined(PLATFORM_NAME_FREERTOS)
xSemaphoreTakeRecursive(mutex->impl, portMAX_DELAY);
#elif defined(PLATFORM_NAME_RPIPICO)
recursive_mutex_enter_blocking(&mutex->impl);
#elif defined(UCLIENT_PLATFORM_ZEPHYR)
k_mutex_lock(&mutex->impl, K_FOREVER);
#elif defined(UCLIENT_PLATFORM_POSIX)
Expand All @@ -43,6 +47,8 @@ void uxr_unlock(
{
#if defined(PLATFORM_NAME_FREERTOS)
xSemaphoreGiveRecursive(mutex->impl);
#elif defined(PLATFORM_NAME_RPIPICO)
recursive_mutex_exit(&mutex->impl);
#elif defined(UCLIENT_PLATFORM_ZEPHYR)
k_mutex_unlock(&mutex->impl);
#elif defined(UCLIENT_PLATFORM_POSIX)
Expand Down