-
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 LaneEnd. #61
Adds LaneEnd. #61
Conversation
Signed-off-by: Franco Cipollone <[email protected]>
@@ -973,6 +973,15 @@ impl LaneSRange { | |||
} | |||
} | |||
|
|||
/// A specific endpoint of a specific Lane. | |||
/// This is analogous to the C++ maliput::api::LaneEnd implementation. | |||
pub enum LaneEnd<'a> { |
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 like this very much.
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.
Is it possible to constraint the creation such that no nullptr is used?
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.
No nullptr will be used.
Lane is a class whose underlying value is a reference.
The methods returning lanes from the underlying cpp implementation will evaluate whether that pointer is valid or not before creating the reference.
We can meet and discuss if needed.
Btw, In a follow up PR (where I add LaneEndSet) I am changing the &Lane by Lane. You will notice why.
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.
Just a nit.
let lane = road_geometry.get_lane(&lane_id); | ||
let lane_end_start = maliput::api::LaneEnd::Start(&lane); | ||
match lane_end_start { | ||
maliput::api::LaneEnd::Start(lane) => assert_eq!(lane.id(), lane_id), |
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.
Should use another name for lane
to avoid alising. Same below.
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.
Yeah makes sense. I am changing a bit this test in a follow-up PR. Let me address it in the next PR.
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.
🎉 New feature
#28 #17
Summary
Checklist