Migration to sdf::Plugin broke plugins relying on Parent information

[luca-della-vedova]

Environment

• OS Version: 20.04
• Source or binary build? Either, behavior change was bisected to https://github.com/gazebosim/gz-sim/pull/1352

One of those classic XKCDs https://xkcd.com/1172/

Description

We have been relying on reading the ElementPtr that is passed to Gazebo plugins on initialization to traverse the hierarchy up and read properties of the model that the plugin was attached to. For example, when simulating doors, we read joint limits of the Joint that the plugin was attached to to read the limits and smoothly accelerate / decelerate. https://github.com/open-rmf/rmf_simulation/blob/main/rmf_building_sim_common/include/rmf_building_sim_common/door_common.hpp#L228-L251

This has been a reasonably stable usage shared between Gazebo classic and gz-sim, however since the linked PR this is not possible anymore and the code above fails when fetching the parent. My guess is that this is due to the Plugin element in SDF not having an API to access its parent, hence when offering "backwards compatible behavior" the information is just not there. I'm not sure this is a workflow that we want to support. I guess a possible fix could be to add an API to sdf::Plugin to have access to its parent so plugins can read information from elsewhere in the world if they need to. Another alternative could be to just populate the parent at the gz-sim level when offering the backward compatible behavior, however this particular workflow would still fail once the migration to sdf::Plugin is completed and there is no more sdf::ElemetPtr API. If that is not a clean fix I'm open to alternative on how we can get the same behavior with the new sdf::Plugin data structure.

If the fix of adding the API to sdf::Plugin sounds reasonable I'm happy to take a stab at implementing it!

Steps to reproduce

1. Try to access the parent from a plugin's sdf::ElementPtr before the PR.
2. Try the same after the PR
3. Notice that it doesn't work.

[chapulina]

Ouch, sorry for the regression, this is not a way we usually use the plugins, so we didn't think to test it. In general, you should be getting properties for entities from the ECM, not from SDF elements. The SDF element's parent may not always be there (i.e. if the plugin was added programmatically), and it may have outdated data.

But this is not a change we should make in a stable release, so I think we should revert just the offending lines from #1352 and target them to Garden, with a note in the migration guide.

Thoughts, @nkoenig ?

[luca-della-vedova]

Closing since it's probably not worth it to put too much effort into this somewhat obscure case, we went with the recommended approach of using the ECM and it works well!