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 actionlib interface for basic Joint Group Command #60

Open
wants to merge 3 commits into
base: kinetic-devel
Choose a base branch
from

Conversation

egordon
Copy link

@egordon egordon commented Jun 5, 2021

In service of ros-controls/ros_controllers#568

Tested via velocity_controllers/JointGroupVelocityController on Kinova JACO2


# A command to the joints listed in joint_names.
# The order must be identical.
float64[] command
Copy link
Member

Choose a reason for hiding this comment

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

Wold it make sense to make this command more explicit, e.g. trajectory_msgs/JointTrajectoryPoint. This would probably avoid future extensions and fields can be used explicitly (positions, velocities, accelerations).

Does this makes sense for you?

Copy link
Author

Choose a reason for hiding this comment

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

Pros:

  • Could allow for dual command types (e.g. position with a max velocity/acceleration)
  • Explicit command is easier to interpret

Cons:

  • Doesn't allow for non-traditional joint commands (only position, velocity, acceleration, effort) outside of overloading a variable
  • Makes the coding in ros_controllers a bit more difficult (need to add logic that detects the command type and writes to the appropriate field)

I think the pros can outweigh the cons, so let me make the changes in both packages.

@egordon
Copy link
Author

egordon commented Aug 12, 2021

@destogl Sorry for the delay. Updated action and corresponding ros_controllers PR (ros-controls/ros_controllers#569) accordingly

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.

2 participants