Skip to content

Conversation

@max-rosin
Copy link
Contributor

@max-rosin max-rosin commented Apr 28, 2021

This PR resolves #510.

  • Added a conceptual topic about coordinate systems and reference points.

  • Added a use case topic about transforming messages.

  • My suggestion follows the style and contributors guidelines.

  • I have taken care about the 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.

Signed-off-by: Maximilian Rosin <maximilian.rosin@parson-europe.com>
@max-rosin max-rosin added Documentation Everything which impacts the quality of the documentation and guidelines. ReadyForCCBReview Indicates that this PR is ready for a final review and merge by the CCB. labels Apr 28, 2021
@stefancyliax stefancyliax added this to the V3.4.0 milestone May 1, 2021
@kmeids
Copy link

kmeids commented May 12, 2021

Output CCB 12.05.2021:

  1. WP Leads to review documentation pull requests set to "ReadyForCCB" by Max.


Object coordinates::
Coordinate system for local object coordinates, for example, vehicle coordinates.

Copy link
Contributor

@clemenshabedank clemenshabedank May 18, 2021

Choose a reason for hiding this comment

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

For all coordinate systems: Would it make sense to say something about the origin? e.g.

  • World coordinates: inertial x/y/z positions

  • Sensor coordinates: mounting position of the physical sensor OR virtual mounting position. I think here it should be differentiated which one is used in (at least) technology specific sensor views, sensor data, feature data, logical detection data

  • Object coordinates: For vehicles it is currently middle rare axle, but there were discussions about maybe changing it to middle bounding box. And of course there would be all the other object coordinate systems...

Copy link

Choose a reason for hiding this comment

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

Output CCB 09.06.2021:

  1. Describe OSI's Coordinate systems in a generic manner.
  2. Describe how to define the position of the coordinate systems in OSI (for example wrt reference points, which may be the center of the bounding box).
  3. Example picture to illustrate the above mentioned points (a picture is already available needs to be updated).

Signed-off-by: Maximilian Rosin <maximilian.rosin@parson-europe.com>
@kmeids kmeids removed the ReadyForCCBReview Indicates that this PR is ready for a final review and merge by the CCB. label May 26, 2021
Signed-off-by: Maximilian Rosin <maximilian.rosin@parson-europe.com>
@max-rosin max-rosin added the ReadyForCCBReview Indicates that this PR is ready for a final review and merge by the CCB. label Jun 8, 2021
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 suggestions and comments

It also shows that the reference point of the vehicle is located in the center of its bounding box whereas the origin of the vehicle coordinate system is the middle of rear axle projected onto the ground.
In this example, the virtual mounting position coincides with the origin of the vehicle coordinate system.

image::{imagedir}/osi_coordinate_systems.png[] No newline at end of file
Copy link

Choose a reason for hiding this comment

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

We should use a better image for this, one that does not cover the "Z" on the virtual coordinate system and that also shows the virtual coordinate system in a different location from the rear axle to evade confusion with the DIN ISO.

Copy link
Contributor Author

@max-rosin max-rosin Jun 24, 2021

Choose a reason for hiding this comment

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

Simplified the image and moved the example to the topic describing the transformations between coordinate systems. Seemed to me to be the more appropriate place.

I don't really understand why I should remove the z-axis on the virtual coordinate system.

@kmeids : Please check out the modified image and topics.

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 suggestions and comments.

@max-rosin max-rosin force-pushed the 510_describe_coordinate_system_ref_points branch from 0cf6266 to 442c64f Compare June 24, 2021 11:32
Signed-off-by: Maximilian Rosin <maximilian.rosin@parson-europe.com>
@max-rosin max-rosin force-pushed the 510_describe_coordinate_system_ref_points branch from 442c64f to 7e878a4 Compare June 24, 2021 11:33
Signed-off-by: Maximilian Rosin <maximilian.rosin@parson-europe.com>
Signed-off-by: Maximilian Rosin <maximilian.rosin@parson-europe.com>
max-rosin added a commit that referenced this pull request Jul 7, 2021
Signed-off-by: Maximilian Rosin <maximilian.rosin@parson-europe.com>
Signed-off-by: Maximilian Rosin <maximilian.rosin@parson-europe.com>
@max-rosin max-rosin force-pushed the 510_describe_coordinate_system_ref_points branch from 25d8fdb to f7e2991 Compare July 7, 2021 12:51
@max-rosin max-rosin merged commit 0ed0f54 into migration Jul 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Documentation Everything which impacts the quality of the documentation and guidelines. ReadyForCCBReview Indicates that this PR is ready for a final review and merge by the CCB.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants