search

maliput / maliput_malidrive

Open-source ready OpenDrive backend for Maliput
BSD 3-Clause "New" or "Revised" License
0 stars 2 forks source link

Fixes ToLanePosition's bug. #212

Closed francocipollone closed 2 years ago

francocipollone commented 2 years ago

Context

A bug was found when calling the method Lane::ToLanePosition when passing a InertialPosition that lays off the Lane.

Replicable via: maliput_query --xodr_file_path=TShapeRoad.xodr --omit_nondrivable_lanes=true -- ToLanePosition 0_0_-1 15 2 0 --> This returned a LanePosition that was off the lane(with an r-coordinate that exceeded the lane boundaries)

Bug

The segment boundaries were being taken into account instead of the lane boundaries.

PR

The implementation was modified to used the lane boundaries as expected and tests were added to catch this case.

agalbachicar commented 2 years ago

The expectation is that the LanePosition is within length of the lane and the s coordinate is within segment_bounds at s (same for h and height_bounds). Look for the DoToLanePosition() method in dragway and multilane.

If the returned position is outside the lane, then we should review why. It should be within the segments though.

francocipollone commented 2 years ago

The expectation is that the LanePosition is within length of the lane and the s coordinate is within segment_bounds at s (same for h and height_bounds). Look for the DoToLanePosition() method in dragway and multilane.

If the returned position is outside the lane, then we should review why. It should be within the segments though.

Agree with your comment. However, what about the r coordinate?

agalbachicar commented 2 years ago

It should be within segment_bounds at s

El El mar, 3 de may. de 2022 a la(s) 14:57, Franco Cipollone < @.***> escribió:

The expectation is that the LanePosition is within length of the lane and the s coordinate is within segment_bounds at s (same for h and height_bounds). Look for the DoToLanePosition() method in dragway and multilane.

If the returned position is outside the lane, then we should review why. It should be within the segments though.

Agree with your comment. However, what about the r coordinate?

— Reply to this email directly, view it on GitHub https://github.com/ToyotaResearchInstitute/maliput_malidrive/pull/212#issuecomment-1116067919, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA5F6OL7O2IFMINIWW7BDV3VIEPFBANCNFSM5U5I75VQ . You are receiving this because your review was requested.Message ID: @.*** com>

-- Agustin Alba Chicar

francocipollone commented 2 years ago

After discussing it, the bug isn't a bug, it is just expected behavior. This evidenced that there was a lack of documentation on this method. See https://github.com/ToyotaResearchInstitute/maliput/issues/492

I will either close this PR or just add tests on those cases that weren't covered before. (Note that I changed the behavior and CI is passing.)

francocipollone commented 2 years ago

https://github.com/ToyotaResearchInstitute/maliput/issues/492