Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add velocity limits component (#2664) #2818

Open
wants to merge 6 commits into
base: gz-sim8
Choose a base branch
from

Conversation

mo-zaghloul
Copy link

🎉 New feature

Closes #2664

Summary

This PR introduces the JointVelocityLimits component and starts integrating it into the Joint class and physics system. However, I need guidance on completing the physics implementation.

Changes in this PR

  • Added JointVelocityLimits.hh component to store joint velocity limits.
  • Integrated the component into Joint.cc.
  • Started integrating it into Physics.cc (incomplete, need guidance).

Test it

The component has been added, but since the physics integration is incomplete, full testing isn't possible yet. Once I receive guidance on the correct approach, I will add appropriate tests.

Guidance Needed

I am currently stuck on integrating velocity limits correctly into the physics system. Specifically, I would appreciate guidance on:

  • The best approach to retrieve and apply velocity limits within the physics update loop.
  • Ensuring that the component interacts correctly with the existing joint properties in simulation.

Checklist

  • Signed all commits for DCO
  • Added new component JointVelocityLimits
  • Completed physics system integration
  • Added tests (Will finalize after physics integration)
  • Updated documentation (Will finalize after implementation)
  • 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

@github-actions github-actions bot added the 🎵 harmonic Gazebo Harmonic label Mar 9, 2025
@@ -0,0 +1,5 @@
{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove this file

@@ -0,0 +1,55 @@
/*
* Copyright (C) 2021 Open Source Robotics Foundation
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* Copyright (C) 2021 Open Source Robotics Foundation
* Copyright (C) 2025 Open Source Robotics Foundation

@mo-zaghloul mo-zaghloul force-pushed the add-joint-limit-methods branch from 43c0fdd to 984b20d Compare March 10, 2025 14:30
@mo-zaghloul
Copy link
Author

mo-zaghloul commented Mar 10, 2025

Hi @ahcorde,

Thanks for the review! I’ve made the changes as suggested.

I need some help with integrating joint velocity limits into the physics system. I’ve created the JointVelocityLimits component, but I’m unsure about how to properly connect it within Joint.cc and the physics system to ensure it works as expected. Could you provide some guidance on how to approach this integration?

Looking forward to your feedback!

Thanks!

@mo-zaghloul mo-zaghloul requested a review from ahcorde March 10, 2025 15:04
@mo-zaghloul mo-zaghloul force-pushed the add-joint-limit-methods branch from 20bc4be to d979e3e Compare March 10, 2025 15:07
@mo-zaghloul mo-zaghloul force-pushed the add-joint-limit-methods branch 2 times, most recently from e427306 to 03a0d8a Compare March 12, 2025 11:31
Signed-off-by: Mohamed Zaghloul <[email protected]>
@mo-zaghloul mo-zaghloul force-pushed the add-joint-limit-methods branch from 03a0d8a to 61f6477 Compare March 13, 2025 03:26
src/Joint.cc Outdated
const EntityComponentManager &_ecm) const
{
return _ecm.ComponentData<components::JointVelocityLimits>(
this->dataPtr->id);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you can just get this information from the JointAxis component, e.g.

auto jointAxis = _ecm.Component<components::JointAxis>(this->dataPtr->id);
if (jointAxis)
{
  return jointAxis->Data().MaxVelocity();
}

For joint velocity, there is no lower or upper limit, but only just one velocity limit field

@mo-zaghloul mo-zaghloul changed the title Add velocity limits component and method to Joint class (#2664) Add velocity limits component (#2664) Mar 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🎵 harmonic Gazebo Harmonic
Projects
Status: In review
Development

Successfully merging this pull request may close these issues.

3 participants