Refactor CameraDetection

[CezarySzyszkaAltran]

Abstract

Data model CameraDetectionData interface is significantly different fro other FeatureData interfaces. To align it with the rest I would suggest the following changes.

• Message CameraDetectionSpecificHeader should be dropped.

• Message CameraDetectionData should drop fields
  optional CameraDetectionSpecificHeader specific_header = 3;
  and
  repeated CameraPoint point = 4;

• Message CameraDetection should drop field
  optional uint32 first_point_index = 31;
  and
  optional uint32 number_of_points = 32;
  and introduce
  repeated CameraPoint point = XX; .

• Set of 22 boolean fields in CameraDetection message should be replaced with one enum.

• Message CameraPoint should be moved to be inside class of CameraDetection.

Of course there might be good reasons to proceed with current design. We can discuss it here.

Proposal

Current data model

Currently Camera detection is modeled in following way.

• (class) CameraDetectionData
  • (field) SensorDetectionHeader header
  • (field) CameraDetectionSpecificHeader specific_header
  • (repeated field) CameraDetection detection
  • (repeated field) CameraPoint point
• (class) CameraDetection
  • (field) double existence_probability
  • (field) Identifier object_id
  • (field) Timestamp time_difference
  • (field) ImageShapeType image_shape_type
  • (field) 22 bool shape_classification_ fields
  • (field) 1 double shape_classification_probability
  • (field) Color color
  • (field) double color_probability
  • (field) Identifier ambiguity_id
  • (field) uint32 first_point_index
  • (field) uint32 number_of_points
  • (enum) Color
  • (enum) ImageShapeType
• (class) CamerPoint
  • (field) double existence_probability
  • (field) Spherical3d point
  • (field) Spherical3d point_rmse
• (class) CameraDetectionSpecificHeader
  • (repeated field) uint32 number_of_valid_points

Deficiencies:

1. Classes CameraDetectionSpecificHeader, CameraDetection, CameraDetectionSpecificHeader and CameraPoint should be inner classes of CameraDetectionData .
2. CameraDetection::time_difference is of type TimeStamp. Is should be specified that it must be positive. If it will ever be negative it will break contract of osi_common.proto > TimeStamp
3. 22 bool shape_classification_ fields should be replaced with one enum
4. Class CameraDetection would be clearer if it would be named DetectedShape

Advantages:

1. If we have ambiguous detection the set of CamerPoint detentions doesn't have to be copied to individual detection. However this could be solved if in CameraDetection class we introduce inner class CameraDetectionClassification. (Or with metter naming detected DetectedShape and DetectedShapeClassification)

Suggested Data Model

One possibility to model the camera data with higher clarity would be something like this.

• (class) CameraDetectionData
  • (field) SensorDetectionHeader header
  • (repeated field) DetectedShape detection
  • (class) DetectedShape
    • (field) double existence_probability
    • (field) Identifier object_id
    • (field) Timestamp time_difference
    • (field) Identifier ambiguity_id
    • (repeated field) CameraPoint point
    • (class) Classification
      • (field) ImageShapeFamily image_shape_Family
      • (field) Shape shape (maybe better name would be type)
      • (field) double shape_classification_probability
      • (field) Color color
      • (field) double color_probability
      • (enum) Shape
      • (enum) Color
      • (enum) ImageShapeFamliy
    • (class) CameraPoint
      • (field) double existence_probability
      • (field) Spherical3d point
      • (field) Spherical3d point_rmse

Advantages

1. Higher clarity of names and intentions
2. Use of inner classes promotes encapsulation
3. Fewer classes

Disadvantages

1. If one detection would have multiple classifications we would need multiple detections in main class. The actual overhead can be estimated if we have some example measured or simulated data.

Copatibility

This solution will break backward compatibility, therefore as 4.0.0 milestone.

[kmeids]

@CezarySzyszkaAltran, I see you are an ASAM member.
Is it possible for you to join one of our Sensor Modeling WG meetings to present your proposal?
They take place on a Bi-weakly basis on Friday from 9:00 to 10:00 CET.

[kmeids]

Adding it to dismissed due to lack of activity on this issue.

[thomassedlmayer]

@CezarySzyszkaAltran Are you still interested in this topic?

[thomassedlmayer]

Closing this PR due to inactivity. Feel free to reopen or create a new one if you'd like to continue the discussion or address the feedback.