New detected lane model (related to Handling Junctions with OSI::GroundTruth::Lane - issue #80 and pull request #110) #111
Conversation
ghost
requested a review
from 
pmai
left a comment
pmai
left a comment
There was a problem hiding this comment.
Should the MeasurementStatus message be called MeasurementState (or vice-versa, however I think state is better)?
ghost
added
the
FeatureRequest
osi_detectedlane.proto
Outdated
|
|
||
| // The lane boundary to which this detection is associated to. | ||
| // | ||
| optional LaneBoundary laneboundary = 2; |
pmai
left a comment
pmai
left a comment
There was a problem hiding this comment.
I think the MeasurementState should me moved out of DetectedTrafficSign, as commented inline; if there is agreement on this, I can provide a relevant patch.
|
|
||
| // The measurement state of the detected lane. | ||
| // | ||
| optional DetectedTrafficSign.MeasurementState measurement_state = 4; |
There was a problem hiding this comment.
I think if the message is to be reused, it should be separated out from the DetectedTrafficSign message (it probably makes sense to have this centrally defined; I can provide a patch for this if agree upon). @CarloVanDriestenBMW ?
There was a problem hiding this comment.
See my comments on PR #117 , I'd propose (and do this during merging) to move MeasurementState into DetectedObjectHeader, and move DetectedObjectHeader out of DetectedObject, which should handle all of this cleanly, I think. @CarloVanDriestenBMW ?
Seperate detected lane boundaries similar to real sensors output.
Feedback from Carlo van Driesten and Pierre Mai added.
pmai
force-pushed
the
New-Detected-Lane-model
branch
from
98d1b4c to
3313aa2
Compare
|
Further refactoring is handled in PR #117, so merging for now. |
According to the OSI workshop in Ulm (November 2017), I propose this new detected lane model and the corresponding OSI messages. Change break downward compatibility. Changes are similar to #110 (new Lane model).
issue #80, pull request #110