Skip to content

Refactor Transport service handlers UserCommands #2996

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 8 commits into from
Aug 18, 2025
Merged

Refactor Transport service handlers UserCommands #2996

merged 8 commits into from
Aug 18, 2025

Conversation

azeey
Copy link
Contributor

@azeey azeey commented Jul 24, 2025 •
edited
Loading

🦟 Bug fix

Summary

This removes a lot of repetitive code where a service handler simply
copies the message, creates and adds the associated command to a command
queue.

This is a precursor to a follow-up PR that changes the service handlers to wait for the execution of the command before returning.

Note: It might be easier to review each commit separately.

Checklist

  • Signed all commits for DCO
  • Added tests
  • Updated documentation (as needed)
  • Updated migration guide (as needed)
  • Consider updating Python bindings (if the library has them)
  • codecheck passed (See contributing)
  • All tests passed (See test coverage)
  • While waiting for a review on your PR, please help review another open pull request to support the maintainers
  • Was GenAI used to generate this PR? If so, make sure to add "Generated-by" to your commits. (See this policy for more info.)

Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining Signed-off-by and Generated-by messages.

azeey added 2 commits July 23, 2025 20:02
@azeey
Refactor Transport service handlers
5a4f534 
This removes a lot of repetitive code where a service handler simply
copies the message, creates and adds the associated command to a command
queue.

Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>
@azeey
Also refactor code for advertising services
e6c43e7 
Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>
@azeey azeey requested a review from mjcarroll as a code owner July 24, 2025 01:30
@github-actions github-actions bot added the 🪵 jetty Gazebo Jetty label Jul 24, 2025
arjo129
arjo129 approved these changes Jul 24, 2025
View reviewed changes
Copy link
Contributor

@arjo129 arjo129 left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM, after style fixes.

@github-project-automation github-project-automation bot moved this from Inbox to In review in Core development Jul 24, 2025
azeey added 4 commits July 23, 2025 21:03
@azeey
Add back missing function
7eac60e 
Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>
@azeey
Update our usage of workerpools (gazebosim#2995)
9822e47 
In `SimulationRunner`, we initialize a worker pool but we never actually
use it so it's pure overhead. In `ServerPrivate` we only use the
worker pool if there are multiple simulation runners, so we can optimize
for the most common use case of one runner.

Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>
@azeey
Fix linter issues
da31fe9 
Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>
@azeey
Merge remote-tracking branch 'origin/main' into refactor_user_commands
5c8884e
@azeey azeey mentioned this pull request Jul 24, 2025
Merged
9 tasks
scpeters
scpeters reviewed Jul 24, 2025
View reviewed changes
azeey added 2 commits July 25, 2025 17:09
@azeey
Fix typo
c8d1331 
Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>
@azeey
Add missing constructor
1bb0343 
Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>
@azeey azeey added this to the Jetty Release milestone Jul 28, 2025
@azeey azeey mentioned this pull request Jul 29, 2025
Merged
10 tasks
@azeey azeey merged commit bf68eae into gazebosim:main Aug 18, 2025
9 checks passed
@github-project-automation github-project-automation bot moved this from In review to Done in Core development Aug 18, 2025
@azeey
Copy link
Contributor Author

azeey commented Oct 16, 2025 •
edited
Loading

@Mergifyio backport gz-sim9 gz-sim8

Backporting to Ionic and Harmonic to get gazebosim/ros_gz#790 backported all the way to Jazzy. See gazebosim/ros_gz#732 (comment)

@azeey azeey deleted the refactor_user_commands branch October 16, 2025 15:28
@azeey azeey mentioned this pull request Oct 16, 2025
Open
@mergify
Copy link
Contributor

mergify bot commented Oct 16, 2025 •
edited
Loading

backport gz-sim9 gz-sim8

✅ Backports have been created

mergify bot pushed a commit that referenced this pull request Oct 16, 2025
@azeey @mergify
Refactor Transport service handlers UserCommands (#2996)
952dacc 
* Refactor Transport service handlers

This removes a lot of repetitive code where a service handler simply
copies the message, creates and adds the associated command to a command
queue.

Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>

* Also refactor code for advertising services

Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>

* Add back missing function

Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>

* Update our usage of workerpools (#2995)

In `SimulationRunner`, we initialize a worker pool but we never actually
use it so it's pure overhead. In `ServerPrivate` we only use the
worker pool if there are multiple simulation runners, so we can optimize
for the most common use case of one runner.

Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>

* Fix linter issues

Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>

* Fix typo

Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>

* Add missing constructor

Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>

---------

Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>
(cherry picked from commit bf68eae)
mergify bot pushed a commit that referenced this pull request Oct 16, 2025
@azeey @mergify
Refactor Transport service handlers UserCommands (#2996)
f5cc094 
* Refactor Transport service handlers

This removes a lot of repetitive code where a service handler simply
copies the message, creates and adds the associated command to a command
queue.

Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>

* Also refactor code for advertising services

Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>

* Add back missing function

Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>

* Update our usage of workerpools (#2995)

In `SimulationRunner`, we initialize a worker pool but we never actually
use it so it's pure overhead. In `ServerPrivate` we only use the
worker pool if there are multiple simulation runners, so we can optimize
for the most common use case of one runner.

Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>

* Fix linter issues

Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>

* Fix typo

Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>

* Add missing constructor

Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>

---------

Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>
(cherry picked from commit bf68eae)

# Conflicts:
#	src/systems/user_commands/UserCommands.cc
This was referenced Oct 16, 2025
Merged
Merged
azeey added a commit that referenced this pull request Oct 17, 2025
@azeey
Refactor Transport service handlers UserCommands (#2996)
7edcccc 
* Refactor Transport service handlers

This removes a lot of repetitive code where a service handler simply
copies the message, creates and adds the associated command to a command
queue.

Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>

* Also refactor code for advertising services

Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>

* Add back missing function

Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>

* Update our usage of workerpools (#2995)

In `SimulationRunner`, we initialize a worker pool but we never actually
use it so it's pure overhead. In `ServerPrivate` we only use the
worker pool if there are multiple simulation runners, so we can optimize
for the most common use case of one runner.

Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>

* Fix linter issues

Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>

* Fix typo

Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>

* Add missing constructor

Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>

---------

Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>
azeey added a commit that referenced this pull request Oct 17, 2025
@azeey
Refactor Transport service handlers UserCommands (#2996)
4860ba7 
* Refactor Transport service handlers

This removes a lot of repetitive code where a service handler simply
copies the message, creates and adds the associated command to a command
queue.

Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>

* Also refactor code for advertising services

Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>

* Add back missing function

Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>

* Update our usage of workerpools (#2995)

In `SimulationRunner`, we initialize a worker pool but we never actually
use it so it's pure overhead. In `ServerPrivate` we only use the
worker pool if there are multiple simulation runners, so we can optimize
for the most common use case of one runner.

Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>

* Fix linter issues

Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>

* Fix typo

Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>

* Add missing constructor

Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>

---------

Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>
(cherry picked from commit bf68eae)
azeey added a commit that referenced this pull request Oct 17, 2025
@azeey
Refactor Transport service handlers UserCommands (#2996)
1017ce6 
* Refactor Transport service handlers

This removes a lot of repetitive code where a service handler simply
copies the message, creates and adds the associated command to a command
queue.

Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>

* Also refactor code for advertising services

Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>

* Add back missing function

Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>

* Update our usage of workerpools (#2995)

In `SimulationRunner`, we initialize a worker pool but we never actually
use it so it's pure overhead. In `ServerPrivate` we only use the
worker pool if there are multiple simulation runners, so we can optimize
for the most common use case of one runner.

Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>

* Fix linter issues

Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>

* Fix typo

Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>

* Add missing constructor

Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>

---------

Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>
This was referenced Oct 17, 2025
Open
Open
Open
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@scpeters scpeters scpeters left review comments

@arjo129 arjo129 arjo129 approved these changes

@mjcarroll mjcarroll Awaiting requested review from mjcarroll

Assignees

No one assigned

Labels

🪵 jetty Gazebo Jetty

Projects

Core development
Status: Done

Milestone

Jetty Release

Development

Successfully merging this pull request may close these issues.

3 participants

@azeey @arjo129 @scpeters