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

Backport to harmonic: Specify System::PreUpdate, Update execution order (#2487) #2530

Merged
merged 1 commit into from
Aug 20, 2024

Conversation

scpeters
Copy link
Member

🎉 Feature backport

Backports #2487 to harmonic

Summary

Since #2232 has been backported to harmonic, I think #2232 can be back ported as well, since it allows users to opt-in to configuring system execution order but doesn't modify any existing systems. I think it is safe, and I know Intrinsic would like to use this feature with harmonic.

Original summary of #2487:

While the PreUpdate, Update, and PostUpdate phases of gz-sim systems allow some control over the order in which code is executed, there are cases in which more control is desired. For example, as noted in #2268 and #2391, sensor systems currently update their data during PostUpdate (after the Update step of the physics system) but can't write their data to the ECM since PostUpdate has read-only access to the ECM. Moving sensor updates to Update would allow writing to the ECM, but would require ensuring that the sensor systems execute their Update step after the physics Update. Alternatively, if one wanted to implement a hierarchical cascade of systems acting on data sequentially, rather than adding PrePreUpdate and PrePrePreUpdate phases, one could specify the system priority during PreUpdate for the relevant systems instead.

This feature is used by specifying an integer value in a <gz:system_priority> tag for a system that will control the order in which System PreUpdate and Update callbacks are executed, with smaller priority values executing first.

For reference, the follow-ups on main include #2494 and #2500.

Test it

From #2487:

Follow the README instructions to build and run the PriorityPrinter example plugin to illustrate the feature.

Checklist

  • Signed all commits for DCO
  • Added tests
  • Added example and/or tutorial
  • 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

Note to maintainers: Remember to use Rebase-and-Merge.

@github-actions github-actions bot added the 🎵 harmonic Gazebo Harmonic label Aug 19, 2024
@scpeters scpeters added the beta Targeting beta release of upcoming collection label Aug 19, 2024
Specifying an integer value in a <gz:system_priority> tag
for a system will control the order in which System PreUpdate
and Update callbacks are executed, with lower values executing
first.

The PriorityPrinter example plugin is added to illustrate
the feature.

Signed-off-by: Steve Peters <scpeters@openrobotics.org>
@scpeters scpeters force-pushed the scpeters/backport_system_priority branch from 9828f7b to 309c009 Compare August 19, 2024 22:58
@scpeters
Copy link
Member Author

some flaky tests on homebrew:

I'm going to retry the build

@scpeters
Copy link
Member Author

some flaky tests on homebrew:

I'm going to retry the build

tests passed on the second try: Build Status https://build.osrfoundation.org/job/gz_sim-ci-pr_any-homebrew-amd64/801/

@scpeters scpeters merged commit 24a37be into gz-sim8 Aug 20, 2024
9 checks passed
@scpeters scpeters deleted the scpeters/backport_system_priority branch August 20, 2024 03:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
beta Targeting beta release of upcoming collection 🎵 harmonic Gazebo Harmonic
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants