Closed saikishor closed 3 weeks ago
Attention: Patch coverage is 90.90909%
with 1 line
in your changes missing coverage. Please review.
Project coverage is 72.97%. Comparing base (
45e93d7
) to head (593dc27
).
Files with missing lines | Patch % | Lines |
---|---|---|
src/realtime_helpers.cpp | 90.90% | 0 Missing and 1 partial :warning: |
@firesurfer can you check this once?
In general this looks good to me with a minor notes: https://en.cppreference.com/w/cpp/thread/thread/native_handle native_handle is only available for systems with a posix threading model. I am not sure if there might be some weird combination of systems (apart from windows) where it is not available and might lead to compilation errors
Most of the systems now supports. The following information, I got it using Copilot
Systems that support a POSIX threading model (also known as POSIX threads or Pthreads) include most Unix-like operating systems. POSIX threads are a standard for thread creation and synchronization, defined by the POSIX.1c standard (IEEE Std 1003.1c-1995). Here are some systems that support POSIX threads:
Unix-like Operating Systems
Other Operating Systems
As this is the case, I think this should be good to go
This PR aims to change the earlier approach with
pthread_setaffinity_np
as it is more generic and it can be scalable to all other controllers.The good thing is it doesn't affect the recently merged changes : https://github.com/ros-controls/ros2_control/pull/1852
I've also added corresponding tests in different places