Skip to content
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

WIP: Split arm class into a collection of pieces #1011

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Conversation

alberth
Copy link
Contributor

@alberth alberth commented Mar 7, 2020

Addresses #911
@jlunenburg A first small step

Copy link
Contributor

@jlunenburg jlunenburg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lijkt me een heel goed begin. Met wat extra stapjes kunnen we die armen echt composen uit meerdere componenten. Wel moeten we dit goed testen.

@@ -318,6 +322,188 @@ class GripperState(object):
OPEN = "open"
CLOSE = "close"

class ArmPiece(object):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still doubting the name 'Piece'. I'd expect that it will all be 'Interfaces', such as a JointInterface, CartesianInterface, ForceInterface, etc.

rospy.logerr(e)
return False

class ArmDatabasePiece(ArmPiece):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dit zijn specifiek joint goals en trajectories. Zouden we die niet strakker willen koppelen aan de ArmsJointPiece klasse? Overigens zou daar dan de bijbehorende action client in geconstrueerd kunnen worden en kunnen daar ook de has_joint_trajectory, send_joint_goals etc. bijhoren.

"""
return self.trajectories.get(configuration)

class ArmMarkerPublisherPiece(ArmPiece):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ik noemde al de CartesianInterface in een voorgaande comment. Deze functionaliteit zou daar onderdeel van kunnen zijn (publish_marker wordt nu alleen aangeroepen vanuit de send_goal functie)

grasp_precompute_goal.goal.yaw = yaw

self._publish_marker(grasp_precompute_goal, [1, 0, 0], "grasp_point")
grasp_goal = make_grasp_precompute_goal(frame_in_baselink, pre_grasp, first_joint_pos_only,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, deze hele functie wordt zo een stuk leesbaarder!


self._publish_marker(grasp_precompute_goal, [1, 0, 0], "grasp_point")
grasp_goal = make_grasp_precompute_goal(frame_in_baselink, pre_grasp, first_joint_pos_only,
allowed_touch_objects)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wat mij betreft zou dit dan een onderdeel worden van de CartesianInterface



class ForceSensingArm(Arm):
def __init__(self, robot_name, tf_listener, get_joint_states, side):
def __init__(self, robot_name, tf_listener, get_joint_states, side, joints_piece=None):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Zouden we dit dan niet ook anders moeten maken? Dus dat we b.v. een HeroArm hebben waar we al die pieces en een ForcePiece aan toevoegen?

@alberth
Copy link
Contributor Author

alberth commented Mar 23, 2020

Making small steps in #1023 and #1024 for real. Mostly only aiming for exporting code out of the arm currently, rather than also finding a final set of interface classes. Very likely, some of the code being moved out early will end up as part of the interface (or even completely internal) of other (larger) interfaces.
Code is just too intertwined and unfamiliar to me, to do all this at the same time.

@alberth alberth mentioned this pull request Mar 31, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants