How to handle the removal of ICUB_USE_REALTIME_LINUX macro

[diegoferigo]

Trying to simplify the resolution of #1215, here I'm going to dissect only the ICUB_USE_REALTIME_LINUX discussion.

This macro is currently used in src/yarprobotinterface/main.cpp and no CMake option does exist in YARP. The reason is that this portion of code was moved a while ago from robotology/icub-main.

Following the issue referred above, all the other locations where this macro is used are in icub-main, where a CMake option does exist but it disabled by default.

The code wrapped by this macro is unix specific, so at this current state it cannot be enabled by default without breaking the build on non-unix platforms. As far as I understood, we agreed that this option should be turned on by default. In order to achieve this, I propose the following strategy:

• Create some static functions into yarp::os::impl::ThreadImpl that allow changing the policy & priority without the need of instantiating any object
• Remove the macro and substitute the unix-specific code with such functions
• Call these function also from within the ThreadImpl class, not breaking any API
• Evaluate if using RR or FIFO
• Start the discussion, better in Discussion about real time thread priorities #1215, of the most proper priority for such threads (ethReceiver, ethSender, yarprobotinterface, wholeBodyDynamicsDevice (?))

I had a quick look on the current status, but I have 0 knowledge about ACE and this is the main blocking factor from my point of view.

We can set a f2f meeting if needed @drdanz @marcoaccame @traversaro @lornat75

Btw, I did some quick test on iCub with and without this flag, and I didn't notice any difference. After a chat with @marcoaccame this could probably be related to the more powerful CPU that now iCub uses, and the bigger buffer of the sending / receiving threads.

[marcoaccame]

Hi @diegoferigo,

so far this code is required to run only on Linux.
Hence the quick-n-dirt way to begin things straight away could be to enable compilation of the existing non-portable code inside some

#if defined(macro-which-only-in-linux-is-defined) 

#endif

and then to proceed with enhancement of ThreadImpl etc. which can make the code more elegant.

[diegoferigo]

Doing that makes sense (and the macro is __unix__), however before proceeding we need at least a quick discussion on what priorities to use.

Keeping in mind that Kernel IRQs have a priority equal to 50, this is a possible strategy:

Thread	Current priority	Proposed priority
ethSender	48	48
ethReceiver	49	49
yarprobotinterface	33	30
wholeBodyDynamicsDevice	not RT	10

I explain better the ranges I chose for the proposed values in #1215 (comment).

cc @marcoaccame @drdanz @traversaro

[lornat75]

I think the RT support can be enabled by default only in linux builds, this can be easily achieved in CMake (we should change the name of the ICUB_USE_REALTIME_LINUX definition to something else (ROBOT_INTERFACE_USE_REALTIME_LINUX). I would keep the default only for yarprobotinterface.

Why not adding the static function for changing priority and scheduling policy to a public interface? this is done for example in yarp::os::Time or yarp::os::Network.

[diegoferigo]

@lornat75
Yes, I think that using static functions is the right way to go. However, it would require a not negligible amount of work and testing if we want to merge this before the upcoming feature freeze. @drdanz

[diegoferigo]

For the time being, with robotology/icub-main#453 I turned on by default the code wrapped by the ICUB_USE_REALTIME_LINUX macro in icub-main.

Doing this in the yarp side would require more work as explained above. For the next freeze I'd leave this as it is, and then slowly work on changing setPriority to a static method and handle yarprobotinterface.

[drdanz]

ICUB_USE_REALTIME_LINUX is no longer in YARP since f75e2c4 and has been disabled for years. Since nobody complained, I'm going to close this. Please reopen if you still consider this an issue.