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

New trajectory follower system #1332

Merged
merged 6 commits into from
Feb 13, 2022
Merged

Conversation

caguero
Copy link
Contributor

@caguero caguero commented Feb 10, 2022

Signed-off-by: Carlos Agüero [email protected]

🎉 New feature

Summary

This system adds a very basic trajectory follower applied to a model. The trajectory is performed by applying forces and torques to a link. It's possible to specify the trajectory in three ways:

  • Providing a set of waypoints.
  • Specifying a segment relative to the starting pose of the model.
  • Specifying a circle relative to the starting pose of the model.

Note that trajectories are in 2D.

Test it

Run the trajectory_follower.sdf example to see it in action with a box.

You can see a more interesting example here.

trajectory_follower

Checklist

  • Signed all commits for DCO
  • Added tests
  • Added example and/or tutorial
  • Updated documentation (as needed)
  • Updated migration guide (as needed)
  • codecheck passed (See contributing)
  • All tests passed (See test coverage)
  • While waiting for a review on your PR, please help review another open pull request to support the maintainers

Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining Signed-off-by messages.

Signed-off-by: Carlos Agüero <[email protected]>
@caguero caguero requested a review from chapulina as a code owner February 10, 2022 16:50
@github-actions github-actions bot added the 🏯 fortress Ignition Fortress label Feb 10, 2022
Copy link
Contributor

@iche033 iche033 left a comment

Choose a reason for hiding this comment

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

works well for me after fixing the force that's applied to objects.
left a couple of other minor suggestions.

filename="ignition-gazebo-trajectory-follower-system"
name="ignition::gazebo::systems::TrajectoryFollower">
<link_name>box_link</link_name>
<loop_forever>true</loop_forever>
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion to name this <loop> for consistency with actor's loop param

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed in f082490.

if (this->dataPtr->localWaypoints.empty())
return;

//this->dataPtr->modelPose = this->dataPtr->link.WorldPose(_ecm);
Copy link
Contributor

Choose a reason for hiding this comment

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

remove?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed in f082490.

//this->dataPtr->modelPose = this->dataPtr->link.WorldPose(_ecm);
this->dataPtr->modelPose = ignition::gazebo::worldPose(
this->dataPtr->link.Entity(), _ecm);
// (*this->dataPtr->modelPose).Pos().Z() = 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

remove?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed in f082490.


// Parse the optional <torque> element.
if (_sdf->HasElement("torque"))
this->forceToApply = _sdf->Get<double>("torque");
Copy link
Contributor

Choose a reason for hiding this comment

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

forceToApply -> torqueToApply

Copy link
Contributor

Choose a reason for hiding this comment

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

after fixing this, I made both <force> and <torque> in demo world file to be 10 and the box moves nicely without too much overshooting when rotating

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch, changed in f082490.

/// The block should contain the relative direction and distance from
/// the initial position in which the vehicle should move, specified
/// in the world frame.
/// <direction>: Relative direction (degrees) in the world frame for
Copy link
Contributor

Choose a reason for hiding this comment

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

I think most of our angles are specified in radians? For the <pose> element in particular, there is a bool degrees attribute to indicate if it's degrees or not. Don't have strong opinion on this though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed to radians in f082490.

Copy link
Contributor

Choose a reason for hiding this comment

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

thanks, I updated doc in 0927042

Signed-off-by: Carlos Agüero <[email protected]>
Signed-off-by: Carlos Agüero <[email protected]>
Copy link
Contributor

@iche033 iche033 left a comment

Choose a reason for hiding this comment

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

looks good to me.

@iche033
Copy link
Contributor

iche033 commented Feb 11, 2022

there's a windows build error:

gazebo\src\systems\trajectory_follower\TrajectoryFollower.cc(168,20): error C2065: 'M_PI': undeclared identifier

Signed-off-by: Ian Chen <[email protected]>
@iche033
Copy link
Contributor

iche033 commented Feb 12, 2022

pushed a fix in 9a1ddd9

Signed-off-by: Ian Chen <[email protected]>
@codecov
Copy link

codecov bot commented Feb 12, 2022

Codecov Report

Merging #1332 (8d365d0) into ign-gazebo6 (77dd855) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##           ign-gazebo6    #1332   +/-   ##
============================================
  Coverage        62.06%   62.06%           
============================================
  Files              278      278           
  Lines            23378    23378           
============================================
  Hits             14510    14510           
  Misses            8868     8868           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 77dd855...8d365d0. Read the comment docs.

@caguero caguero merged commit 49d0673 into ign-gazebo6 Feb 13, 2022
@caguero caguero deleted the caguero/trajectory_follower branch February 13, 2022 18:03
@iche033 iche033 restored the caguero/trajectory_follower branch February 17, 2022 20:53
@iche033 iche033 deleted the caguero/trajectory_follower branch February 17, 2022 20:55
@osrf-triage
Copy link

This pull request has been mentioned on Gazebo Community. There might be relevant details there:

https://community.gazebosim.org/t/new-ignition-releases-2022-03-01-citadel-edifice-fortress/1313/1

@chapulina
Copy link
Contributor

I'm porting this forward in #1380 and it doesn't work out of the box in Garden. I'm looking into it.

I also noticed that the plugin was added without a test. We should add one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏯 fortress Ignition Fortress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants