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

Update for robot/util clean-up in AIKIDO. #47

Merged
merged 1 commit into from
Oct 23, 2020
Merged

Conversation

sniyaz
Copy link

@sniyaz sniyaz commented Oct 15, 2020

As part of personalrobotics/aikido#588, we killed both

  1. CRRT functionality
  2. The planning methods in robot/util

This PR removes the former methods from libada so we still build, and calls snap planner directly to handle the latter in executePreshape().

Testing: I've tested this in sim using both feeding_demo and simple_trajectories from ada_demos, and all seems well.


Before creating a pull request

  • Document new methods and classes
  • Format code with make format

Before merging a pull request

  • Add unit test(s) for this change

@sniyaz sniyaz requested review from egordon and brianhou October 15, 2020 21:38
Copy link
Collaborator

@egordon egordon left a comment

Choose a reason for hiding this comment

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

LGTM, bonus is that these functions were basically never used in the current feeding demo, so I'm not that concerned.

// explicitly call SnapPlanner
auto trajectory = aikido::robot::util::planToConfiguration(
mSpace, mHand, goalState, satisfied, nullptr, 1.0);
// Snap planner is the only planner that makes sense for this.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hard to comment on this part, as I haven't used a DART planner directly before. With that said, we use this function basically never in the current feeding demo, so I'm okay letting it through.

But eventually this should be a call in the new Robot class, right? If so, then this is fine for now. If not, we should consider adding a TODO, since I feel like this is functionality we should add to a Robot class derivative.

@egordon egordon merged commit 7c3cb88 into master Oct 23, 2020
@sniyaz sniyaz deleted the sniyaz_clean_util branch October 23, 2020 18:13
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.

4 participants