This repository has been archived by the owner on Aug 3, 2020. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 104
Add recursive mimic joint #177
Merged
Merged
Changes from 1 commit
Commits
Show all changes
15 commits
Select commit
Hold shift + click to select a range
6f7cc85
Add recursive mimic joint
alextoind 258de29
Fix crash on 0 free joints, opens empty window (#178)
bmagyar c9d0168
changelogs
wjwwood 7563f00
1.12.7
wjwwood fc71f00
remove divide by 2 when writing boxes to collada format (#133)
jacquelinekay 4d03177
Do a few cleanup tasks in collada_urdf (#183)
clalancette 3c7933b
fix missed mandatory -std=c++11 flag (#181)
4eetah 6218443
add Chris and Shane as maintainers (#184)
wjwwood 3384a0a
Allow supplying NodeHandle for initParam (#168)
piyushk 2755753
Remove old gazebo settings.
clalancette 05b4a41
Fix recursive offset and add infinite loop check
alextoind a88c94e
Fix computation
alextoind 139299d
Merge branch 'patch-1' of https://github.com/alextoind/robot_model in…
sloretz 4c59e35
Added joint_state_publisher tests for mimic_chain and mimic_cycle
sloretz 3e97af7
Make URDF valid
sloretz File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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.
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 think you should take the parent offset into account too. Why would you not?
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 believed that too at first: I summed up all the offset, I tried it with a simple three-joint scheme and I made some tests with ad-hoc values. Results? Everything concerning the revolution motion was right. The offset was... Strange.
I thought about it for a while and I concluded that a user is expecting to set the speed of the joint w.r.t. the whole chain of recursive joints, but the initial placement is logical to be w.r.t. the immediate first parent from which the URDF is computing the distance.
Example: imagine to have three joints
Ja
mimicsJb
which mimicsJc
. And suppose thatJb
has an offset ofpi/2
w.r.t.Jc
. If you sum up all the offset, even ifJa
has no offset w.r.t.Jb
, it ends to inherit the other offsets and to refer them toJb
.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.
Hi @alextoind, thank you for your answer, and sorry for my late reply
I would suggest to implement exactly what the spec/doc states instead of guessing what users have in mind (they might all have a different thing in mind).
Here is the mimic doc:
this tag is used to specify that the defined joint mimics another existing joint. The value of this joint can be computed as value = multiplier * other_joint_value + offset.
I'm not sure to understand. Is this the example you imagined?
Then I would expect the following behavior:
Do you agree?
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.
Yes, I agree with you about the last formula (which I've already computed one month ago before testing this PR). Expanded it becomes the following:
Nonetheless I chose the other implementation because of some specific cases that I tested and which seem to me a nonsense.
With the URDF I'm saying something like:
Jb
has to movex
time faster thanJc
with an offset ofy
radians w.r.t.Jc
itself.Ja
has to movex'
time faster thanJb
with an offset ofy'
radians w.r.t.Jb
itself.Now:
x
andx'
are2
, i.e. twice the velocity of their parent.y = 0.8' and
y' = -1.6'.Following the formula
Ja
will move four time as faster asJc
(pretty fine) and it will start in the same location ofJb
(2 * 0.8 - 1.6 = 0
). This seems to me non intuitive starting from the URDF, because it forces me to think of all the interactions within the whole recursive chain.Now, I see that it could be a feature for someone, but I would like to be sure that you are aware of this behavior.
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.
A part from the specific example, with the above forumlation the starting position of
Ja
depends not only on its offset, but also from its velocity ratio: if I set my application and later I need to improve a bit the factor ofJa
for a faster motion (x' = 3
in the example), I must also modify its offset to guarantee that the starting position remains unchanged (y' = -2.4
if I want thatJa
andJb
start from the same location).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.
agreed
I'm aware of the behavior, and it seems sane to me.
I really think it is the only possible interpretation of the URDF spec.
I have the opposite impression ;)
equation (1) and (2) above are the direct translations of the URDF mimic tags. There is no need to consider the whole chain to understand them and they combine naturally into equation (3).
If we were instead to implement
Then, since it is incompatible with equation (2), when reading the urdf we would need to look at Jb definition to know if I need to compute Ja from equation (2) or equation (3').
yes.
Yes, that seems normal to me too.
The usages of the mimic tags (and the URDF in general) I know of are not to describe behavior but hardware. For instance mimic tags are used for NAO and Pepper hands (a single motor drives all the finger joints) and for NAO HipPitch joints (again two joints driven by a single motor).
I don't know what you're trying to achieve, but it seems out of URDF's (mimic tag) scope.
Would you detail what you are trying to do?
It's probably possible to address your need in joint_state_publisher without changing the interpretation of the URDF files.
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 even use offset in my application, so it is definitely not a problem to me.
I just wanted to make a useful change for the community and I thank you for your time and experience in this field.