Support nonholonomic movement for slotcar

[ddengster]

New feature implementation

Implemented feature

Support nonholonomic movement for slotcar

Implementation description

Robots using the slotcar plugin can now attempt to follow a path made by nonholonomic robots via publishing to /ackmann_path_requests and adding the tag <holonomic>0</holonomic> to their sdf model files. An example script and level can be used to test in this PR open-rmf/rmf_demos#70.

Currently the nonholonomic movement update code does not consider

• battery usage (not needed yet)
• turning accelerations (due to bugs with the rate of change code)

[codecov]

Codecov Report

Merging #33 (b0926bf) into main (0455e7a) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff            @@
##            main     #33      +/-   ##
========================================
  Coverage   0.00%   0.00%              
========================================
  Files        222      94     -128     
  Lines      18659    8066   -10593     
========================================
+ Misses     18659    8066   -10593     

Flag	Coverage Δ
tests	0.00% <ø> (ø)

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

Impacted Files	Coverage Δ
...lation/rmf_robot_sim_common/src/slotcar_common.cpp
...uilding_sim_gazebo_plugins/src/crowd_simulator.hpp
...uilding_sim_gazebo_plugins/src/crowd_simulator.hpp
...tion/rmf_robot_sim_common/src/dispenser_common.cpp
...uilding_sim_gazebo_plugins/src/crowd_simulator.cpp
...tion/rmf_robot_sim_gazebo_plugins/src/readonly.cpp
...ation/rmf_robot_sim_common/src/readonly_common.cpp
...rmf_building_sim_common/crowd_simulator_common.hpp
...uilding_sim_gazebo_plugins/src/crowd_simulator.cpp
...uilding_sim_gazebo_plugins/src/crowd_simulator.hpp
... and 302 more

[luca-della-vedova]

Before I look too much into the code I can see from a quick glance that mostly new functions are used (i.e. the ackermann topic callback, an update specifically for non holonomic vehicles), and not too much of the slotcar pre-existing code.
Hence I wonder if it would be cleaner / easier to maintain (avoiding slotcar becoming even more complex) if the ackermann vehicles existed in a separate plugin, or do you think there is enough shared logic between ackermann drives and normal slotcar robots to justify the code being part of the slotcar plugin?

[ddengster]

There were some code like movement, battery usage, etc and I thought the potential of reusing them would be a good thing, however the movement code didn't turn out to be that suited. Still I think it can work out correctly once we've solved the bugs. The battery code is another part that would be useful to have in the future.

Still I think we can stick to this arrangement - I don't feel a new movement and path construction function warrants a new plugin.

[luca-della-vedova]

Finished a first quick round of review, I think there is a bit of cleanup (there is quite a bit of commented code / unused variables scattered around), as well as documentation to help follow the logic better that can be done.

[luca-della-vedova]

Nit, typo

[ddengster]

what's the typo? last?

[luca-della-vedova]

it's -> its