-
Notifications
You must be signed in to change notification settings - Fork 60
[RSDK-10924] Add IsHoldingSomething to Gripper API #935
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
[RSDK-10924] Add IsHoldingSomething to Gripper API #935
Conversation
…d57bbb650e95807d8b89a4b84408a
Warning your change may break code samples. If your change modifies any of the following functions please contact @viamrobotics/fleet-management. Thanks!
|
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.
Minor change request
from typing import Any, Dict, Final, Optional, Tuple | ||
|
||
from viam.components.component_base import ComponentBase | ||
from viam.resource.types import API, RESOURCE_NAMESPACE_RDK, RESOURCE_TYPE_COMPONENT | ||
|
||
from . import KinematicsFileFormat | ||
|
||
@dataclass |
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.
Can we move this inside of Gripper
? that way this class is Gripper.HoldingStatus
?
I don't think this class makes sense outside the context of a gripper
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 modulo Naveed's comment and a question for my own understanding.
# Grab with the gripper. | ||
holding_status = await my_gripper.is_holding_something() | ||
# get the boolean result | ||
is_holding_something = holding_status.is_holding_something |
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.
(q) naively, is_holding_something
is the only thing I care about here and I don't understand what the meta
field does at all, so this looks to me like we're just asking for an extra step to get the relevant information. Can you clarify what the meta
field in a HoldingStatus
actually is?
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'll add a comment, but it's something eliot specifically asked for here: https://docs.google.com/document/d/1N1TRTOMww_4NUNDVbeeWg9raNpSK0Srb16kG4R2qYks/edit?disco=AAABlaa3cpE
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.
🤷 okay fair enough!
No description provided.