-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Route Tool Rviz Panel #4775
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
Route Tool Rviz Panel #4775
Conversation
@john-chrosniak, all pull requests must be targeted towards the |
a1f5337
to
d107248
Compare
This pull request is in conflict. Could you fix it @john-chrosniak? |
b68157b
to
813d7fe
Compare
This pull request is in conflict. Could you fix it @john-chrosniak? |
60521ab
to
752caf0
Compare
This pull request is in conflict. Could you fix it @john-chrosniak? |
7f803b1
to
3bbb1bd
Compare
This pull request is in conflict. Could you fix it @john-chrosniak? |
d196e24
to
9127d4f
Compare
Question before I review on Friday: I noticed that you didn't modify the original server to have a service in the Route Server itself to save the graph to file. If we did that, we'd probably have to create services to also modify the graph by adding / removing a edge / node but then there's less logic that's happening in the rviz program itself that isn't portable to another GUI window. Can you say a little bit about why you did this all inline in the rviz tool? I assume you modify a bunch of stuff inline that is easier with direct access. I just want that in in mind when giving this a critical look Also: Does this dumping to file also do the |
Signed-off-by: John Chrosniak <[email protected]>
Signed-off-by: John Chrosniak <[email protected]>
Signed-off-by: John Chrosniak <[email protected]>
Signed-off-by: John Chrosniak <[email protected]>
…e is moved Signed-off-by: John Chrosniak <[email protected]>
Signed-off-by: John Chrosniak <[email protected]>
Signed-off-by: John Chrosniak <[email protected]>
Signed-off-by: John Chrosniak <[email protected]>
Signed-off-by: John Chrosniak <[email protected]>
Signed-off-by: John Chrosniak <[email protected]>
Signed-off-by: John Chrosniak <[email protected]>
Signed-off-by: John Chrosniak <[email protected]>
Signed-off-by: John Chrosniak <[email protected]>
Signed-off-by: John Chrosniak <[email protected]>
Signed-off-by: John Chrosniak <[email protected]>
Signed-off-by: John Chrosniak <[email protected]>
Signed-off-by: John Chrosniak <[email protected]>
Signed-off-by: John Chrosniak <[email protected]>
Signed-off-by: John Chrosniak <[email protected]>
Signed-off-by: John Chrosniak <[email protected]>
Signed-off-by: John Chrosniak <[email protected]>
ab7cf87
to
a33d1c3
Compare
My assumption was that route graphs are almost always going to be defined ahead of time, so there wasn't a need to support editing/saving graphs on the fly using the route server. The data structures used in the route tool class are only needed to keep track of nodes/edges as new elements are added/edited/removed, so if we're only modifying route graphs through this UI then this logic should be specific to the route tool.
Yes both node and edge metadata is saved. |
nav2_route/src/plugins/graph_file_savers/geojson_graph_file_saver.cpp
Outdated
Show resolved
Hide resolved
nav2_route/src/plugins/graph_file_savers/geojson_graph_file_saver.cpp
Outdated
Show resolved
Hide resolved
nav2_route/src/plugins/graph_file_savers/geojson_graph_file_saver.cpp
Outdated
Show resolved
Hide resolved
nav2_route/src/plugins/graph_file_savers/geojson_graph_file_saver.cpp
Outdated
Show resolved
Hide resolved
Codecov ReportAttention: Patch coverage is
... and 5 files with indirect coverage changes 🚀 New features to boost your workflow:
|
Signed-off-by: John Chrosniak <[email protected]>
Co-authored-by: Steve Macenski <[email protected]> Signed-off-by: John Chrosniak <[email protected]>
…ver.cpp Co-authored-by: Steve Macenski <[email protected]> Signed-off-by: John Chrosniak <[email protected]>
…ver.cpp Co-authored-by: Steve Macenski <[email protected]> Signed-off-by: John Chrosniak <[email protected]>
…ver.cpp Co-authored-by: Steve Macenski <[email protected]> Signed-off-by: John Chrosniak <[email protected]>
…ver.cpp Co-authored-by: Steve Macenski <[email protected]> Signed-off-by: John Chrosniak <[email protected]>
Signed-off-by: John Chrosniak <[email protected]>
…igation2 into route-server-develop
Just waiting on CI to merge! I announced on discourse, linkedin and other places for beta testers for the next 2 weeks so hopefully we'll start getting some big users of this work starting soon! |
Awesome!!! I'm happy to help out with any fixes/maintenance needed as beta testers are brought on! |
33f4015
into
ros-navigation:nav2_route_server
Great, I appreciate it! |
Basic Info
Description of contribution in a few bullet points
Description of documentation updates required from your changes
Future work that may be required in bullet points
For Maintainers: