[SW-2049] ability to stream joint gains through hardware interface #588
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This stack of pull requests is managed by Graphite. Learn more about stacking. |
Pull Request Test Coverage Report for Build 13526106956Details
💛 - Coveralls |
khughes-bdai
force-pushed
the
khughes/SW-2049-stream-joint-gains
branch
from
04c33fc to
a80e374
Compare
This was referenced Feb 21, 2025
khughes-bdai
force-pushed
the
khughes/SW-2049-stream-joint-gains
branch
2 times, most recently
from
b9c0919 to
61b56f5
Compare
khughes-bdai
requested review from
METEORITENMAX,
jbarry-bdai,
amessing-bdai,
mpickett-bdai,
bhung-bdai and
tcappellari-bdai
as code owners
khughes-bdai
requested review from
a team,
mbrauckmann-bdai,
jcarpinelli-bdai,
bjin-bdai,
mgoh-bdai and
mvictora-bdai
khughes-bdai
force-pushed
the
khughes/SW-2049-stream-joint-gains
branch
from
61b56f5 to
7390aab
Compare
jcarpinelli-bdai
approved these changes
Feb 24, 2025
tcappellari-bdai
approved these changes
Feb 25, 2025
added 2 commits
February 25, 2025 11:45
Signed-off-by: Katie Hughes <khughes-bdai@theaiinstitute.com>
Signed-off-by: Katie Hughes <khughes-bdai@theaiinstitute.com>
khughes-bdai
force-pushed
the
khughes/SW-2049-stream-joint-gains
branch
from
f2b6813 to
749ed00
Compare
tcappellari-bdai
approved these changes
Feb 25, 2025
|
merging this in now as i verified that the user interface stays the same, will keep working on the follow up controller separately |
robodreamer
reviewed
Feb 28, 2025
Comment on lines
+267
to
+275
| for (size_t i = 0; i < njoints_; i++) { | ||
| // copy over current position, velocity, and load from state to current command | ||
| hw_commands_[command_interfaces_per_joint_ * i] = hw_states_[state_interfaces_per_joint_ * i]; | ||
| hw_commands_[command_interfaces_per_joint_ * i + 1] = hw_states_[state_interfaces_per_joint_ * i + 1]; | ||
| hw_commands_[command_interfaces_per_joint_ * i + 2] = hw_states_[state_interfaces_per_joint_ * i + 2]; | ||
| // Fill in k_q_p and k_qd_p gains from the initial_value field from the URDF | ||
| const auto& joint = info_.joints.at(i); | ||
| hw_commands_[command_interfaces_per_joint_ * i + 3] = std::stof(joint.command_interfaces.at(3).initial_value); | ||
| hw_commands_[command_interfaces_per_joint_ * i + 4] = std::stof(joint.command_interfaces.at(4).initial_value); |
There was a problem hiding this comment.
nit: we could replace the hard-coded offsets with pos_offset = 0, vel_offset = 1, load_offset = 2, ... to make it easier to parse and don't assume or rely on remembering a known order in the code.
robodreamer
reviewed
Feb 28, 2025
Comment on lines
+293
to
+295
| joint_commands_.load.at(i) = hw_commands_[command_interfaces_per_joint_ * i + 2]; | ||
| joint_commands_.k_q_p.at(i) = hw_commands_[command_interfaces_per_joint_ * i + 3]; | ||
| joint_commands_.k_qd_p.at(i) = hw_commands_[command_interfaces_per_joint_ * i + 4]; |
There was a problem hiding this comment.
With the constants defined for the offsets, all these lines will not rely on a specific ordering.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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_valuefield 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.
\