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

Add approach_speed field to Location message #28

Merged
merged 5 commits into from Nov 24, 2021
Merged

Conversation

luca-della-vedova
Copy link
Member

New feature implementation

Implemented feature

This PR allows simulated robots to respect speed limits. Speed limits for lanes are kept track of by upstream nodes (i.e. fleet adapters / managers) and published in the robot_path_requests message through a new field approach_speed. For each location approach_speed denotes the speed limit of the lane leading to it.

Signed-off-by: Luca Della Vedova <luca@openrobotics.org>
@codecov
Copy link

codecov bot commented Oct 11, 2021

Codecov Report

Merging #28 (b50f983) into main (71e039d) will not change coverage.
The diff coverage is n/a.

@@          Coverage Diff          @@
##            main     #28   +/-   ##
=====================================
  Coverage   0.00%   0.00%           
=====================================
  Files       1098    1098           
  Lines      66768   66836   +68     
=====================================
- Misses     66768   66836   +68     
Flag Coverage Δ
tests 0.00% <ø> (ø)

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

Impacted Files Coverage Δ
...sidl_generator_py/rmf_fleet_msgs/msg/_location_s.c 0.00% <0.00%> (ø)
..._c/rmf_fleet_msgs/msg/detail/location__functions.c 0.00% <0.00%> (ø)
...cpp/rmf_fleet_msgs/msg/detail/location__struct.hpp 0.00% <0.00%> (ø)
...fleet_msgs/msg/detail/location__type_support_c.cpp 0.00% <0.00%> (ø)
...rmf_fleet_msgs/msg/detail/location__type_support.c 0.00% <0.00%> (ø)
...f_fleet_msgs/msg/detail/location__type_support.cpp 0.00% <0.00%> (ø)
...msg/detail/dds_fastrtps/location__type_support.cpp 0.00% <0.00%> (ø)

Signed-off-by: Luca Della Vedova <luca@openrobotics.org>
Yadunund
Yadunund previously approved these changes Nov 17, 2021
rmf_fleet_msgs/msg/Location.msg Outdated Show resolved Hide resolved
@aaronchongth
Copy link
Member

On second thought, after seeing how it's used in open-rmf/rmf_ros2#132, would it be better to emulate an optional field with

bool obey_approach_speed_limit = false
float32 approach_speed_limit

This will prevent any ambiguity, floating point errors, and also allow us to check for errors easier down the line. Let me know what you think.

Signed-off-by: Luca Della Vedova <luca@openrobotics.org>
@luca-della-vedova
Copy link
Member Author

On second thought, after seeing how it's used in open-rmf/rmf_ros2#132, would it be better to emulate an optional field with

bool obey_approach_speed_limit = false
float32 approach_speed_limit

This will prevent any ambiguity, floating point errors, and also allow us to check for errors easier down the line. Let me know what you think.

Done 6d805f3
It seems booleans are initialized to false by default but I added it for extra clarity. Updated the matching PR in rmf_simulation to account for the change as well, didn't do the rmf_ros2 since it seems we might drop it and just have the new feature in the new fleet adapter

aaronchongth
aaronchongth previously approved these changes Nov 24, 2021
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.

This looks great! LGTM

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 the quick fix.

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.

None yet

4 participants