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 lane speed limit to graph parsing function #124

Merged
merged 1 commit into from
Sep 28, 2021

Conversation

luca-della-vedova
Copy link
Member

@luca-della-vedova luca-della-vedova commented Sep 21, 2021

New feature implementation

Implemented feature

Depends on open-rmf/rmf_traffic#44
This PR is to work towards open-rmf/rmf_traffic#43, it adds parsing the speed_limit from lane properties and assigning them to the rmf_traffic lanes.

Implementation description

The implementation is very simple but it cannot be really tested unless we add a unit test with a sample graph, in alternative we might need to address the remaining repos in the linked issue to get an end to end test in rmf_demos.

@codecov
Copy link

codecov bot commented Sep 23, 2021

Codecov Report

Merging #124 (df39b5d) into main (87ce942) will increase coverage by 0.05%.
The diff coverage is n/a.

❗ Current head df39b5d differs from pull request most recent head 2c82e3b. Consider uploading reports for the commit 2c82e3b to get more accurate results

@@            Coverage Diff             @@
##             main     #124      +/-   ##
==========================================
+ Coverage   21.40%   21.45%   +0.05%     
==========================================
  Files        1260      840     -420     
  Lines      101676    67832   -33844     
  Branches    48378    32284   -16094     
==========================================
- Hits        21760    14551    -7209     
+ Misses      56505    37612   -18893     
+ Partials    23411    15669    -7742     
Flag Coverage Δ
tests 21.45% <ø> (+0.05%) ⬆️

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

Impacted Files Coverage Δ
...2/src/rmf_traffic_ros2/schedule/convert_Writer.cpp
...pp/RxCpp-4.1.0/Rx/v2/src/rxcpp/rx-notification.hpp
...rmf_fleet_adapter_python/src/schedule/schedule.cpp
...rc/rmf_fleet_adapter/jobs/detail/impl_Planning.hpp
...RxCpp-4.1.0/Rx/v2/src/rxcpp/sources/rx-iterate.hpp
...mf_ros2/rmf_fleet_adapter/test/tasks/test_Loop.cpp
...f_fleet_adapter/agv/internal_FleetUpdateHandle.hpp
...raffic_ros2/src/rmf_traffic_ros2/blockade/Node.cpp
...ffic_ros2/src/rmf_traffic_ros2/schedule/Writer.cpp
..._adapter/src/rmf_fleet_adapter/make_trajectory.cpp
... and 2090 more

@luca-della-vedova luca-della-vedova marked this pull request as ready for review September 24, 2021 05:11
@luca-della-vedova
Copy link
Member Author

Python bindings and their tests have been updated, this is now ready for review.
Test failures on rmf_fleet_adapter are pre-existing.

mxgrey
mxgrey previously approved these changes Sep 27, 2021
Copy link
Contributor

@mxgrey mxgrey 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 also updating the Python bindings!

@mxgrey
Copy link
Contributor

mxgrey commented Sep 27, 2021

I'm not sure why there's a problem, but GitHub is complaining about unsigned commits. Maybe you can do a squash-merge with signing to fix it?

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

Done! I squash merged the three commits and force pushed into a single signed commit

@mxgrey mxgrey merged commit 78e85e6 into main Sep 28, 2021
@mxgrey mxgrey deleted the feature/parse_speed_limit branch September 28, 2021 12:58
Research & Development automation moved this from In Review to Done Sep 28, 2021
arjo129 pushed a commit that referenced this pull request Oct 12, 2021
Signed-off-by: Luca Della Vedova <luca@openrobotics.org>
Signed-off-by: Arjo Chakravarty <arjo@openrobotics.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants