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.
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
Added IO Gripper Control and Configuration Messages, Services, and Actions #164
base: master
Are you sure you want to change the base?
Added IO Gripper Control and Configuration Messages, Services, and Actions #164
Changes from 4 commits
65b2736
aece6f9
5ea829d
4b40c30
ebcff47
90a82bf
af79a5c
e823a9e
db1f368
d1d054a
c02e54b
5aeb6d2
d460ba8
017ed60
49f4a85
4427a63
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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 custom Gripper action maybe should be renamed to
IOGripperCommand
.@bmagyar the reason to have a separate action here is that this kind of gripper has only
open
/close
states, and nothing can be controlled much. Usually, those are the pneumatic ones so some cylinders are moving somehow, but we can not control intermediate states, i.e., "positions".Looking at the existing Actions and having, e.g.,
JointState
as input to the action (as done inParallelGripperAction
could be possible, but confusing and inconvenient. Possibly as a secondary interface, but as of now I don't see the usability of it, as we would command something implicitly that we cannot control.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.
Thanks for the suggestion. I have updated the interface names.
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.
What if you want to control the percentage of the closure?
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.
That's a good idea. I will update it.
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.
@saikishor in this case you cannot control the percentage of the closure. As there are IOs controlled behind it and usually pneumatic gripper that might have different states, but those are then different IOs, in most cases those have only 2 states,
open
andclose
and not a chance to detect anything in between.What here might be interesting, that we have different states besides
open
andclose
as we have 2 closed states and then we can set the target state of the gripper - bit this smells a lot to the configuration, so I am not sure if we should go this way, but rather we should keep only two states that we can command:open
andclose
.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.
@bmagyar
There is no similar Action as of now. The IO Gripper might be quite complex, to have different configurations that are independent of
open
/close
functionality. For example, configuration to grasp narrower and wider objects. In this case, the configuration has to be set before the grasping (and planning) as the collision model of the gripper changes by moving a degree of freedom through configuration.Independently of configuration, the
open
/close
stays the same, i.e., moving some other joints, but overall kinematics changes depending on the gripper configuration.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.
If it's just state, why not a percentage of closure instead of binary state
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.
We don't know the percentage, usually don't. We can typically have discrete sensors for open/close, but not other sensors. Assuming that there is a case where this can be detected we would need to add additional interface for it. So I would wait until we have this use-case.
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.
same comment as for the action.
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.
How do you configure just based on string?. I think It is missing some documentation
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.
@sachinkum0009 please add in general documentation about all fields in the message. Add those as comments at the begin of the message.
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.
general documentation about all fields in the message added.