@bmagyar @erickisos @malapatiravi , I'll look to set some time aside this weekend to start taking a look at these changes.

Thanks @erickisos for opening this pull-request.

Let me check if is finished then, because I saw something about a bad comparison between vars

Hi @erickisos,

I am still getting up to speed with the codebase and the joint_trajectory_controller.cpp script as a result I have not as of yet completed my review but hope to do so in the coming days.

One note I came across was what looks like a typo in some of the variable names for instance here should read interface rather than inteface. While minor and somewhat inconsequential I'd be happy to open a separate pr to clean this up if you agree this looks like a simple naming typo.

Also it looks like certain tests/checks are currently failing as you mentioned above. Once I get up to speed I can take a look at these also.

Have a great start to the new week and hopefully we can get these changes merged by the end of the upcoming week.

The error you see on the CI has to do with the time we use in the tests. The different time source error appears when you are trying ot mix ROS time with system time or simulated time. Probably we don't use the right one to compare values in the tests.

@bmagyar should we open a separate issue to review the CI tests? I haven't completely finished my review but in order to not slow things down (and since a maintainer already approved this pr) I will also approve this pr.

Just in case, the inteface problem is solved now, I didn't check the problem with the RHEL verifications, but if you need help with that I can check that later on.

Ignore the RHEL issue, focus on this instead:

  >>>
- joint_trajectory_controller.OnlyVelocityTrajectoryControllers/TrajectoryControllerTestParameterized test_ignore_partial_old_trajectory/0
  <<< failure message
    unknown file
    C++ exception with description "can't compare times with different time sources" thrown in the test body.
  >>>
- joint_trajectory_controller.OnlyVelocityTrajectoryControllers/TrajectoryControllerTestParameterized test_ignore_partial_old_trajectory/1
  <<< failure message
    unknown file
    C++ exception with description "can't compare times with different time sources" thrown in the test body.

Thanks @bmagyar let me check

Looking good thanks!

P.S. Thanks for the typo correction. Probably have introduced it smile

Apologies I had exams so was away for a while, great to see this merged. Also happy to go through the codebase when I have the chance to, hopefully I'll be able to help out further at some point in the future when I'm more familiar with the codebase.