gz-sim
gz-sim copied to clipboard
Change various CMDs to use `std::optional`
Desired behavior
LinearVelocityCmds are reset to zero at every time step instead of being removed. This means that once a user sets a Link to have a certain predefined velocity, then it is assumed in all subsequent updates that the link will have zero velocity (https://github.com/ignitionrobotics/ign-gazebo/blob/77dd8551e942c6f2a9dfbd6360a51cab9185ebab/src/systems/physics/Physics.cc#L3068-L3073). This is somewhat counter intuitive as it is not documented. Furthermore it also prevents users from setting an initial velocity on a certain link which may be useful at certain times for testing purposes (particularly with systems like LiftDrag and Hydrodynamics).
Alternatives considered
Removing the component altogether. This however would have a performance penalty as it may lead to memory deallocation.
Implementation suggestion
Recommend making the following section: https://github.com/ignitionrobotics/ign-gazebo/blob/77dd8551e942c6f2a9dfbd6360a51cab9185ebab/include/ignition/gazebo/components/LinearVelocityCmd.hh#L38-L42
Into:
using LinearVelocityCmd = Component<
std::optional<math::vector3d>, class LinearVelocityCmdTag>;
IGN_GAZEBO_REGISTER_COMPONENT(
"ign_gazebo_components.LinearVelocityCmd", LinearVelocityCmd)
It would be nice if we can somehow use the idea of std::optional but bake into the ECM so that removing a component is equivalent to resetting a std::optional. And hopefully, this can be done without breaking API/ABI.
Alternatives considered
Removing the component altogether. This however would have a performance penalty as it may lead to memory deallocation.
I'm actually not sure if this is true - when I worked on #856, I created a "removed components" mechanism which is used internally by the ECM to mimic components being added/removed without actually removing them. The reason why I did this is because I found that the pre-existing approach of actually removing components from memory was a costly operation. If a downstream user calls APIs to remove a component from the ECM, internally, the ECM should ignore the corresponding component, even though this component still exists. Then, if the user adds this component again, the component data is updated accordingly and is no longer ignored. So, from the user's perspective, the component has been added/removed, but internally, the component is never actually deallocated from memory (it's just being ignored).
@arjo129 have you tried removing the *Cmd components you don't need after applying the command to see if there's any noticeable performance impact?
- when I worked on https://github.com/ignitionrobotics/ign-gazebo/pull/856, I created a "removed components" mechanism which is used internally by the ECM to mimic components being added/removed without actually removing them.
Ah, this is already solved then. That's great! We just need to update the behavior of the physics system to remove the components instead of resetting them. I think that's feasible for Garden.
Ah, this is already solved then. That's great! We just need to update the behavior of the physics system to remove the components instead of resetting them. I think that's feasible for Garden.
this is similar to (if not a duplicate of) #1926 and an implementation attempt is in #2228