-
Notifications
You must be signed in to change notification settings - Fork 0
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
Adds base RoadNetwork/RoadGeometry Rust API. #32
Adds base RoadNetwork/RoadGeometry Rust API. #32
Conversation
Signed-off-by: Franco Cipollone <[email protected]>
@agalbachicar Note that this case for the RoadNetwork and RoadGeometry is probably not the one that is related to the F2F talk about rust vs cpp ownership and heap allocation
A different case happens with simpler structs/classes like LanePosition or InertialPosition. In those cases, having a cpp heap allocation sounds wrong and it is where I should narrow down the search for investigating that use case. |
maliput/data/xodr/TShapeRoad.xodr
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could bring the xodr files from the maliput_malidrive binary installation. However I do think that from the Rust API perspective makes sense to have XODR files in the package. Otherwise the source files might be hard to follow:
maliput(rust) -> maliput-sdk -> maliput(bazel) -> maliput (repo where the xodr files' src code are located)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm OK to simplify now, but with a TODO.
I think we should not repeat resources and expose a variable to installed resources through the bindings.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
maliput/examples/road_geometry.rs
Outdated
let mut properties = HashMap::new(); | ||
properties.insert("road_geometry_id", "my_rg_from_rust"); | ||
properties.insert("opendrive_file", xodr_path.as_str()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let mut properties = HashMap::new(); | |
properties.insert("road_geometry_id", "my_rg_from_rust"); | |
properties.insert("opendrive_file", xodr_path.as_str()); | |
let road_network_properties = HashMap::from([("road_geometry_id", "my_rg_from_rust"), ("opendrive_file", xodr_path.as_str())]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
maliput/examples/road_geometry.rs
Outdated
use maliput::api::RoadNetwork; | ||
use std::collections::HashMap; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can move these inside the main function, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
Signed-off-by: Franco Cipollone <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
🎉 New feature
Related to #30 #31
Summary
Adds Rust API for a minor part of the maliput::api::RoadNetwork/RoadGeometry (cpp) interface.
Checklist