-
Notifications
You must be signed in to change notification settings - Fork 29
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
Support for continuous joints and fixed joints in URDF #16
base: main
Are you sure you want to change the base?
Conversation
Thanks, Bohao! Adding fixed joint support was something that I've had on the roadmap. So it's great to see the support in this PR. Let's work together to finish this out. Do you have some example URDFs that you're looking to support that you might be able to share? If so, perhaps we can add some unit tests that compare some select computations (e.g., inverse dynamics and mass matrix) with the robot systems toolbox for verification. Regarding the fixed joint, there is one other place that I think needs to be addressed. See the
I'm expecting this could return 0 for both
|
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.
This is a great start! With an example system, a unit test (happy to help on that one), and resolution to the operation in remove_fixed_joints
, I think this will be ready to merge in. Thanks for all your efforts to pull this together!
robot = importrobot(file); | ||
model = struct(); | ||
model.NB = robot.NumBodies; | ||
if nargin == 1 | ||
addRandomInertia = 0; | ||
end | ||
|
||
if nargin < 3 | ||
removeFixedJoints = 1; |
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 = i + 1; | ||
else | ||
model.NB = model.NB - 1; | ||
model.parent(i) = []; |
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.
Have you tried using the resulting model in any of the conventional algorithms? My suspicion is that leaving an "empty" body in the list will lead to trouble. For example, the empty XTree
cannot be applied to anything, and the empty parent would cause the kinematics propagation to fail as well.
|
||
for j = i:model.NB | ||
if model.parent(j) >= i | ||
model.parent(j) = model.parent(j) - 1; |
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.
This line seems to be assuming that you are removing the body from the list and decrementing the body labels of everyone else in the tree by one? However all the old model data (e.g., for I
, Xtree
, etc.) remains stored at the original index.
I might be mentally parsing this incorrectly -- if so, I think an example URDF will help a great deal to instill confidence.
Hi, here's a simple add-up for support for continuous joints and fixed joints appearing in the URDF of some of the robot arms.
I did not include sufficient tests for the fixed joint features yet. By modifying "jcalc", all functions should still work for a model with fixed joints technically. The robot states still need to include these fixed joints but the states will only be served as placeholders since they will not actually change the position, velocity, and acceleration of the fixed joints.
Please feel free to edit the code directly to match your original style. I'm happy to help with any additional related features. Thanks for your review in advance!