Skip to content

Conversation

azeey
Copy link
Contributor

@azeey azeey commented Sep 10, 2024

🦟 Bug fix

Fixes gazebosim/gazebo_test_cases#1640

Alternative to: #2590

Summary

Loading the logging playback plugin before SceneBroadcaster prevents playback from being displayed on the GUI. This order was changed in #2452 which caused the regression.

I haven't had time to delve into why the order matters, but it can easily be verified by running test/worlds/log_playback.sdf with <playback_path> set to the log directory. If the plugin comes before SceneBroadcaster, the playback is not displayed on the GUI.

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

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

Loading the logging playback plugin before `SceneBroadcaster`
prevents playback from being displayed on the GUI.

Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>
@azeey azeey requested a review from mjcarroll as a code owner September 10, 2024 06:06
@azeey azeey requested review from arjo129 and nkoenig September 10, 2024 06:06
@github-actions github-actions bot added the 🏛️ ionic Gazebo Ionic label Sep 10, 2024
Copy link
Contributor

@arjo129 arjo129 left a comment

Choose a reason for hiding this comment

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

Wow, nice catch! This makes sense but we probably should add a regression test (and some comments) to at the very least make sure we don't repeat this same error.

@arjo129 arjo129 mentioned this pull request Sep 10, 2024
8 tasks
@avanmalleghem
Copy link
Contributor

When will it be merged so that we can finish some tutorials ?

@arjo129
Copy link
Contributor

arjo129 commented Sep 12, 2024

Regression test added in #2619

arjo129 and others added 2 commits September 12, 2024 09:24
@azeey
Copy link
Contributor Author

azeey commented Sep 12, 2024

The CI failure seems to be flaky and is also present in the target branch https://build.osrfoundation.org/view/gz-ionic/job/gz_sim-ci-gz-sim9-noble-amd64/test_results_analyzer/

@azeey azeey merged commit ca40c1d into gazebosim:gz-sim9 Sep 12, 2024
9 of 10 checks passed
arjo129 added a commit that referenced this pull request Sep 12, 2024
* Adds a regression test for logplayback

gazebosim/gazebo_test_cases#1640

This commit adds a really simple regression check to make sure we
actually publish on the right topic.

Signed-off-by: Arjo Chakravarty <arjoc@intrinsic.ai>

* typo fix

Signed-off-by: Arjo Chakravarty <arjoc@intrinsic.ai>

* Make atomic

Signed-off-by: Arjo Chakravarty <arjoc@intrinsic.ai>

---------

Signed-off-by: Arjo Chakravarty <arjoc@intrinsic.ai>
Co-authored-by: Addisu Z. Taddese <addisu@openrobotics.org>
@arjo129
Copy link
Contributor

arjo129 commented Aug 5, 2025

https://github.com/Mergifyio backport gz-sim8

@mergify
Copy link
Contributor

mergify bot commented Aug 5, 2025

backport gz-sim8

✅ Backports have been created

mergify bot pushed a commit that referenced this pull request Aug 5, 2025
Loading the logging playback plugin before `SceneBroadcaster`
prevents playback from being displayed on the GUI.

---------

Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>
Co-authored-by: Arjo Chakravarty <arjoc@intrinsic.ai>
(cherry picked from commit ca40c1d)

# Conflicts:
#	src/SimulationRunner.cc
mergify bot pushed a commit that referenced this pull request Aug 5, 2025
* Adds a regression test for logplayback

gazebosim/gazebo_test_cases#1640

This commit adds a really simple regression check to make sure we
actually publish on the right topic.

Signed-off-by: Arjo Chakravarty <arjoc@intrinsic.ai>

* typo fix

Signed-off-by: Arjo Chakravarty <arjoc@intrinsic.ai>

* Make atomic

Signed-off-by: Arjo Chakravarty <arjoc@intrinsic.ai>

---------

Signed-off-by: Arjo Chakravarty <arjoc@intrinsic.ai>
Co-authored-by: Addisu Z. Taddese <addisu@openrobotics.org>
(cherry picked from commit 0f16949)
azeey added a commit that referenced this pull request Aug 6, 2025
Loading the logging playback plugin before `SceneBroadcaster`
prevents playback from being displayed on the GUI.

---------

Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>
Co-authored-by: Arjo Chakravarty <arjoc@intrinsic.ai>
(cherry picked from commit ca40c1d)

# Conflicts:
#	src/SimulationRunner.cc
azeey added a commit that referenced this pull request Aug 13, 2025
* Adds a regression test for logplayback

gazebosim/gazebo_test_cases#1640

This commit adds a really simple regression check to make sure we
actually publish on the right topic.

Signed-off-by: Arjo Chakravarty <arjoc@intrinsic.ai>

* typo fix

Signed-off-by: Arjo Chakravarty <arjoc@intrinsic.ai>

* Make atomic

Signed-off-by: Arjo Chakravarty <arjoc@intrinsic.ai>

---------

Signed-off-by: Arjo Chakravarty <arjoc@intrinsic.ai>
Co-authored-by: Addisu Z. Taddese <addisu@openrobotics.org>
(cherry picked from commit 0f16949)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🏛️ ionic Gazebo Ionic

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

gz-sim: log_record_dbl_pendulum.sdf

3 participants