Add realtime priority inheritance mutexes

[saikishor]

Add realtime priority inheritance mutexes for using within the realtime loops

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 46.42857% with 30 lines in your changes missing coverage. Please review.

Project coverage is 69.82%. Comparing base (97f9f41) to head (c5250a1).

Files with missing lines	Patch %	Lines
include/realtime_tools/mutex.hpp	46.42%	19 Missing and 11 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #197      +/-   ##
==========================================
- Coverage   73.11%   69.82%   -3.30%     
==========================================
  Files           8        9       +1     
  Lines         398      454      +56     
  Branches       65       77      +12     
==========================================
+ Hits          291      317      +26     
- Misses         68       87      +19     
- Partials       39       50      +11     

Flag	Coverage Δ
unittests	69.82% <46.42%> (-3.30%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
include/realtime_tools/mutex.hpp	46.42% <46.42%> (ø)

---- 🚨 Try these New Features:

• Flaky Tests Detection - Detect and resolve failed and flaky tests

[atzaros]

I would replace plain ints for class enums where apply.
These enums should be located inside the sub namespace detail

[atzaros]

I would expose the template parameters in the class itself because it is quite handy to check for them in generic code.
I mean something like:

namespace ... {
class enum mutex_type_t { ... };
namespace detail {
  class mutex<mutex_type_t MutexType, ...> { constexpr auto type = MutexType; ... }
} // detail
} // ...

[atzaros]

No logging through cerr, I would use any logging mechanism in the framework or do nothing.

[saikishor]

Can you please explain why std::cerr cannot be used here?

[atzaros]

Because, as a system integrator using your stuff, I would like to configure, redirect and filter the logs of my system as it is documented in the framework.

Maybe using something like RCLCPP_FATAL(rclcpp::get_logger("ros_control_logger"), msg); ?

[atzaros]

as explained aside, if your concern is to be more ROS-agnostic then rely on weak symbols, in C parlance, extern declarations.

extern "C" {
extern int logAsError(int line, const char* file, const char* msg);
extern int logAsInfo(int line, const char* file, const char* msg);
}

// in ros-control
#define LOG_ERROR(msg) do { (void)logAsError(__LINE__, __FILE__, (msg)); } while(0);
...

and then the one generating the executable must implement them to prevent a linker error, for instance:

// in a ROS node project linking with ros-control...
// my_log_as_handlers.cpp
extern "C" {
int logAsError(int line, const char* file, const char* msg) {
  ROS_LOG_ERROR(...);
}
int logAsInfo(int line, const char* file, const char* msg);
  ROS_LOG_INFO(...);
}
}

[atzaros]

If your intention is to implement a general wrapper for POSIX mutexes, this review is going to take time...

I would suggest to decrease the scope of the endeavor and just implement the PTHREAD_PRIO_INHERIT.

That means:

• rephrase the documentation
• prevent users to instantiate non-PRIO_INHERIT mutexes (easy if you use class enums as suggested below)

Additionally I think it is not good idea to support some combinations ever (i.e. PTHREAD_MUTEX_NORMAL + PTHREAD_PRIO_NONE) because they are already implemented in the STL [1]

[1] I am a bit nervous here when trying to understand which c++ mutexes conform to the POSIX standard:

• A c++ mutex
• pthread_mutexattr_getprotocol: The default value of the attribute shall be PTHREAD_PRIO_NONE
• PTHREAD_MUTEX_DEFAULT: The default mutex type. May be mapped to any of the above mutex types or may be an implementation-defined type

[saikishor]

I agree with you, I will reduce the scope. Thanks for pointing out

Regarding to the PTHREAD_MUTEX_DEFAULT, I choose it because of the explanation given here : https://linux.die.net/man/3/pthread_mutexattr_settype

[atzaros]

as explained aside, if your concern is to be more ROS-agnostic then rely on weak symbols, in C parlance, extern declarations.

extern "C" {
extern int logAsError(int line, const char* file, const char* msg);
extern int logAsInfo(int line, const char* file, const char* msg);
}

// in ros-control
#define LOG_ERROR(msg) do { (void)logAsError(__LINE__, __FILE__, (msg)); } while(0);
...

and then the one generating the executable must implement them to prevent a linker error, for instance:

// in a ROS node project linking with ros-control...
// my_log_as_handlers.cpp
extern "C" {
int logAsError(int line, const char* file, const char* msg) {
  ROS_LOG_ERROR(...);
}
int logAsInfo(int line, const char* file, const char* msg);
  ROS_LOG_INFO(...);
}
}

[atzaros]

build vs run-time errors:

static_assert to prevent unsupported instantiations like detail::mutex<83, 29>

[saikishor]

ok

[atzaros]

testing a dead-lock would be great!