gazebosim / gz-sim

Open source robotics simulator. The latest version of Gazebo.
https://gazebosim.org
Apache License 2.0
621 stars 251 forks source link

Merge 8 -> main #2334

Closed iche033 closed 3 months ago

iche033 commented 3 months ago

➡️ Forward port

Port gz-sim8 to main

Branch comparison: https://github.com/gazebosim/gz-sim/compare/main...gz-sim8

Note to maintainers: Remember to Merge with commit (not squash-merge or rebase)

iche033 commented 3 months ago

depends on https://github.com/gazebosim/gz-msgs/pull/430

iche033 commented 3 months ago

The INTEGRATION_triggered_publisher test is failing because it's receiving more msgs than expected. The test also just started failing on main (failing build). It's likely due to https://github.com/gazebosim/gz-transport/pull/479. If I revert to a gz-transport commit before the merge forward in gz-transport, the test passes.

Interestingly if I run just that problematic test by itself, it passes:

./build/gz-sim9/bin/INTEGRATION_triggered_publisher --gtest_filter="*InputMessagesTriggerServices*"

I think it could be that the test is getting msgs from previous test that uses the same topic name, cc @caguero

Not sure if we should fix the test here or if this exposes an issue in gz-transport

Edit: found the PR that cause the test to fail: https://github.com/gazebosim/gz-transport/pull/470. I think gz-transport is doing what it's supposed to do.

azeey commented 3 months ago

I also noticed this and went down a rabbit hole. It turns out gz::transport:NodeShared in gz-transport is never destroyed. I thought when all the gz::transport::Node were destroyed, NodeShared would be destroyed as well, but this is not the case, so there is a good chance for tests to interfere with each other. I've worked on a potential solution: https://github.com/gazebosim/gz-transport/compare/gz-transport13...azeey:gz-transport:shared_ptr_for_nodeshared It fixes INTEGRATION_triggered_publisher for me.

azeey commented 3 months ago

INTEGRATION_triggered_publisher is also failing on gz-sim8 now (cc @claraberendsen). I'm fairly happy with https://github.com/gazebosim/gz-transport/pull/484, but we probably need a few eyes on it.

iche033 commented 3 months ago

I think the gz-transport issue may take some time to fix. Can we get this in first?