Pendulum fixes

[clalancette]

This PR, along with ros2/realtime_support#81 , should fix an issue in the pendulum_control demo where we publish uninitialized data. There are three fixes in here, corresponding to the 3 commits:

1. Remove some unused code in rtt_executor.hpp. This isn't strictly required here, but there is no reason for this code to be here since it is never used.
2. Switch the min_latency and max_latency fields in the RttestResults.msg to be signed integers. The underlying latency data coming from realtime_tools is signed, so this should be signed as well.
3. Make it so that we don't publish data if it hasn't yet been initialized.

With this in place, it looks to me like the uninitialized data problems that we have with pendulum_control tests should be gone. This may solve ros2/build_farmer#133 ; I'll see what happens when CI comes back. @cottsay FYI as current build farmer.

Fixes ros2/build_farmer#133
Fixes #386

[clalancette]

CI:

• Linux
• Linux-aarch64
• macOS
• Windows

[clalancette]

Given that this was failing on aarch64 every night for quite a while with a very similar configuration, I'd say this actually fixes it. Woohoo! I'll update the description.

[clalancette]

I had a silly bug in here, so here is another CI run:

• Linux
• Linux-aarch64
• macOS
• Windows

[clalancette]

And there was one more bug. Now I think I really have it fixed. Another CI to see if I'm correct:

• Linux
• Linux-aarch64
• macOS
• Windows

[dirk-thomas]

Remove some unused code in rtt_executor.hpp. This isn't strictly required here, but there is no reason for this code to be here since it is never used.

It would be good to keep changes like this in a separate PR. Simply because it makes reviewing each PR easier and each one can iterate and get merged independently. (Doesn't have to be for this change now but please do that in the future.)

Switch the min_latency and max_latency fields in the RttestResults.msg to be signed integers. The underlying latency data coming from realtime_tools is signed, so this should be signed as well.

How can the values be signed? If they conceptionally can't I would advocate to keep the unsigned in the message and do the necessary conversion (or consider updating realtime_tools).

Make it so that we don't publish data if it hasn't yet been initialized.

👍

[clalancette]

Switch the min_latency and max_latency fields in the RttestResults.msg to be signed integers. The underlying latency data coming from realtime_tools is signed, so this should be signed as well.

How can the values be signed? If they conceptionally can't I would advocate to keep the unsigned in the message and do the necessary conversion (or consider updating realtime_tools).

I struggled a bit with this. While I agree that latency should be an unsigned number, https://github.com/ros2/realtime_support/blob/master/rttest/src/rttest.cpp#L247 clearly wants to sometimes store it as a signed (which eventually gets assigned to {min,max}_latency). That was added in c5254392 (in 2015!), but it is not clear to me why.

[clalancette]

As we had previously discussed, I tried putting the {min,max}_latency fields back to uint64, and adding an assert to ensure the data coming from rttest is always >= 0. Under certain circumstances (Opensplice on Linux, with the machine heavily loaded), that assert sometimes triggers. I'm not entirely sure why that is; I'll have to do some additional debugging.

[clalancette]

I finally figured this out; it was an overflow. This now requires ros2/realtime_support#82 , and should now fix the pendulum failures for real. Here's some more CI for it:

• Linux
• Linux-aarch64
• macOS
• Windows

[jacobperron]

One comment below, but LGTM!