Fix sensordata reference frame definitions

[thomassedlmayer]

Resolves #712

• I removed the incorrect parent frame definition for detected moving object base and base_rmse.
• I added definitions of the respective parent frame for all SensorData objects (moving objects, stationary objects, traffic sign, traffic light, road marking, lane boundary). Some of those SensorData objects have example images where the reference frame of the detected objects is shown to be the vehicle coordinate system (with its origin in the center of the rear axle of the ego vehicle). Though, this only applies if virtual mounting position and orientation are set to (0,0,0) for x, y, z and yaw, pitch, roll. I tried to clarify those images by adding some explanation.

[jdsika]

I think ISO23150 demands for detected objects to be in vehicle frame. @PhRosenberger we need to crosscheck this

[pmai]

Since the OSI definition is a strict superset, I don't think this is an issue: If the virtual mounting position is 0,0,0, then you get the vehicle frame (as defined via bbcenter to rear axle); if you use a different virtual mounting position, you get something else... So if ISO 23150 is so restricted, then people who care will use virtual mounting position of 0,0,0 (which will be the most common vmp anyways, at least for cars - e.g. trucks are very different at least for some sensors).

[PhRosenberger]

I agree to @pmai as long as we can support the way ISO23150 demands it, we are fine. However, OSI is and should stay more flexible than that, as simulation can do more than the actual sensor.

[jdsika]

I just want to note here that at least how I personally understand this it is NOT a bug but the currently desired defintion which initially came from ISO. For that reason it was mentioned that \note The parent frame of \c base is the sensor's vehicle frame.

I understand that apparently I was the only person who was ok with having two different refernece frames for objects and detections (which were always in sensor frame).

If we merge this it is breaking backwards compatibility. Otherwise we have to make a poll if every OSI user did see it your way?

[ClemensLinnhoff]

In my opinion the current documentation in itself is not backwards (or forwards) compatible because it is confusing and some parts contradict others of the same documentation. Therefore this needs to be changed and unified asap.
I don't think a poll of all OSI users is necessary, because it is still possible to use the ISO definition.

[jdsika]

Sorry, the poll comment was with "sarcasm on".

Can you state where the documentation contradicts regarding this reference frame for detected objects?

[thomassedlmayer]

@jdsika: See #712

Also Clemens and I noticed that the terms (host) vehicle coordinate or (host) vehicle coordinate system are used all around the OSI docs while never being defined properly anywhere. As Clemens said it is quite hard to understand the used coordinate system from reading the documentation.

I'm currently working on clarifying things (and also fixing some minor errors) in this chapter of the OSI documentation. For example, the image of the example at the end of chapter 2.2.4 is showing a different (probably wrong?) parent frame of the physical mounting position.

[jdsika]

I just checked it internally and at BMW we are doing it "correctly" in accordance with the main branch documentation:

• object lists are in vehicle frame "ego-vehicle-rear-axis"
• detection lists are in sensor coordinate frame

Discussion still needed

[thomassedlmayer]

I just checked it internally and at BMW we are doing it "correctly" in accordance with the main branch documentation:

• object lists are in vehicle frame "ego-vehicle-rear-axis"
• detection lists are in sensor coordinate frame

I think, if you're not setting a virtual mounting position/orientation (= SensorView::mounting_position) this should be ok. Though, IMO this is still not strictly based on the "main branch documentation". The OSI documentation itself is quite clear on this (see https://opensimulationinterface.github.io/osi-documentation/#_sensor_data). It is just the class reference doc that was probably forgotten to be adapted at some time.

I fixed another contradiction for logical detection reference frames. Within the message LogicalDetection the attribute "position" was defined to be with respect to the host vehicle coordinate system while the higher-level description of the LogicalDetection message itself referred to the virtual sensor coordinate system for contained attributes. I assumed, the latter is the correct definition and adapted it accordingly.

[thempen]

As I am not to deep into the coodinate systems...

I understood, that there is the object coordinate system -> leading to the host vehicle coordinate system -> leading tho the (virtual) sensor coordinate system. For me it is sometimes hard to distinguish between the actual and virtual mounting position.
This can be a topic for 4.0 to rename the parameters itself to have it in a more "speaking" way.

In total, the clarification makes sense for me.

// The virtual sensor coordinate system is relative to the vehicle coordinate // system which has its origin in the center of the rear axle of the ego // vehicle. This means if virtual sensor mounting position and orientation are // set to (0,0,0) the virtual sensor coordinate system coincides with the // vehicle coordinate system.
This is something, i would make as a \note as it should not be necessary to understand the overall concept.

For the position and velocity, the changes will be breaking, yes.
E.g. for trucks, the virtual sensor mounting position can be dynamic to the host vehicle coordinate system.
Seen as a bug from the beginning, i would suggest on changing it within a minor release.

[thomassedlmayer]

I would agree on renaming the SensorData::mounting_position field for OSI 4.0 to emphasize that it is not the mounting position of a physical sensor (e.g. virtual_mounting_position). I will create a github issue for that.

I also agree on adding the /note tag. I will add this change soon and then add the ReadyForCCB label as discussed in today's meeting.

For the position and velocity, the changes will be breaking, yes.

It only fixes a contradiction/bug in the documentation. So I suppose it is not a breaking change but a bug fix.

[pmai]

CCB 2023-05-22: Merge as-is, @pmai to resolve conflicts.