Proposal for simulation interfaces by adamdbrw · Pull Request #1 · ros-simulation/simulation_interfaces

adamdbrw · 2024-12-17T14:20:46Z

Following ros-infrastructure/rep#410, this PR contains a first version of simulation interfaces.

Signed-off-by: Adam Dąbrowski <adam.dabrowski@robotec.ai>

ghost

As stated in the comments, I think EntityState should not be a part of Entity. Instead, a message EntityWithState could be created to bind the two.

Co-authored-by: Martin Pecka <peci1@seznam.cz> Co-authored-by: Steve Peters <computersthatmove@gmail.com>

Signed-off-by: Adam Dąbrowski <adam.dabrowski@robotec.ai>

Signed-off-by: Adam Dąbrowski <adam.dabrowski@robotec.ai> robot_namespace -> namespace Signed-off-by: Adam Dąbrowski <adam.dabrowski@robotec.ai>

Signed-off-by: Adam Dąbrowski <adam.dabrowski@robotec.ai>

ghost

I left a couple comments. Unfortunately, I haven't had much time to review this PR, so there are still parts I haven't reached. I'll be on vacation for a couple weeks, but I'll try to make this a priority when I return.

Signed-off-by: Adam Dąbrowski <adam.dabrowski@robotec.ai>

ghost · 2025-01-07T14:14:02Z

All comments so far have been addressed, please take another look and approve / suggest changes.

ghost

Thank you for putting in this work.

For progress sake, you may want to consider breaking this up into several PRs. If you made one with just the package metadata, then merge that immediately, and then the other bits can go in chunks.

I'd also like to integrate another interface for collisions but that is blocked at the moment.

Co-authored-by: David V. Lu!! <davidvlu@gmail.com>

- wording change for stepping service - features message - comment documentation for USD and URDF formats - custom formats list field Signed-off-by: Adam Dąbrowski <adam.dabrowski@robotec.ai>

ghost · 2025-01-14T14:18:58Z

@DLu thank you for the review, I applied most of your comments and responded to remaining 2 with some argumentation.
I am open to break the PR down if needed, and to include collisions at a later stage (once this one is merged)

ghost

Sorry for the long delay in responding. I like the scope of the PR and the direction it's been evolving so far.

I'd like to ask if @omichel would also chime in for the Webots perspective.

ghost

@DLu asked me to take a look at this due to my work on https://github.com/sea-bass/pyrobosim -- as such, you now have even more feedback in this very ambitious (and unenviable) task of standardizing such a thing.

Hope this is helpful!

Co-authored-by: David V. Lu!! <davidvlu@gmail.com> Co-authored-by: Addisu Z. Taddese <addisu@openrobotics.org>

- Changed SpawnPose to NamedPose, added tags. - SpawnEntity interface substantially updated. - Some improved documentation, especially regarding spawning. - applied other code review comments Signed-off-by: Adam Dąbrowski <adam.dabrowski@robotec.ai>

Signed-off-by: Adam Dąbrowski <adam.dabrowski@robotec.ai>

ghost

So I think we should mark Bounds.msg and Result.msg as things that should be migrated upstream in the long term. Bounds.msg should probably be integrated with shape_msgs and @mjcarroll probably has a better opinion of where Result should go.

ghost · 2025-03-10T07:45:00Z

It is a busy time for me now but I will do my best to apply all the feedback this week.
Update: on it now

Co-authored-by: David V. Lu!! <davidvlu@gmail.com>

- Added SetEntityInfo to be enable setting of items such as tags. - EntityCategory is now self-contained. - Updated SimulationFeatures to reflect changes - Adjusted documentation in a few places. - Added 2 new sections to the README. Signed-off-by: Adam Dąbrowski <adam.dabrowski@robotec.ai>

ghost · 2025-03-17T12:31:43Z

@Amronos @ayushgnv could you check if your requested changes have been applied as intended and remove the change request / approve if that's the case?

ghost

I went through the result again and it looks pretty good.

I had some minor comments, but they mostly affect the comments in files, so should not affect the interfaces themselves.

- Documentation of relevant feature support flag per each interface. - Enumeration naming changed to TYPE_, CATEGORY_ etc. - Default to 1 for steps. - Other minor suggested docs changes. Signed-off-by: Adam Dąbrowski <adam.dabrowski@robotec.ai>

ghost · 2025-03-18T10:40:59Z

@peci1 all applied.
@ayushgnv if you could, please also check the current iteration for your requested changes and comment further or approve.

Signed-off-by: Adam Dąbrowski <adam.dabrowski@robotec.ai>

ghost

Great proposition! I have only one remark about handling a specific case.

Co-authored-by: Paweł Liberadzki <pawel.liberadzki@gmail.com>

ghost · 2025-03-20T12:04:55Z

@mjcarroll @tfoote please let me know if you would like to approve the current version of the PR or any other changes need to happen. Thank you!

ghost

I am sorry, I introduced this mistake (in comment) with my last suggestion. Here is the correct one. Except that, everything is fine.

Co-authored-by: Paweł Liberadzki <pawel.liberadzki@gmail.com>

ghost

I think that this is in good shape to get started with. We should be ready to evolve it as the implementations get filled in and we learn what we overlooked in the design, either too detailed or not fully specified.

ghost · 2025-03-21T17:01:09Z

ghost · 2025-04-30T18:22:27Z

So I think we should mark Bounds.msg and Result.msg as things that should be migrated upstream in the long term. Bounds.msg should probably be integrated with shape_msgs and @mjcarroll probably has a better opinion of where Result should go.

For anyone who wants to debate ROS interfaces even more, I have created two upstream PRs for upstream migration.

simulation interfaces

5267ea2

Signed-off-by: Adam Dąbrowski <adam.dabrowski@robotec.ai>

adamdbrw changed the title ~~First (complete) draft of simulation interfaces~~ Proposal for simulation interfaces Dec 17, 2024

peci1 reviewed Dec 17, 2024

View reviewed changes

scpeters reviewed Dec 18, 2024

View reviewed changes

Comment thread msg/Entity.msg Outdated

Comment thread msg/EntityState.msg Outdated

Adam Dąbrowski and others added 6 commits December 19, 2024 15:32

Apply suggestions from code review

0cfd0fd

Co-authored-by: Martin Pecka <peci1@seznam.cz> Co-authored-by: Steve Peters <computersthatmove@gmail.com>

applied review suggestions

d874c23

Signed-off-by: Adam Dąbrowski <adam.dabrowski@robotec.ai>

added missing cmake entries

10f59d2

Signed-off-by: Adam Dąbrowski <adam.dabrowski@robotec.ai>

stepping service

ea23f59

Signed-off-by: Adam Dąbrowski <adam.dabrowski@robotec.ai>

robot_namespace -> namespace

97b79f7

Signed-off-by: Adam Dąbrowski <adam.dabrowski@robotec.ai> robot_namespace -> namespace Signed-off-by: Adam Dąbrowski <adam.dabrowski@robotec.ai>

changed filter semantics to regex

8bae938

Signed-off-by: Adam Dąbrowski <adam.dabrowski@robotec.ai>

azeey reviewed Dec 21, 2024

View reviewed changes

Comment thread msg/EntityState.msg Outdated

Comment thread msg/SpawnableBounds.msg Outdated

Adam Dąbrowski added 4 commits January 7, 2025 12:30

Updated EntityState and SpawnableBounds messages to address feedback

885a3fa

Signed-off-by: Adam Dąbrowski <adam.dabrowski@robotec.ai>

Added action for multi-stepping

53d6ee9

Signed-off-by: Adam Dąbrowski <adam.dabrowski@robotec.ai>

Added stepping alternatives documentation

aaee904

Signed-off-by: Adam Dąbrowski <adam.dabrowski@robotec.ai>

Added GetSimulatorFeatures interface

df98b24

Signed-off-by: Adam Dąbrowski <adam.dabrowski@robotec.ai>

adamdbrw requested review from azeey, peci1 and scpeters January 7, 2025 14:13

DLu reviewed Jan 10, 2025

View reviewed changes

Adam Dąbrowski and others added 3 commits January 13, 2025 13:39

Apply suggestions from code review

4d551f1

Co-authored-by: David V. Lu!! <davidvlu@gmail.com>

Update srv/GetEntityState.srv

95f46d9

Co-authored-by: David V. Lu!! <davidvlu@gmail.com>

Code review improvements:

0169e10

- wording change for stepping service - features message - comment documentation for USD and URDF formats - custom formats list field Signed-off-by: Adam Dąbrowski <adam.dabrowski@robotec.ai>

DLu reviewed Jan 14, 2025

View reviewed changes

Comment thread msg/SimulatorFeatures.msg Outdated

DLu reviewed Jan 14, 2025

View reviewed changes

Comment thread msg/SpawnPose.msg Outdated

azeey reviewed Jan 15, 2025

View reviewed changes

sea-bass reviewed Jan 16, 2025

View reviewed changes

Adam Dąbrowski and others added 2 commits January 20, 2025 10:48

Apply suggestions from code review

1e92784

Co-authored-by: David V. Lu!! <davidvlu@gmail.com> Co-authored-by: Addisu Z. Taddese <addisu@openrobotics.org>

Revision following the review:

9bf757f

- Changed SpawnPose to NamedPose, added tags. - SpawnEntity interface substantially updated. - Some improved documentation, especially regarding spawning. - applied other code review comments Signed-off-by: Adam Dąbrowski <adam.dabrowski@robotec.ai>

Amronos suggested changes Mar 4, 2025

View reviewed changes

Comment thread srv/GetEntitiesStates.srv Outdated

Comment thread msg/EntityFilters.msg Outdated

corrected entities to strings

5f0aefb

Signed-off-by: Adam Dąbrowski <adam.dabrowski@robotec.ai>

DLu approved these changes Mar 5, 2025

View reviewed changes

Comment thread msg/Bounds.msg Outdated

Comment thread msg/Bounds.msg Outdated

Adam Dąbrowski and others added 2 commits March 17, 2025 11:35

Update msg/Bounds.msg

5fe0d30

Co-authored-by: David V. Lu!! <davidvlu@gmail.com>

Amronos approved these changes Mar 17, 2025

View reviewed changes

michalpelka approved these changes Mar 17, 2025

View reviewed changes

michalpelka mentioned this pull request Mar 17, 2025

RFC : Simulation interfaces o3de/sig-simulation#95

Merged

peci1 approved these changes Mar 17, 2025

View reviewed changes

Comment thread msg/Bounds.msg Outdated

Comment thread msg/EntityFilters.msg Outdated

Comment thread msg/EntityCategory.msg Outdated

Comment thread msg/SimulatorFeatures.msg Outdated

Comment thread srv/DeleteEntity.srv

Comment thread srv/StepSimulation.srv Outdated

Applied review

e98721f

- Documentation of relevant feature support flag per each interface. - Enumeration naming changed to TYPE_, CATEGORY_ etc. - Default to 1 for steps. - Other minor suggested docs changes. Signed-off-by: Adam Dąbrowski <adam.dabrowski@robotec.ai>

peci1 reviewed Mar 18, 2025

View reviewed changes

Comment thread srv/GetEntitiesStates.srv Outdated

peci1 reviewed Mar 18, 2025

View reviewed changes

Comment thread srv/GetEntityState.srv

feature naming fix

65a653f

Signed-off-by: Adam Dąbrowski <adam.dabrowski@robotec.ai>

PawelLiberadzki reviewed Mar 19, 2025

View reviewed changes

Comment thread srv/SetEntityState.srv

Update srv/SetEntityState.srv

4e6867d

Co-authored-by: Paweł Liberadzki <pawel.liberadzki@gmail.com>

PawelLiberadzki reviewed Mar 20, 2025

View reviewed changes

Comment thread srv/SetEntityState.srv Outdated

Update srv/SetEntityState.srv

2206c58

Co-authored-by: Paweł Liberadzki <pawel.liberadzki@gmail.com>

tfoote approved these changes Mar 21, 2025

View reviewed changes

pijaro approved these changes Mar 21, 2025

View reviewed changes

mjcarroll approved these changes Mar 21, 2025

View reviewed changes

mjcarroll merged commit 86d4cce into ros-simulation:main Mar 21, 2025

masterccioli mentioned this pull request Apr 22, 2025

SpawnEntity, DeleteEntity, and SetEntityPose NOT working with ros_gz_bridge gazebosim/ros_gz#711

Closed

This was referenced Apr 30, 2025

[shape_msgs] New Bounds Msg ros2/common_interfaces#278

Open

Result.msg for Service Responses ros2/common_interfaces#279

Open

Uh oh!

Conversation

adamdbrw commented Dec 17, 2024

Uh oh!

ghost left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ghost left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

ghost commented Jan 7, 2025

Uh oh!

ghost left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ghost commented Jan 14, 2025

Uh oh!

Uh oh!

Uh oh!

ghost left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ghost left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ghost left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

ghost commented Mar 10, 2025 • edited by adamdbrw Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ghost commented Mar 17, 2025

ghost commented Mar 10, 2025 •

edited by adamdbrw

Loading