New lane model (Handling Junctions with OSI::GroundTruth::Lane - issue #80) #110

carsten-kuebler · 2018-01-17T14:01:19Z

According to the OSI workshop in Ulm (November 2017), I propose this new lane model and the corresponding OSI messages. Change break downward compatibility.

pmai · 2018-01-17T15:38:00Z

This looks good to me at first glance, but I think it would be good to get feedback from the other workshop participants, e.g. @HendrikAmelunxen, @AndreHildebrandt, @MHerrmannIPG, @JonathanSchmalhoferBMW, or @AndreasVIRES if this matches what they expect.

pmai

This is just a review of the technical changes needed to retain technical backward compatibility; this can be ignored to a certain degree if we decide to switch to V3.0.0 release, which might make sense since even if we stay technically backward compatible, I do not think that the changes to the lane model makes new code usable with old data or vice versa...

pmai · 2018-01-18T11:57:36Z

osi_lane.proto

-    // also be additional free lanes.
-    repeated LaneBoundary lane_boundary = 4;
+    // \note CenterlineIsDrivingDirection is defined for \c Lane::type = TYPE_DRIVING.
+    optional bool center_line_is_driving_direction = 4;


To maintain basic backward compatibility field tag 4 should be reserved and center_line_is_driving_direction should get a new tag (after existing ones).

pmai · 2018-01-18T11:58:23Z

osi_lane.proto

+
+    // The id of the free boundary ID.
    //
-    optional LaneFraming lane_framing = 9;


lane_framing tag number should be reserved, and not reused.

pmai · 2018-01-18T12:01:59Z

osi_lane.proto


        // A normal lane.
        // E.g. lanes with ids 1, 2, 3, 4 and 7 of the highway_exit image.
-        TYPE_NORMAL = 2;


To retain backward compatiblity TYPE_NORMAL can be changed to TYPE_DRIVING, I think, but the new TYPE_NONDRIVING and TYPE_INTERSECTION enum values should be new unused values to avoid misinterpretation, I think...

pmai · 2018-01-18T12:04:02Z

osi_lane.proto

 //
 message LaneBoundary
 {
+    //The id of the lane boundary.


Again, if we want to technically remain backward compatible, the id field need to use a new tag number and the remaining fields should retain their tag numbers.

pmai · 2018-01-18T12:06:47Z

@CarloVanDriestenBMW I think we need to decide soonish whether it makes more sense to release as 3.0.0 rather than 2.3.0 given the changes to the lane model (and some other changes)...

ghost · 2018-01-18T14:43:39Z

I am starting to think that the version change to 3.0.0 is reflecting the now aligned development with VDA in a better way -> and yes it may not be backward compatible

pmai · 2018-01-26T14:37:54Z

BTW, I think this changeset is still missing the predecessor/successor pairings that @CarloVanDriestenBMW suggested to better express valid ways to cross a junction, right?

carsten-kuebler · 2018-01-29T13:04:44Z

@pmai no, there will be no pairings for junctions. This decision was taken at OSI workshop in Ulm (2017). Valid ways depend on road markings, traffic signs linked to predecessor lanes and road markings and free lane boundaries of the junction. For this purpose, junctions have no center lines...

pmai · 2018-01-29T14:57:38Z

Yes the Ulm consensous was to throw out the relationships for junctions. @CarloVanDriestenBMW proposed switching from separate predecessor/successor lists to a new combined pair list afterwards, to enable the logical possibilities to be still represented after the Workshop. Just wanted to ensure that it was clear whether this changeset included that or not. Thanks, will merge as soon as someone from the Environment Simulation side has reviewed.

carsten-kuebler · 2018-01-31T19:04:21Z

we add the lane pairing to OSI::Lanes. We have to improve the documentation with this image.

So for lane 6 the lane pairing will be (4,7) and (4,8).
(In the previous version of the image lane 6 will be only the right adjacent lane.)

@pmai and @CarloVanDriestenBMW: PR now with lane pairing.

carsten-kuebler · 2018-02-01T08:15:55Z

In this case lane 4 has two right adjacent lanes (lane 5 and 6). 6 is no successor lane of lane 4. @pmai and @CarloVanDriestenBMW do you agree?

Restructuring the lane model: Lane boundaries are seperated from the lanes and only referenced in lanes.

Logical definition for possible lane pairings, improved logic for intersections.

pmai

The proposed changes look good to me, and are in line with the Ulm Workshop findings, will merge with cleanup.

…e-model New detected lane model (related to Handling Junctions with OSI::GroundTruth::Lane - issue #80 and pull request #110)

solosito

I added some comments that, in my opinion, are making it more understandable.

solosito · 2018-02-08T17:21:30Z

osi_groundtruth.proto

    //
    repeated Lane lane = 9;

+    // A list of lane boundaries from lanes.


In my opinion this is redundant. Just A list of lane boundaries. would be enough.

solosito · 2018-02-08T17:31:46Z

osi_landmark.proto

-        
+
        // Marking with violet color.
+        //


Suggestion: the same typo might be fixed in the line 58

solosito · 2018-02-08T17:47:48Z

osi_lane.proto

-    // might be used e.g. in construction sites. The lane markings do not have to be parallel to the center_line, but
-    // they are the basis for calculating the width of the lane at any given distance.
+    // Defined and used for driving lanes.
+    // true means driving direction is according to ascending order of center line points.


true means driving direction agrees with ascending storage order of the points defining the center_line.

solosito · 2018-02-08T17:47:53Z

osi_lane.proto

-    // they are the basis for calculating the width of the lane at any given distance.
+    // Defined and used for driving lanes.
+    // true means driving direction is according to ascending order of center line points.
+    // false means driving direction is according to descending order of center line points.


false means driving direction disagrees with descending storage order of the points defining the center_line.

carsten-kuebler mentioned this pull request Jan 17, 2018

New detected lane model (related to Handling Junctions with OSI::GroundTruth::Lane - issue #80 and pull request #110) #111

Merged

ghost added this to the v2.3.0 milestone Jan 17, 2018

ghost requested a review from pmai January 17, 2018 14:17

carsten-kuebler mentioned this pull request Jan 17, 2018

Add lane driving direction to the message lane (#84) #85

Closed

pmai reviewed Jan 18, 2018

View reviewed changes

pmai requested review from a user, AndreasVIRES, HendrikAmelunxen and MHerrmannIPG January 18, 2018 12:08

This was referenced Jan 26, 2018

Reduction of duplicated data #93

Closed

Compare: OSI and OpenDrive #37

Closed

carsten-kuebler mentioned this pull request Jan 31, 2018

Lane driving direction #84

Closed

ghost added the FeatureRequest Proposals which enhance the interface or add additional features. label Feb 1, 2018

carsten-kuebler and others added 2 commits February 6, 2018 00:13

New lane model based on Ulm workshop on 2017-11-30

f07ecfd

Restructuring the lane model: Lane boundaries are seperated from the lanes and only referenced in lanes.

Replace successor and antecessor with lane pairings

ad055b1

Logical definition for possible lane pairings, improved logic for intersections.

pmai force-pushed the New-lane-model branch from fff26b7 to ad055b1 Compare February 5, 2018 23:25

pmai self-assigned this Feb 5, 2018

pmai approved these changes Feb 5, 2018

View reviewed changes

pmai merged commit 046e743 into master Feb 5, 2018

pmai added a commit that referenced this pull request Feb 6, 2018

Merge pull request #111 from OpenSimulationInterface/New-Detected-Lan…

5d2a106

…e-model New detected lane model (related to Handling Junctions with OSI::GroundTruth::Lane - issue #80 and pull request #110)

solosito reviewed Feb 8, 2018

View reviewed changes

pmai deleted the New-lane-model branch March 27, 2018 12:15

This was referenced Mar 27, 2018

Handling Junctions with OSI::GroundTruth::Lane #80

Closed

Enhance Doxygen Documentation with Images for Lane Model #185

Closed

raue mentioned this pull request Oct 13, 2021

Tunnel description in OSI #311

Closed

New lane model (Handling Junctions with OSI::GroundTruth::Lane - issue #80) #110

New lane model (Handling Junctions with OSI::GroundTruth::Lane - issue #80) #110

Uh oh!

Conversation

carsten-kuebler commented Jan 17, 2018

Uh oh!

pmai commented Jan 17, 2018

Uh oh!

pmai left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pmai commented Jan 18, 2018

Uh oh!

ghost commented Jan 18, 2018

Uh oh!

pmai commented Jan 26, 2018

Uh oh!

carsten-kuebler commented Jan 29, 2018

Uh oh!

pmai commented Jan 29, 2018

Uh oh!

carsten-kuebler commented Jan 31, 2018

Uh oh!

carsten-kuebler commented Feb 1, 2018

Uh oh!

pmai left a comment

Choose a reason for hiding this comment

Uh oh!

solosito left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants