689 add raytracerview config #699

LukasElster · 2023-01-12T10:55:12Z

Reference to a related issue in the repository

Add a description

PR adds RaytracerViewConfig to SensorView fpr raytracinf interfaces and changes Lidar/RadarSensorView in interfaces without rendering techniques related fields. Changes in Lidar and RadarSensorView are not backward compatible.

Take this checklist as orientation for yourself, if this PR is ready for the Change Control Board:

My suggestion follows the style and contributors guidelines.
I have taken care about the message documentation and the fields and enums documentation.
I have done the DCO signoff.
My changes generate no errors when passing CI tests.
I have successfully implemented and tested my fix/feature locally.
Appropriate reviewer(s) are assigned.

If you can’t check all of them, please explain why.
If all boxes are checked or commented and you have achieved at least one positive review, you can assign the label ReadyForCCBReview!

PhRosenberger

I would not merge before clarifying if we write "ray tracer" or "raytracer" and we should correct the other small typos.

Additionally, I raised some additional questions to be answered before merging.

osi_common.proto

PhRosenberger · 2023-01-18T15:22:57Z

osi_sensorview.proto

+
+    // Raytracer-specific View(s).
+    //
+    repeated RaytracerView raytarcer_view = 1005;


typo. Should be raytracer_view

As more as I think about it, I even assume repeated RayTracerView ray_tracer_view would be correct.

And if this is correct, this would mean to write ray tracing and ray tracer separated all over the file, not?

Absolutely right. Changes done in osi_sensorviewconfiguration and osi_sensorview

osi_sensorview.proto

osi_common.proto

PhRosenberger · 2023-01-20T20:43:05Z

osi_sensorview.proto

+    // The raw raytracer data in the memory layout and order specified by the
+    // raytracer input configuration.
+    //
+    optional bytes raytarcer_data = 2;


typo. This should be raytracer_data

As written in the comment above, we should clarify whether ray tracing should be written separately or not.

Corrected and changed to ray_tracer_data

osi_sensorviewconfiguration.proto

PhRosenberger

No, I guess, it's fine.

thomassedlmayer

We should probably rewrite the section between line 34 to 46 in osi_sensorviewconfiguration.proto as the number of rays is not included in any of the configurations anymore? Were these number_of_rays fields removed on purpose? In the current OSI version they are marked as raytracer specific characteristic, so I was just wondering where they went...

thomassedlmayer · 2023-01-25T16:04:09Z

osi_sensorviewconfiguration.proto

    //
-    // \note Rotation is defined analog Spherical3d


This note on the rotation got lost in the new SpatialSignalGain message which replaces the AntennaDiagramEntry message. Maybe we should add it again just to be clear?

Then we should add this note it in the osi_common, where the message is defined. What is not completly clear is the note itself, because we dont have the type Spherical3d here and one dimension less. Therefore the rotation can't be analog to Spherical3d. What do you think, @thomassedlmayer ?

Yes, it should be added in osi_common. I just added the note to the removed line.
You're right, we have one dimension less. But we should still determine the direction of the angles, right? E.g. right hand rule

osi_sensorviewconfiguration.proto

thomassedlmayer · 2023-01-26T08:11:20Z

osi_sensorviewconfiguration.proto

    //
-    optional uint32 max_number_of_interactions = 8;
+    optional double beam_divergence_horizontal = 7;


Maybe also rename the field of view-fields in RadarSensorViewConfiguration/Ultrasonic to beam divergence then? Does this make sense there too?

Also, the note that the rotation is as defined in Spherical3d is missing again.

beam_divergence is typically a parameter in the datasheet of the lidar sensor. In the case of radar the continious character of the electromagnetic wave together with the antenna diagramm characteristics define the beam_divergence. Therefore this field should not be added to radar. In case of ultrasonic I'm not shure. Can you help @PhRosenberger?

thomassedlmayer · 2023-01-26T08:21:26Z

osi_sensorviewconfiguration.proto

    //
-    repeated Vector3d directions = 11;
+    optional MountingPosition receiver_pose = 10;


We could maybe be consistent with the naming. In RayTracerSensorViewConfig these fields are called receiver_position/transmitter_position. Maybe also move these position fields up to the mounting position if we already move fields around anyway?

Good suggestion. I also added als explanation in transmitter_position // In case of lidar sensors this could be e.g. a mirror position. and for receiver_position // In case of lidar sensors this could be e.g. a photodiode.

osi_sensorview.proto

ClemensLinnhoff · 2023-03-03T07:29:27Z

osi_sensorview.proto

+    //
+    // The signal can be received from a different angle than it has been emitted to.
+    //
+    // \note Data is in sensor coordinate system.


Specify that it is the physical sensor coordinate system and not the virtual sensor coordinate system

ClemensLinnhoff · 2023-03-03T07:35:12Z

The corresponding issue #689 is assigned to milestone 3.6.0. This will not work as this PR breaks backwards compatibility. Can the PR be split into a compatible part for 3.6.0 and an incompatible part for 4.0?

jdsika · 2023-03-03T08:20:14Z

osi_sensorviewconfiguration.proto

+
+    // Different predefined ray tracer formats
+    // 
+    enum RayTracerFormat


I do not agree to have a raytracer format for every project. What's next? GaiaX format, Pegasus format? We need to come to an agreement here

I fully agree. Wouldn't it be possible to merge the four different raytracer types?
“unknown” and “other” have no attributes anyway. Usage of those types will incorporate ambiguities and doesn't contribute to harmonization.
The approaches of DIVP and Vivaldi largely overlap:

distance (which should be further elaborated) may coincide or be calculated form intersection_path_length + hitpoints

relative_speed is included in both approaches

polarized_attenuation can be calculated (afaik) from the Jones Vector

independent (optional?) parameters incorporate angles of incidence/reflection as well as first/last hit points...

Shouldn't it be possible to reach an agreement here?

Sensor Modeling Project Meeting (16.04.2025): Maybe describe the types in a technical way, not project name-based.

Can we change the jones vector to a jones matrix
float4 -> float8 (4 complex values)
M = VV VH
HV HH
jones_matrix (float8; in V/m; jones matrix of the ray with information of the electromagnetic wave`s phase, signal strength and polarisation, which gives you additonaly the projection to the subset of the jones vectors )

Sensor Modeling Project Meeting (16.04.2025): Maybe describe the types in a technical way, not project name-based.

The various ray tracing formats were developed in the VIVID project and have enabled a connection between different tool worlds, with project names inserted as placeholders. I agree that not every ray tracer or tool provider can simply open a format here, and the technical specifications must be in the foreground.

LudwigFriedmann

As commented, I'd suggest to merge the four proposed RayTracerFormats.

LudwigFriedmann · 2023-03-03T09:16:03Z

osi_sensorviewconfiguration.proto

+
+    // Different predefined ray tracer formats
+    // 
+    enum RayTracerFormat


I fully agree. Wouldn't it be possible to merge the four different raytracer types?
“unknown” and “other” have no attributes anyway. Usage of those types will incorporate ambiguities and doesn't contribute to harmonization.
The approaches of DIVP and Vivaldi largely overlap:

distance (which should be further elaborated) may coincide or be calculated form intersection_path_length + hitpoints

relative_speed is included in both approaches

polarized_attenuation can be calculated (afaik) from the Jones Vector

independent (optional?) parameters incorporate angles of incidence/reflection as well as first/last hit points...

Shouldn't it be possible to reach an agreement here?

Signed-off-by: @lukas.elster <lukas.elster@tu-darmstadt.de>

pmai · 2025-05-28T14:35:50Z

I've refactored to only contain the raytracerview-specific content.

Likely the device-type enum should move to the configuration to take part in auto-negotiation. This can be handled via further commits going forward - spell-check-changes etc. will also go in later commits.

LukasElster added the SensorModeling The Group in the ASAM development project working on sensor modeling topics. label Jan 12, 2023

LukasElster added this to the V4.0.0 milestone Jan 12, 2023

LukasElster requested a review from PhRosenberger January 12, 2023 10:55

LukasElster self-assigned this Jan 12, 2023

LukasElster linked an issue Jan 12, 2023 that may be closed by this pull request

RaytracerView_Config #689

Open

PhRosenberger requested changes Jan 20, 2023

View reviewed changes

PhRosenberger approved these changes Jan 24, 2023

View reviewed changes

LukasElster requested review from thomassedlmayer and pmai January 25, 2023 09:09

thomassedlmayer reviewed Jan 26, 2023

View reviewed changes

ClemensLinnhoff reviewed Mar 3, 2023

View reviewed changes

jdsika added the OpenMSL Required to enable sub-libraries in OpenMSL. label Mar 3, 2023

jdsika reviewed Mar 3, 2023

View reviewed changes

LudwigFriedmann requested changes Mar 3, 2023

View reviewed changes

PhRosenberger mentioned this pull request Jan 17, 2024

Getting SensorView and SensorViewConfig topics done #764

Open

4 tasks

thomassedlmayer moved this to Todo in Sensor Modeling Apr 29, 2025

thomassedlmayer added this to Sensor Modeling Apr 29, 2025

pmai force-pushed the 689-add_raytracerview_config branch from 9b5b2d0 to 0c65e35 Compare May 28, 2025 14:30

First version of RaytracerView/Config

70d4dea

Signed-off-by: @lukas.elster <lukas.elster@tu-darmstadt.de>

pmai force-pushed the 689-add_raytracerview_config branch from 0c65e35 to 70d4dea Compare May 28, 2025 14:33

689 add raytracerview config #699

Are you sure you want to change the base?

689 add raytracerview config #699

Uh oh!

Conversation

LukasElster commented Jan 12, 2023

Reference to a related issue in the repository

Add a description

Take this checklist as orientation for yourself, if this PR is ready for the Change Control Board:

Uh oh!

PhRosenberger left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

PhRosenberger left a comment

Choose a reason for hiding this comment

Uh oh!

thomassedlmayer left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ClemensLinnhoff commented Mar 3, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

LudwigFriedmann left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pmai commented May 28, 2025

Uh oh!

Uh oh!