-
Notifications
You must be signed in to change notification settings - Fork 2
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
SW-2049 add joint gains as a command interface for spot ros2 control #3
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't love this but I don't see any other way around it either. xacro
was not built for this much logic.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! I'm surprised that gains are being exposed as "hardware" in this way, though. Where are these gains used? Is this the "low level" control for Spot, even "lower" than the ROS control layer?
My naivete on all things Spot might be showing on this one, sorry if this question misunderstands how things work.
@jcarpinelli-bdai Yeah, it's a little uncommon, but they are a part the low level API command to spot through BD's sdk, alongside position/velocity/load which are the more "standard" command interfaces. For our hardware interface to have all of the functionality the SDK provides, these interfaces need to be exposed too. there's more info on it here: https://dev.bostondynamics.com/docs/concepts/joint_control/readme#robot-command-stream |
Sounds good, thank you @khughes-bdai! |
Going to merge this in, then merge in the first PR in the linked stack (bdaiinstitute/spot_ros2#588) as that results in the same behavior for the user |
) ## Change Overview Adds k_q_p and k_qd_p to the command interface of Spot's hardware interface.These get set to an initial value via the `initial_value` field from the URDF, so existing logic here in the hardware interface to parse this input has been removed in favor of this new method. Existing controllers will not need to change anything, as they simply don't claim the interface. The following PR in the stack will set up a controller to allow users to easily change this at runtime. This PR also depends on the changes from bdaiinstitute/spot_description#3 to expose these new command interfaces in the URDF. ## Testing Done The default behavior from the user interface does not change with this PR. - [x] Tested wiggle arm example successfully on robot with default gains - [x] Tested wiggle arm example with different initial gains, verified that robot behavior adjusts accordingly \\
Adds the gains
k_q_p
andk_qd_p
as a command interface so it can be used by the spot hardware interface . With this addition, the input validation needs to be done at the URDF level so there are some checks to ensure that the user passed in gains are the correct length, and to replace them with the defaults if not. This leads to some slightly ugly list parsing xacro logic but it does not seem like there's a cleaner way around this with current xacro features.Testing:
i would like to wait to merge this in until the rest of the linked stack is ready, as currently this will break spot_ros2 main