Skip to content

Introducing emitting objects #472

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
Jul 28, 2021
Merged

Conversation

LukasElster
Copy link
Contributor

Reference to a related issue in the repository

#455

Add a description

As street lights and other emitting structures of electromagnetic waves is not part of the actual definition of stationary object this PR introduces it.

Mention a member

Result of the discussion with @PhRosenberger and @kmeids

Check the checklist

  • My code and comments follow the style guidelines and contributors guidelines of this project.
  • I have performed a self-review of my own code.
  • I have made corresponding changes to the documentation.
  • My changes generate no new warnings.
  • I have added tests that prove my fix is effective or that my feature works.
  • New and existing unit tests / travis ci pass locally with my changes.

@LukasElster LukasElster requested a review from kmeids February 3, 2021 16:18
@LukasElster LukasElster self-assigned this Feb 3, 2021
@LukasElster LukasElster added the SensorModeling The Group in the ASAM development project working on sensor modeling topics. label Feb 3, 2021
@LukasElster LukasElster linked an issue Feb 3, 2021 that may be closed by this pull request
@LukasElster LukasElster added the ReadyForCCBReview Indicates that this MR is ready for a final review and merge by the CCB. label May 28, 2021
@kmeids
Copy link

kmeids commented Jun 4, 2021

Tagging the related issue: Fixes #455

osi_object.proto Outdated
// Distribution of the emitted electromagnetic wave's intensity based on the intensity_per_wavelength as
// maximum value.
//
repeated IntensityDistribution emitted_intensity_distribution = 3;
Copy link

@kmeids kmeids Jun 9, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Output from CCB 09.06.2021

  1. Since this message defines the "FoV" of the emitting object and not the actual distribution of each "ray" a better name should be suggested (including the word Spatial either in the message name or in the definition).
  2. A note to be added describing the fact the number of emitted_intensity_distribution should be equal to the number of samples defined in the WavelengthData.

osi_object.proto Outdated
//
// Unit: W/m^2
//
// \note max_intensity_per_wavelength.size() = WavelengthData.samples_number.size()
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Output from CCB 09.06.2021
A none "equation" based definition to be added describing the fact the number of max_intensity_per_wavelength should be equal to the number of samples defined in the WavelengthData.

Copy link

@kmeids kmeids left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please check comments and suggestions.

osi_object.proto Outdated
//
message EmittingStructureAttribute
{
// This message determines the range of the wavelength and its
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add relationship to normal, i.e. angle is symmetrical across the normal, which is defined by the mounting position...

@kmeids kmeids removed the ReadyForCCBReview Indicates that this MR is ready for a final review and merge by the CCB. label Jun 9, 2021
@LukasElster LukasElster added the ReadyForCCBReview Indicates that this MR is ready for a final review and merge by the CCB. label Jun 14, 2021
@stefancyliax stefancyliax added this to the V3.4.0 milestone Jun 23, 2021
@stefancyliax stefancyliax requested a review from pmai June 23, 2021 09:34
Copy link
Contributor

@pmai pmai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good to me.

osi_object.proto Outdated
// \brief Definition of a spatial intensity distribution with a horizontal and a vertical angle
// and the corresponding intensity related to a maximum intensity.
//
message SpatialIntensity
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@LukasElster
Please move the message SpatialIntensity to osi_common.proto.
It is rather generic and will be used in the SETLevel project e.g. in the LidarSensorViewConfiguration to describe the lidar signal emission there (will be used e.g. for ray tracer config).

Moving it now will avoid the effort to move it later ;-)

Copy link
Contributor

@PhRosenberger PhRosenberger Jun 30, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, @ClemensLinnhoff and I discussed this today and we strongly believe that a signal strength in dBm would fit best here (intensity is rather a power distribution in watts per square metre (W/m2)).

We expect that using a logarithmic unit will be better for high difference btw. emision and reception compared to simple %.

It would mean to get rid of the maximum intensity as well, which is on the pro-side, as well, I guess.

Therefore, we recommend the following for osi_common:

//
// \brief Definition of a spatial signal strength distribution
// for an emitting / transmitting / receiving entity
// with a horizontal and a vertical angle
// and the corresponding signal strength in dBm (decibels per milliwatt).
//
message SpatialSignalStrength
{
    // Horizontal angle (azimuth) of emission / transmission / reception
    // in the entity's coordinate system.
    //
    // Unit: rad
    //
    optional double horizontal_angle = 1;

    // Vertical angle (elevation) of emission / transmission / reception
    // in the entity's coordinate system.
    //
    // Unit: rad
    //
    optional double vertical_angle = 2;

    // Emitted / transmitted /received signal strength
    // of the emitting / transmitting / receiving entity
    // at the previously defined horizontal and
    // vertical angle for one specific wavelength.
    // The value for the signal strength
    // is given in dBm (decibels per milliwatt).
    //
    // Unit: dBm
    //
    optional double signal_strength = 3;
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@PhRosenberger I integrated your recommendations

@kmeids
Copy link

kmeids commented Jul 21, 2021

CCB Output 21.07.2021

  1. Stefan will deactivate the checks for the "Build Protos"
  2. @LukasElster and @stefancyliax and @kmeids will fix the conflicting files.
  3. Merge after conflicts are resolved.

@kmeids kmeids added ReadyToMerge This PR has been approved to merge and will be merged by a member of the CCB. and removed ReadyForCCBReview Indicates that this MR is ready for a final review and merge by the CCB. labels Jul 21, 2021
…n in the SensorModeling Bi-Weekly on 05.02.21.

Signed-off-by: @lukas.elster <[email protected]>
… e.g. radar antenna pattern and changed osi_object.proto accordingly.

Signed-off-by: @lukas.elster <[email protected]>
…ntensity based on the recommendations of Philipp Rosenberger

Signed-off-by: @lukas.elster <[email protected]>
@stefancyliax stefancyliax force-pushed the 455-introducing-emitting-objects branch from 1d9e91c to 75e73ea Compare July 28, 2021 11:11
@stefancyliax
Copy link
Contributor

Branch rebased to master with @kmeids @LukasElster.
Merged.

@stefancyliax stefancyliax merged commit f1b1588 into master Jul 28, 2021
@stefancyliax stefancyliax deleted the 455-introducing-emitting-objects branch October 21, 2021 13:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ReadyToMerge This PR has been approved to merge and will be merged by a member of the CCB. SensorModeling The Group in the ASAM development project working on sensor modeling topics.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Introducing Emitting Objects
5 participants