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 and method to Joint class (#2664) #2818

Open
wants to merge 3 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

Hi @ahcorde,

Thanks for the review! I’ve updated the copyright year as suggested.

I also noticed check failures in my PR, and I was wondering if I should be targeting gz-sim9 instead of gz-sim8. Since the issue originally appeared in gz-sim8, I based my PR on that branch, but I’d appreciate any guidance on whether I should rebase it to gz-sim9 instead.

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
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.

2 participants