Skip to content
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.

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

[Core]: Add support for CMSIS RTOS 2 Thread synchronization #458

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
11 changes: 11 additions & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,17 @@ if(NOT CAN_STACK_DISABLE_THREADS AND NOT ARDUINO)
endif()
endif()

option(
USE_CMSIS_RTOS2_THREADING
"Set to ON to use ARM CMSIS RTOS2 thread syncronization. Replaces std::mutex and implements a CMSIS lock_guard."
OFF)
if(USE_CMSIS_RTOS2_THREADING)
message(
AUTHOR_WARNING
"Using CMSIS RTOS2 threading requires you to implement a hardware timebase (_gettimeofday) for the stack. Make sure you do this using a hardware timer!"
)
endif()

# A handy function to prepend text to all elements in a list (useful for
# subdirectories)
function(prepend var prefix)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -146,9 +146,13 @@ namespace isobus
void stop_threads();

/// @brief The receiving thread loop for this CAN channel
#ifdef USE_CMSIS_RTOS2_THREADING
static void receive_thread_function(void *parent);
#else
void receive_thread_function();
#endif
Comment on lines +149 to +153
Copy link
Member

Choose a reason for hiding this comment

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

Hmm, since we'll be limited by C now I'd suggest we just refactor the old function instead of duplicating it. So we use the C void * pattern again which we previously refactored away from I believe 😆


std::unique_ptr<std::thread> receiveMessageThread; ///< Thread to manage getting messages from a CAN channel
std::unique_ptr<Thread> receiveMessageThread; ///< Thread to manage getting messages from a CAN channel
bool receiveThreadRunning = false; ///< Flag to indicate if the receive thread is running
#endif

Expand All @@ -170,15 +174,19 @@ namespace isobus
virtual ~CANHardwareInterface();

/// @brief The main thread loop for updating the stack
#if defined USE_CMSIS_RTOS2_THREADING
static void update_thread_function(void *);
#else
static void update_thread_function();
#endif
Comment on lines +177 to +181
Copy link
Member

Choose a reason for hiding this comment

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

Same with the duplication here


/// @brief Starts all threads related to the hardware interface
static void start_threads();

/// @brief Stops all threads related to the hardware interface
static void stop_threads();

static std::unique_ptr<std::thread> updateThread; ///< The main thread
static std::unique_ptr<Thread> updateThread; ///< The main thread
static std::condition_variable updateThreadWakeupCondition; ///< A condition variable to allow for signaling the `updateThread` to wakeup
#endif
static std::uint32_t lastUpdateTimestamp; ///< The last time the network manager was updated
Expand Down
57 changes: 54 additions & 3 deletions hardware_integration/src/can_hardware_interface.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
namespace isobus
{
#if !defined CAN_STACK_DISABLE_THREADS && !defined ARDUINO
std::unique_ptr<std::thread> CANHardwareInterface::updateThread;
std::unique_ptr<Thread> CANHardwareInterface::updateThread;
std::condition_variable CANHardwareInterface::updateThreadWakeupCondition;
#endif
std::uint32_t CANHardwareInterface::periodicUpdateInterval = PERIODIC_UPDATE_INTERVAL;
Expand Down Expand Up @@ -112,7 +112,11 @@ namespace isobus
receiveThreadRunning = true;
if (nullptr == receiveMessageThread)
{
receiveMessageThread.reset(new std::thread([this]() { receive_thread_function(); }));
#ifdef USE_CMSIS_RTOS2_THREADING
receiveMessageThread.reset(new Thread(receive_thread_function, this));
#else
receiveMessageThread.reset(new Thread([this]() { receive_thread_function(); }));
#endif
}
}

Expand All @@ -129,6 +133,31 @@ namespace isobus
}
}

#ifdef USE_CMSIS_RTOS2_THREADING
void CANHardwareInterface::CANHardware::receive_thread_function(void *parent)
{
auto target = static_cast<CANHardware *>(parent);
while (target->receiveThreadRunning)
{
if ((nullptr != target->frameHandler) && target->frameHandler->get_is_valid())
{
if (!target->receive_can_frame())
{
// There was no frame to receive, and osThreadYield may not exist
osDelay(1);
Copy link
Member

Choose a reason for hiding this comment

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

nice-to-have (non-blocking): I guess we can make generalize the thread "delay / sleep / yield" functions in the future

}
else
{
CANHardwareInterface::updateThreadWakeupCondition.notify_all();
}
}
else
{
osDelay(1000); // Arbitrary, but don't want to infinite loop on the validity check.
}
}
}
#else
void CANHardwareInterface::CANHardware::receive_thread_function()
{
while (receiveThreadRunning)
Expand All @@ -151,6 +180,7 @@ namespace isobus
}
}
}
#endif
#endif

bool send_can_message_frame_to_hardware(const CANMessageFrame &frame)
Expand Down Expand Up @@ -407,6 +437,22 @@ namespace isobus
}

#if !defined CAN_STACK_DISABLE_THREADS && !defined ARDUINO

#if defined USE_CMSIS_RTOS2_THREADING
void CANHardwareInterface::update_thread_function(void *)
{
hardwareChannelsMutex.lock();
// Wait until everything is running
hardwareChannelsMutex.unlock();

while (started)
{
LOCK_GUARD(Mutex, updateMutex);
updateThreadWakeupCondition.wait_for(updateMutex, periodicUpdateInterval); // Update with at least the periodic interval
update();
}
}
#else
void CANHardwareInterface::update_thread_function()
{
std::unique_lock<std::mutex> hardwareLock(hardwareChannelsMutex);
Expand All @@ -420,13 +466,18 @@ namespace isobus
update();
}
}
#endif

void CANHardwareInterface::start_threads()
{
started = true;
if (nullptr == updateThread)
{
updateThread.reset(new std::thread(update_thread_function));
#ifdef USE_CMSIS_RTOS2_THREADING
updateThread.reset(new Thread(update_thread_function, nullptr));
#else
updateThread.reset(new Thread(update_thread_function));
#endif
}
}

Expand Down
2 changes: 1 addition & 1 deletion isobus/src/isobus_task_controller_client.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ namespace isobus

if (!initialized)
{
#if !defined CAN_STACK_DISABLE_THREADS && !defined ARDUINO
#if !defined CAN_STACK_DISABLE_THREADS && !defined ARDUINO && !defined USE_CMSIS_RTOS2_THREADING
if (spawnThread)
{
workerThread = new std::thread([this]() { worker_thread_function(); });
Expand Down
2 changes: 1 addition & 1 deletion isobus/src/isobus_virtual_terminal_client.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ namespace isobus
{
languageCommandInterface.initialize();
}
#if !defined CAN_STACK_DISABLE_THREADS && !defined ARDUINO
#if !defined CAN_STACK_DISABLE_THREADS && !defined ARDUINO && !defined USE_CMSIS_RTOS2_THREADING
if (spawnThread)
{
workerThread = new std::thread([this]() { worker_thread_function(); });
Expand Down
Loading
Loading