Feedback from review

Signed-off-by: Caspar de Haes <caspar.dehaes@five.ai>

Author: caspar-ai

osi_common.proto

@@ -885,3 +885,21 @@ message GeodeticPosition
     //
     optional double altitude = 3;
 }
+
+
+//
+// \brief Generic key-value pair structure
+//
+// A generic key-value pair structure which can be used to capture information
+// which is opaque to the general OSI interface.
+//
+message KeyValuePair
+{
+    // A generic string key.
+    //
+    optional string key = 1;
+
+    // A generic string value.
+    //
+    optional string value = 2;
+}

osi_hostvehicledata.proto

@@ -75,7 +75,7 @@ message HostVehicleData
     //
     optional VehicleLocalization vehicle_localization = 8;
 
-        // What driver assistance is active.
+    // State of driver assistance systems.
     //
     // This can include:
     //  - information presented to the driver, for example, parking sensors
@@ -323,7 +323,7 @@ message HostVehicleData
         // be about settings which would influence evaluation, such as
         // sensitivity settings.
         //
-        repeated CustomDetail custom_detail = 5;
+        repeated KeyValuePair custom_detail = 5;
 
         // ADAS feature that is raising the notification.
         //
@@ -463,22 +463,5 @@ message HostVehicleData
             //
             ACTIVATION_STATE_ACTIVE_DRIVER_OVERRIDE = 6;
         }
-
-        //
-        // \brief Custom detail message
-        //
-        // To contain driver-assist related information that is too function
-        // specific to be captured in a generic way.
-        //
-        message CustomDetail
-        {
-            // A generic string key to identify the information.
-            //
-            optional string key = 1;
-
-            // A generic string value to capture the information.
-            //
-            optional string value = 2;
-        }
     }
 }

osi_trafficupdate.proto

@@ -56,6 +56,11 @@ message TrafficUpdate
 
     // Internal state for each vehicle.
     //
+    // This is an optional field as internal state may not be known or relevant,
+    // for example, a trailer is not going to have any internal state.
+    // It is also allowed to only specify internal_state for a subset of the
+    // objects referenced in the update.
+    //
     // \note This covers any information which cannot be externally perceived
     // and therefore cannot be included in messages available in ground truth.
     //