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.

Sign up for GitHub

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

Jump to bottom

Introduce Waypoint classes for describing targets with multiple points. #421

Merged
merged 18 commits into from
May 31, 2024
Merged

Introduce Waypoint classes for describing targets with multiple points. #421

merged 18 commits into from
May 31, 2024

Conversation

yck011522
Copy link
Contributor

This PR introduces a new class similar to Targets that are used for plan_motion(), but now for a sequence of targets. This is used for planning motions with multiple segments similar to the current plan_cartesian_motion().

The intended structure is a "Waypoints" parent class with two childs "FrameWaypoints" and "PointAxisWaypoints". First is used with a list of Frames similar to the current plan_cartesian_motion use case, second is used for 3D printing type of work.

What type of change is this?

  • Bug fix in a backwards-compatible manner.
  • New feature in a backwards-compatible manner.
  • Breaking change: bug fix or new feature that involve incompatible API changes.
  • Other (e.g. doc update, configuration, etc)

Checklist

Put an x in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.

  • I added a line to the CHANGELOG.md file in the Unreleased section under the most fitting heading (e.g. Added, Changed, Removed).
  • I ran all tests on my computer and it's all green (i.e. invoke test).
  • I ran lint on my computer and there are no errors (i.e. invoke lint).
  • I added new functions/classes and made them available on a second-level import, e.g. compas_fab.robots.CollisionMesh.
  • I have added tests that prove my fix is effective or that my feature works.
  • I have added necessary documentation (if appropriate)

@jf---
Copy link

jf--- commented Apr 27, 2024

PointAxisWaypoints is exciting!

@yck011522 yck011522 marked this pull request as ready for review May 3, 2024 06:22
@yck011522
Copy link
Contributor Author

@gonzalocasas ping

@yck011522 yck011522 self-assigned this May 10, 2024
yck011522
yck011522 commented May 29, 2024
View reviewed changes
src/compas_fab/robots/targets.py
@@ -261,9 +274,9 @@ def scaled(self, factor):
"""
target_point = self.target_point.scaled(factor)
tolerance_position = self.tolerance_position * factor if self.tolerance_position else None
target_z_axis = self.target_z_axis.scaled # Vector is unitized and is not scaled
target_z_axis = self.target_z_axis # Vector is unitized and is not scaled
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Mistake in the past

yck011522
yck011522 commented May 29, 2024
View reviewed changes
src/compas_fab/robots/targets.py
tool_coordinate_frame = self.tool_coordinate_frame.scaled(factor) if self.tool_coordinate_frame else None
return PointAxisTarget(target_point, target_z_axis, tool_coordinate_frame, tolerance_position, self.name)
return PointAxisTarget(target_point, target_z_axis, tolerance_position, tool_coordinate_frame, self.name)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also a stupid mistake in the past PR. I didnt have testing back then.

@yck011522 yck011522 requested a review from gonzalocasas May 31, 2024 07:11
gonzalocasas
gonzalocasas approved these changes May 31, 2024
View reviewed changes
Copy link
Member

@gonzalocasas gonzalocasas left a comment

Choose a reason for hiding this comment

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

LGTM! 🚀

gonzalocasas
gonzalocasas reviewed May 31, 2024
View reviewed changes
src/compas_fab/robots/targets.py
Waypoints are useful for tasks like painting, welding, or 3D printing, where the programmer
wants to define the waypoints the robot should pass through.

Waypoints are intended to be used for motion planning with a planning backend by using :meth:`compas_fab.robot.plan_motion_with_waypoints`.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Waypoints are intended to be used for motion planning with a planning backend by using :meth:`compas_fab.robot.plan_motion_with_waypoints`.
Waypoints are intended to be used for motion planning with a planning backend by using :meth:`compas_fab.robot.plan_cartesian_motion`.

@gonzalocasas gonzalocasas merged commit 506fd5e into main May 31, 2024
14 checks passed
@gonzalocasas gonzalocasas deleted the feature-multiple-targets-waypoints branch May 31, 2024 07:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gonzalocasas gonzalocasas gonzalocasas approved these changes

Assignees

@yck011522 yck011522

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@yck011522 @jf--- @gonzalocasas