Adding HaltMotion (Or new suggested name) to physics plugin

[Lobotuerk]

Signed-off-by: Tomas Lorente [email protected]

🎉 New feature

Summary

Adds the functionality to stop a model's joint's movement in other cases than the battery being 0. By adding a HaltMotion component to the model, its joints movement can be turned on/off. Also added some QoL to kineticEnergyMonitor plugin with profilers and some checking.

(Updates)
19acc8d adds the capability to turn movement halting off if you only need the advice (and making it backward compatible).
4a9f551 Moved everything into GameLogicPlugin, just left here the halt component evaluation in physics and the QoL changes in kineticEnergyMonitor

Test it

TO-DO tested it with SubT worlds for now

Checklist

• Signed all commits for DCO
• Added tests
• Added example and/or tutorial
• Updated documentation (as needed)
• Updated migration guide (as needed)
• 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

[codecov]

Codecov Report

Merging #728 (4a9f551) into ign-gazebo4 (302f5ed) will decrease coverage by 10.20%.
The diff coverage is 68.63%.

❗ Current head 4a9f551 differs from pull request most recent head 6b19cd7. Consider uploading reports for the commit 6b19cd7 to get more accurate results

@@               Coverage Diff                @@
##           ign-gazebo4     #728       +/-   ##
================================================
- Coverage        77.37%   67.16%   -10.21%     
================================================
  Files              217      241       +24     
  Lines            12217    18893     +6676     
================================================
+ Hits              9453    12690     +3237     
- Misses            2764     6203     +3439     

Impacted Files	Coverage Δ
include/ignition/gazebo/EntityComponentManager.hh	100.00% <ø> (ø)
include/ignition/gazebo/rendering/SceneManager.hh	100.00% <ø> (ø)
src/Conversions.cc	81.56% <ø> (-0.33%)	⬇️
.../plugins/component_inspector/ComponentInspector.cc	7.29% <0.00%> (-1.32%)	⬇️
.../plugins/component_inspector/ComponentInspector.hh	28.57% <ø> (ø)
src/network/PeerTracker.hh	100.00% <ø> (ø)
src/systems/apply_joint_force/ApplyJointForce.hh	100.00% <ø> (ø)
src/systems/breadcrumbs/Breadcrumbs.hh	100.00% <ø> (ø)
src/systems/buoyancy/Buoyancy.hh	100.00% <ø> (ø)
src/systems/contact/Contact.hh	100.00% <ø> (ø)
... and 98 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 86fe608...6b19cd7. Read the comment docs.

[azeey]

IMO, the kinetic energy monitor should not be concerned with halting a vehicle. If we adhere to the separation of concerns principle, I would think another System in SubT (eg. GameLogicPlugin) would monitor the output of the kinetic energy monitor and set the state of the HaltMotion component.

I also left some comments in case we decide to proceed with keeping that logic in the kinetic energy monitor.

[Lobotuerk]

IMO, the kinetic energy monitor should not be concerned with halting a vehicle. If we adhere to the separation of concerns principle, I would think another System in SubT (eg. GameLogicPlugin) would monitor the output of the kinetic energy monitor and set the state of the HaltMotion component.

I do agree with moving the HaltMotion logic into the GameLogicPlugin, but that would require adding A PreUpdate there. @nkoenig Do you have a strong feeling for either of them?

[nkoenig]

IMO, the kinetic energy monitor should not be concerned with halting a vehicle. If we adhere to the separation of concerns principle, I would think another System in SubT (eg. GameLogicPlugin) would monitor the output of the kinetic energy monitor and set the state of the HaltMotion component.

I also left some comments in case we decide to proceed with keeping that logic in the kinetic energy monitor.

Agreed. @Lobotuerk , can you remove the logic for halting motion from the Kinetic Energy Monitor and place it in the SubT Game Logic Plugin?

[nkoenig]

I do agree with moving the HaltMotion logic into the GameLogicPlugin, but that would require adding A PreUpdate there. @nkoenig Do you have a strong feeling for either of them?

Adding a PreUpdate to the GameLogicPlugin sounds good.

[Lobotuerk]

@azeey When moving the halting logic to GameLogic, should I leave the HaltMotion component creation inside KineticEnergyMonitor?

[azeey]

@azeey When moving the halting logic to GameLogic, should I leave the HaltMotion component creation inside KineticEnergyMonitor?

IMO, We don't need to modify KineticEnergyMonitor at all. All the logic can be in GameLogic and Physics. And I think the HaltMotion component creation can go into GameLogic

[azeey]

LGTM! It would be great if you can add a test showing that a vehicle stops once the HaltMotion component is set.

[chapulina]

I'm wondering if this functionality couldn't be achieved using components::JointVelocityReset / components::JointPositionReset?

[Lobotuerk]

I'm wondering if this functionality couldn't be achieved using components::JointVelocityReset / components::JointPositionReset?

It's better for a long time halt to simply stop the physics layer before applying forces, than trying to reset the joint every single timestep