Skip to content
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

ROS2 Relative spawning #806

Open
wants to merge 1 commit into
base: development
Choose a base branch
from

Conversation

pijaro
Copy link
Contributor

@pijaro pijaro commented Dec 6, 2024

What does this PR do?

Enables spawning entities in an arbitrary coordinate system.

Similar to spawning in WGS coordinate system, enabling the option and setting "relative" in ROS 2 service "reference_frame" spawns entity relatively to the selected coordinate system.

image
image

How was this PR tested?

  1. Spawning with feature off
  2. Spawning with feature on/off and with/without reference frame set
  3. Checked different positions of reference frame

Signed-off-by: Piotr Jaroszek <[email protected]>
@pijaro pijaro marked this pull request as ready for review December 9, 2024 08:54
@pijaro pijaro requested review from a team as code owners December 9, 2024 08:54
@byrcolin byrcolin added the needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. label Dec 10, 2024
@jhanca-robotecai jhanca-robotecai added sig/simulation Categorizes an issue or PR as relevant to SIG Simulation and removed needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Dec 12, 2024
Copy link
Contributor

@jhanca-robotecai jhanca-robotecai left a comment

Choose a reason for hiding this comment

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

What is the difference between the reference_frame (using AZ::Entity) and the ROS2SpawnPointComponent introduced earlier? Is there a need to use both? This feature should be implemented following a discussion (maybe RFC?), as there are many problems around the spawner topic.

@pijaro
Copy link
Contributor Author

pijaro commented Jan 13, 2025

reference_frame is used when we want to spawn an object using (x, y, z) coordinates. The default coordinate system is at O3DE root - (0,0,0) and no rotation. The reference_frame changes it to any other system - useful when we want to spawn an object relatively to another object. It is common to use georeference component that is practically our new "origin" of the level, and we want to use it instead of global O3DE (0,0,0).

Named spawn points (ROS2SpawnPointComponent) positions are implicitly defined (a location in a level, we don't truly care about the coordinates) and are always in O3DE root (0, 0, 0). Relative spawning has no use in that case.

To sum up, these two frames are solving different issues and serves different purposes.

@jhanca-robotecai
Copy link
Contributor

Why not adding an option to spawn an object using xyz coordinates with a named spawn point used as a reference frame? I still don't get why do we need another tooling for that. The change looks like a quickfix for a particular problem within a particular project.

gazebo_msg package is deprecated and we will be removing it in the next full O3DE release, hence most of the spawner's code will be reimplemented. We could start the work already by preparing an RFC and start listing all features that should be included in the spawner.

@adamdbrw
Copy link
Contributor

Could you make it compliant with incoming simulation_interfaces?

@pijaro
Copy link
Contributor Author

pijaro commented Jan 13, 2025

Why not adding an option to spawn an object using xyz coordinates with a named spawn point used as a reference frame?

That is an alternative way of doing it.

However, I would argue if it is more semantically correct. In my opinion, it would be a better idea to guide the spawner where its reference frame really is (as this is a level-only information, like the position of geo frame in a level), rather than passing that in the message. Your suggestion looks like adding an "offset" to spawning point, rather than changing the reference frame.

This PR adds a feature that is very useful in any project that uses WGS geo reference origin.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
sig/simulation Categorizes an issue or PR as relevant to SIG Simulation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants