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

-    // \brief The radar antenna diagram.
+    // emitter_power must be considered together with tx_antenna_diagram for absolute power calculation. 
    //
-    // \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

+    // Unit: rad
    //
-    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

+    // Pose of the receiver unit in the sensor coordinate system.
    //
-    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?

osi_sensorviewconfiguration.proto

LudwigFriedmann

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

osi_sensorviewconfiguration.proto

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.

SandroReith

This is a good and short description of the format and the expected data.

AsamDiegoSanchez · 2025-09-25T08:22:19Z

OSI CCB meeting 25.09.2025: further discussions and feedback needed.

pmai · 2025-10-16T01:04:44Z

osi_sensorview.proto

+    //
+    optional DeviceType device_type = 3;
+
+    // Enum consits of different predefined device types.


Suggested change

// Enum consits of different predefined device types.

// Enum consists of different predefined device types.

pmai

Also raytracer terms need to be added to spellcheck dictionary, to make this build.

ClemensLinnhoff · 2025-10-16T05:32:00Z

Also raytracer terms need to be added to spellcheck dictionary, to make this build.

I think ray tracing is two words in English.

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

Reformatted to utilize the ray tracing approach. Offers a generic description, independent of tool provider or sensor technology Signed-off-by: SandroReith <40294694+SandroReith@users.noreply.github.com>

updated enum for RAYTRACER_FORMAT_SBR 3 -> 2 Signed-off-by: SandroReith <40294694+SandroReith@users.noreply.github.com>

Signed-off-by: Thomas Sedlmayer <tsedlmayer@pmsfit.de>

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 pmai and thomassedlmayer 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

osi_sensorviewconfiguration.proto Show resolved Hide resolved

LudwigFriedmann requested changes Mar 3, 2023

View reviewed changes

osi_sensorviewconfiguration.proto Show resolved Hide resolved

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 2 times, most recently from 0c65e35 to 70d4dea Compare May 28, 2025 14:33

SandroReith mentioned this pull request Jun 11, 2025

689 add raytracerview config - RAYTRACER_FORMAT #865

Merged

SandroReith approved these changes Sep 24, 2025

View reviewed changes

thomassedlmayer added the ReadyForCCBReview Indicates that this MR is ready for a final review and merge by the CCB. label Sep 25, 2025

thomassedlmayer modified the milestones: V4.0.0, V3.8.0 Sep 25, 2025

pmai reviewed Oct 16, 2025

View reviewed changes

thomassedlmayer removed the ReadyForCCBReview Indicates that this MR is ready for a final review and merge by the CCB. label Oct 16, 2025

LukasElster and others added 6 commits October 23, 2025 14:33

First version of RaytracerView/Config

8dcec93

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

Update osi_sensorviewconfiguration.proto

d445c13

Reformatted to utilize the ray tracing approach. Offers a generic description, independent of tool provider or sensor technology Signed-off-by: SandroReith <40294694+SandroReith@users.noreply.github.com>

Update osi_sensorviewconfiguration.proto

7209fe3

updated enum for RAYTRACER_FORMAT_SBR 3 -> 2 Signed-off-by: SandroReith <40294694+SandroReith@users.noreply.github.com>

Update binary layout information

c170ba9

Signed-off-by: Thomas Sedlmayer <tsedlmayer@pmsfit.de>

Fix minor formatting and spelling issues

334a1bb

Signed-off-by: Thomas Sedlmayer <tsedlmayer@pmsfit.de>

Add reference to SBR approach

d88aaaa

Signed-off-by: Thomas Sedlmayer <tsedlmayer@pmsfit.de>

pmai force-pushed the 689-add_raytracerview_config branch from 06a9d52 to d88aaaa Compare October 23, 2025 12:34

thomassedlmayer added 4 commits October 30, 2025 08:46

Remove RaytracerView

4f86986

Signed-off-by: Thomas Sedlmayer <tsedlmayer@pmsfit.de>

Add details on ray tracing use case

501a1e6

Signed-off-by: Thomas Sedlmayer <tsedlmayer@pmsfit.de>

Change ray tracer spelling and minor fixes

d62699a

Signed-off-by: Thomas Sedlmayer <tsedlmayer@pmsfit.de>

Fix spelling and spellchecks

920568a

Signed-off-by: Thomas Sedlmayer <tsedlmayer@pmsfit.de>

thomassedlmayer force-pushed the 689-add_raytracerview_config branch from e0aa59b to 920568a Compare October 30, 2025 07:59

	// Enum consits of different predefined device types.
	// Enum consists of different predefined device types.

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!

Uh oh!

LudwigFriedmann left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

pmai commented May 28, 2025

Uh oh!

SandroReith left a comment

Choose a reason for hiding this comment

Uh oh!

AsamDiegoSanchez commented Sep 25, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pmai left a comment

Choose a reason for hiding this comment

Uh oh!

ClemensLinnhoff commented Oct 16, 2025

Uh oh!

Reviewers

Assignees