Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Follow speed limits for slotcar robots #56

Closed
wants to merge 16 commits into from

Conversation

luca-della-vedova
Copy link
Member

@luca-della-vedova luca-della-vedova commented Oct 11, 2021

New feature implementation

Implemented feature

This PR allows simulated robots to respect speed limits specified in path requests, requires open-rmf/rmf_internal_msgs#28 and open-rmf/rmf_ros2#132

Implementation description

A new std::optional<double> max_speed is added to UpdateResult in slotcar_common, if a speed limit was specified in the message (specifically if it was > 0.0) the minimum between vehicle nominal speed and speed limit will be the navigation speed of the vehicle, before it was fixed at being the maximum robot speed at all times.

Since we need to store more information than a simple pose for differential drive robots' trajectories (i.e. speed limit), a new struct DiffDriveTrajectory was added and a minor refactoring took place to use it throughout the code.

ddengster and others added 6 commits September 29, 2021 16:47
Signed-off-by: ddengster <ed.fan@osrfoundation.org>
Signed-off-by: ddengster <ed.fan@osrfoundation.org>
Signed-off-by: Luca Della Vedova <luca@openrobotics.org>
Signed-off-by: Luca Della Vedova <luca@openrobotics.org>
Signed-off-by: Luca Della Vedova <luca@openrobotics.org>
Signed-off-by: Luca Della Vedova <luca@openrobotics.org>
@luca-della-vedova luca-della-vedova marked this pull request as ready for review October 13, 2021 09:26
@luca-della-vedova luca-della-vedova changed the title Follow speed limits for ackermann vehicles Follow speed limits for slotcar robots Oct 13, 2021
Signed-off-by: Luca Della Vedova <luca@openrobotics.org>
Signed-off-by: Luca Della Vedova <luca@openrobotics.org>
Signed-off-by: Luca Della Vedova <luca@openrobotics.org>
Copy link
Member

@aaronchongth aaronchongth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for adding this feature, @luca-della-vedova! I've taken a quick look, other than the comments, I also feel that we should revisit functions like calculate_control_signals and calculate_joint_control_signals in the future and remove the use of default arguments in the next major versions release, as those arguments are used wholly by our downstream packages so far.

rmf_robot_sim_common/src/slotcar_common.cpp Show resolved Hide resolved
rmf_robot_sim_common/src/slotcar_common.cpp Show resolved Hide resolved
rmf_robot_sim_common/src/slotcar_common.cpp Show resolved Hide resolved
rmf_robot_sim_common/src/slotcar_common.cpp Show resolved Hide resolved
rmf_robot_sim_common/src/slotcar_common.cpp Show resolved Hide resolved
rmf_robot_sim_common/src/slotcar_common.cpp Show resolved Hide resolved
Signed-off-by: Luca Della Vedova <luca@openrobotics.org>
Signed-off-by: Luca Della Vedova <luca@openrobotics.org>
@luca-della-vedova
Copy link
Member Author

Added checks for speed limit being greater than 0 02dc918.

I also feel that we should revisit functions like calculate_control_signals and calculate_joint_control_signals in the future and remove the use of default arguments in the next major versions release, as those arguments are used wholly by our downstream packages so far.

This can be done, so the idea is that all the arguments must be specified at all times instead of relying on default arguments? In that case I think some refactoring is also in order to reduce the number of input arguments to these functions, maybe by packing more motion parameters in structs

@codecov
Copy link

codecov bot commented Dec 1, 2021

Codecov Report

Merging #56 (425357a) into main (5d0f65a) will not change coverage.
The diff coverage is n/a.

@@          Coverage Diff           @@
##            main     #56    +/-   ##
======================================
  Coverage   0.00%   0.00%            
======================================
  Files        138     138            
  Lines      12845   12969   +124     
======================================
- Misses     12845   12969   +124     
Flag Coverage Δ
tests 0.00% <ø> (ø)

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

Impacted Files Coverage Δ
..._sim_common/include/rmf_robot_sim_common/utils.hpp
...lding_sim_ignition_plugins/src/crowd_simulator.cpp
...tion/rmf_robot_sim_common/src/dispenser_common.cpp
...uilding_sim_gazebo_plugins/src/crowd_simulator.cpp
...rmf_building_sim_common/crowd_simulator_common.hpp
...ation/rmf_building_sim_gazebo_plugins/src/lift.cpp
...ion/rmf_robot_sim_ignition_plugins/src/slotcar.cpp
...lation/rmf_building_sim_common/src/lift_common.cpp
...lation/rmf_building_sim_common/src/door_common.cpp
..._robot_sim_gazebo_plugins/src/TeleportIngestor.cpp
... and 254 more

@luca-della-vedova
Copy link
Member Author

Closed via #71

@luca-della-vedova luca-della-vedova deleted the feature/speed_limits branch April 7, 2022 01:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants