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

[DiffDriveController] Add velocity limits for individual wheels #471

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

arslogavitabrevis
Copy link

Pull request for feature request #470

@bmagyar
Copy link
Member

bmagyar commented Nov 24, 2022

Could you please add some testing too?

@arslogavitabrevis
Copy link
Author

arslogavitabrevis commented Nov 28, 2022

I just had a look at the test that are already there.

I think I should bring the code added to the update function and the code that I will have add to the on_configure function for the test to be working well in a subclass like if was done for the speed_limiter to keep it clean. Do you think I should create a new class wheel_speed_limiter or should I enhance the actual speed_limiter class with the wheel limit feature?

@destogl destogl changed the title Add velocity limits for individual wheels [DiffDriveController] Add velocity limits for individual wheels Nov 30, 2022
Copy link
Member

@destogl destogl left a comment

Choose a reason for hiding this comment

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

Can we somehow use the pre-defined structure for joint limits to declare and read the parameters? (see JointLimitsParam class

Can this limiter be useful for other controllers?

A year ago we were working on a "SimpleJointLimiter" that would be in the same package as above limiter and reusable across controllers and other packages.

@mergify
Copy link
Contributor

mergify bot commented Dec 4, 2022

This pull request is in conflict. Could you fix it @arslogavitabrevis?

@arslogavitabrevis
Copy link
Author

Yes I think it would be interesting to use the JointLimitsParam. I will try use it.

Do you think I could pause this pull request in the meantime?

Copy link
Contributor

mergify bot commented Jan 8, 2024

This pull request is in conflict. Could you fix it @arslogavitabrevis?

@christophfroehlich
Copy link
Contributor

As we have now the possibility to parse the URDF directly, it make sense to use the velocity limits directly from the wheel joints. see #949 for an example.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants