Skip to content

Add sensor-specific data to detected stationary objects #702

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

Merged
merged 2 commits into from
Mar 24, 2023

Conversation

pmai
Copy link
Contributor

@pmai pmai commented Jan 25, 2023

This addresses #462 partially, probably data should also be added to detected traffic signs and lights. Maybe add to DetectedItemHeader instead? Needs to be harmonized with ISO 23150 as well.

@pmai pmai added this to the V3.6.0 milestone Jan 25, 2023
@pmai pmai self-assigned this Jan 25, 2023
@jdsika
Copy link
Contributor

jdsika commented Jan 25, 2023

Where is the definition of the field? :)

@pmai
Copy link
Contributor Author

pmai commented Jan 25, 2023

Where is the definition of the field? :)

Already exists in sensorspecifics...

@@ -56,7 +56,7 @@ message UltrasonicSpecificObjectData
//
optional double maximum_measurement_distance_sensor = 1;

// This value indicates the propability height for the classification in the
// This value indicates the probability height for the classification in the
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

now that I have corrected the typo I am not sure if "probability height" is even a proper english term haha

@jdsika
Copy link
Contributor

jdsika commented Jan 26, 2023

Considering the fact that the RCS is a measured unit somehow with dependencies on frequency, viewing angle etc... maybe it is suited better in an detectedItemHeader

@pmai
Copy link
Contributor Author

pmai commented Jan 26, 2023

Considering the fact that the RCS is a measured unit somehow with dependencies on frequency, viewing angle etc... maybe it is suited better in an detectedItemHeader

Or something similar; let's see where ISO 23150 nowadays puts that information, maybe this gives a clearer picture, at least for 4.0.0; for 3.6.0 we likely should keep as similar to the moving objects as possible...

@jdsika
Copy link
Contributor

jdsika commented Jan 26, 2023

Considering the fact that the RCS is a measured unit somehow with dependencies on frequency, viewing angle etc... maybe it is suited better in an detectedItemHeader

Or something similar; let's see where ISO 23150 nowadays puts that information, maybe this gives a clearer picture, at least for 4.0.0; for 3.6.0 we likely should keep as similar to the moving objects as possible...

Then I would opt for a creating an issue for a potential breaking change in v4.0.0 as the fields of movingObjekts would need to be moved as well and at the same time for consistency merge this proposal for v3.6.0

@thomassedlmayer
Copy link
Contributor

As far as I can tell, sensor specific data are only referenced in moving objects in the ISO as of 2022-11-22 (even though they generically refer to a 'recognized object' in the description of RCS/lidar reflectivity).

I'd also say that it generally makes sense to put it in the generic header in the longterm as they are not specific to some object type but rather a commonality among any detected objects. If this is what we want, I can create another PR for the 4.0 milestone.

@thomassedlmayer
Copy link
Contributor

Sensor Model WG (08.02.): No opinions on that. Passed on to harmonization WG to check with ISO23150.

@thomassedlmayer thomassedlmayer added the ReadyForCCBReview Indicates that this MR is ready for a final review and merge by the CCB. label Mar 1, 2023
@PhRosenberger
Copy link
Contributor

Sensor Model WG (08.02.): No opinions on that. Passed on to harmonization WG to check with ISO23150.

I would say it does not matter what is written in the ISO23150. We use these information for a different purpose and you want to have this information on all objects for your sensor model, anyways. The actual sensor will calculate e.g. an RCS for any detected object, as all are potentially moving at first sight and could be actually static objects by error. So in simulation you need it on all objects to be able to replicate such erroneous detected (potentially moving) objects.

@ClemensLinnhoff ClemensLinnhoff added the OpenMSL Required to enable sub-libraries in OpenMSL. label Mar 3, 2023
@pmai pmai marked this pull request as ready for review March 13, 2023 09:36
@pmai pmai added ReadyToMerge This PR has been approved to merge and will be merged by a member of the CCB. and removed ReadyForCCBReview Indicates that this MR is ready for a final review and merge by the CCB. labels Mar 13, 2023
@pmai
Copy link
Contributor Author

pmai commented Mar 13, 2023

CCB 2023-03-13: Merge as-is.

pmai and others added 2 commits March 24, 2023 17:48
This addresses #462 partially, probably data should also be added to
detected traffic signs and lights. Maybe add to DetectedItemHeader
instead? Needs to be harmonized with ISO 23150 as well.

Signed-off-by: Pierre R. Mai <pmai@pmsf.de>
Signed-off-by: Carlo van Driesten <carlo.van-driesten@vdl.digital>
@pmai pmai force-pushed the feature/sm/rcs-non-moving-detected-objects branch from 489acdf to 1066206 Compare March 24, 2023 16:48
@pmai pmai merged commit 50b0332 into master Mar 24, 2023
@jdsika jdsika deleted the feature/sm/rcs-non-moving-detected-objects branch April 25, 2024 11:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
OpenMSL Required to enable sub-libraries in OpenMSL. ReadyToMerge This PR has been approved to merge and will be merged by a member of the CCB.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants