Fix reset for rendering sensors

[azeey]

🦟 Bug fix

Summary

In the case where objects are spawned after simulation has started and a Reset occurs, the Sensors system was failing to remove the spawned objects. Subsequent attempts to respawn the same objects also fail because they are likely to be assigned the same Entity IDs.

The thing to note here is that the EachNew and EachRemoved function of the ECM have to be called from the Reset handler, since the newly created/removed entities are cleared by the next PreUpdate.

TODO:

• Suppress the "already exists" warning if a Reset has occured.

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.

🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸

[arjo129]

Any chance we can get a regression test for this?

[iche033]

I arrived at a very similar fix. I had this call inside the mutex lock block a few lines above, i.e.

    std::unique_lock<std::mutex> lock2(this->dataPtr->renderUtilMutex);
    this->dataPtr->updateTime =  _info.simTime;
    this->dataPtr->updateTimeToApply =  _info.simTime;
    this->dataPtr->updateTimeApplied =  _info.simTime;
    this->dataPtr->renderUtil.UpdateFromECM(_info, _ecm);

but want to check, does this need to be in a separate block with the wait condition?

[azeey]

I'm not 100% sure. Did it work for you? I saw the comment

gz-sim/src/systems/sensors/Sensors.cc

Lines 974 to 980 in 7374a6c

	GZ_PROFILE("UpdateFromECM");
	// Make sure we do not override the state in renderUtil if there are
	// still ECM changes that still need to be propagated to the scene,
	// i.e. wait until renderUtil.Update(), has taken place in the
	// rendering thread
	std::unique_lock<std::mutex> lock(this->dataPtr->renderUtilMutex);
	this->dataPtr->updateTimeCv.wait(lock, [this]()

which made me think it was necessary.

[iche033]

yeah it worked for me.

My thinking is that we could just override the state in renderUtil without waiting for any pending updates (updates from before the reset) to be applied because it's going to be reset to the initial state anyway.