Debug navigation tests

[rosalievanark]

Bug fix of the navigation tests

Some hidden bugs had entered our code, and this PR has focused on solving these issues.

Original error: constant timeouts during the navigation tests. Examples of these tests:

• https://github.com/alliander-opensource/alliander-robotics/actions/runs/23732914827/job/69130699637
• https://github.com/alliander-opensource/alliander-robotics/actions/runs/23598528651/job/68736032974
• https://github.com/alliander-opensource/alliander-robotics/actions/runs/23602736236/job/68737245212

An important change here was to use assert instead of raise, both in terms of how a raise seems to crash all tests afterwards, as well as that a raise in a pytest does not guarantee that no more code of the test itself will be executed (for example, the assert after the raise was still triggered, which was not the desired behaviour). These raise statements are still all over the tests, it is better to start replacing these with assert statements (then the test workflow will still continue with testing instead of stopping immediately).

Additionally found error (see diff_drive_controller for origin error) in the gazebo container: Command message contains NaNs. Not updating reference interfaces.
This was solved using the twist_mux package as a filter between the drive_controller of the Husarion package, and the behavior_server, controller_server, and collision_monitor of the nav2 package. Apparently during startup in the Github CI, nav2 would send invalid cmd_vel messages to the drive_controller, getting stuck in the NaN error. Adding the twist_mux filter fixed it.

Incorrect time usage by gazebo: https://github.com/alliander-opensource/alliander-robotics/actions/runs/24189542218/job/70603291944?pr=420
Fixed by adding use_sim_time to the Husarion launch file.

Additional improvements:

• Add a login to our Docker account to the workflows for "unlimited" image pulling.
• Run each test on it's own ROS domain (and as a safety precaution, also wait before starting all containers for no nodes to be active to avoid "ghost nodes").
• Fix the pub_topic remapping in the nav2.launch.py file.

Testing

Explain how you tested your changes.

Documentation

• I have updated the documentation (if necessary)

Additional Notes

Any relevant screenshots, logs, or context.

[rosalievanark]

By default, these are the timeouts in ms:

default_server_timeout: 20
default_cancel_timeout: 50
wait_for_service_timeout: 1000

On github, this irregularly could cause the following error:

[panther.bt_navigator_navigate_to_pose_rclcpp_node]: Timed out while waiting for action server to acknowledge goal request for compute_path_to_pose

Which would then result in failing to navigate to the goal during the test.

Not sure whether the increase to 10s each is too extreme, but maybe we can also find a neat way to only use these increased values for the github CI?

[Jelmerdw]

I don't see a direct problem of having a 10s timeout. If this appears to be a problem when using hardware, we can make a change to use different timeouts for hardware and simulation.

[rosalievanark]

Do we want these parameters to be moved to its own yaml file? And would that then be placed in alliander_description?

[Jelmerdw]

For me it is fine to keep it like this for now. If we need to use the parameters add more places or change them often, we can start using a yaml file.

[rosalievanark]

Do we also need to add this to the controllers.launch.py file?

[Jelmerdw]

I think the SetParameter sets the parameter for all nodes, also nodes defined in the sub launch files included. So it should not be necessary to add this to the controllers.launch.py.

[Jelmerdw]

LGTM!

[Jelmerdw]

For me it is fine to keep it like this for now. If we need to use the parameters add more places or change them often, we can start using a yaml file.

[Jelmerdw]

I think the SetParameter sets the parameter for all nodes, also nodes defined in the sub launch files included. So it should not be necessary to add this to the controllers.launch.py.

[Jelmerdw]

I don't see a direct problem of having a 10s timeout. If this appears to be a problem when using hardware, we can make a change to use different timeouts for hardware and simulation.

[Jelmerdw]

I like the naming of cmd_vel_nav and 'cmd_vel_joy. Only cmd_vel_raw` feels odd now. But I am not sure if another name would clarify it...

cmd_vel_raw is actually the input for the collision monitor, so we could call it something like cmd_vel_cm_in. But again, this name can also be vague, especially due to the abbreviation of cm for collision monitor. Any other thoughts?

[geurto]

_raw is used more often in ROS context to indicate something that is fed into a different node for processing, so it feels okay to me. Could also use cmd_vel_in to indicate it will be used by another node

[Jelmerdw]

Do we want this rename also for GPS? So Navgiation -> GPSNavigation

[geurto]

Are all these additions still needed with the twist_mux fix? I guess it is also cleaner this way as we separate the environments more between tests. Are there any downsides to adding this, e.g. increased test time?