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

Mecanum wheels demo #683

Merged
merged 18 commits into from
Mar 24, 2021
Merged

Mecanum wheels demo #683

merged 18 commits into from
Mar 24, 2021

Conversation

scpeters
Copy link
Member

🎉 New feature

Closes #579

Summary

This adds an example world that is based on the diff_drive_skid example but uses a new feature in dartsim exposed by an SDFormat attribute in a custom xml namespace in gazebosim/gz-physics#224. The friction parameters are modified in 48010ca to use a friction direction that is constant in the chassis frame and points at a 45 degree angle from each wheel's revolute joint axis. The friction coefficients are set to 1.0 and 0.0 in the perpendicular directions.

Test it

The vehicle can be operated like a skid-steer vehicle using the diff-drive system that is currently attached. I need to create a new system to enable driving sideways.

To see it move sideways, you can do the following:

  1. comment out the diff-drive plugin
  2. modify the gravity vector to have a non-zero value in the x or y direction
  3. rotate the vehicle so that the x/y gravity component is pointing in the lateral direction of the vehicle
  4. watch it go sideways

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

scpeters added 3 commits March 14, 2021 00:18
@scpeters
Initial diff_drive_mecanum example world
17df362 
Initial version copied from diff_drive_skid example.

Signed-off-by: Steve Peters <[email protected]>
@scpeters
Set friction parameters for mecanum wheel model
48010ca 
This modifies the friction parameters of the wheels in the
skid-steer example vehicle to behave like Mecanum wheels,
with anisotropic friction and friction directions that
are constant in the chassis frame. To specify these
friction directions, the //fdir1/@Gazebo:expressed_in
attribute is used. The friction values are specified as 1.0
in the direction corresponding to the Mecanum wheel roller
axes, and 0.0 in the perpendicular direction, which
corresponds to the rolling direction.

Signed-off-by: Steve Peters <[email protected]>
@scpeters
debug: box visuals for wheels to see rotations
f0b9962 
Signed-off-by: Steve Peters <[email protected]>
@scpeters scpeters requested review from azeey and chapulina March 14, 2021 08:34
@github-actions github-actions bot added the 🏢 edifice Ignition Edifice label Mar 14, 2021
@chapulina chapulina added enhancement New feature or request needs upstream release Blocked by a release of an upstream library beta Targeting beta release of upcoming collection labels Mar 15, 2021
scpeters added 4 commits March 19, 2021 01:07
@scpeters
Duplicate diff_drive system as mecanum_drive
9f30a09 
No files changed yet, this is only a file copy.

Signed-off-by: Steve Peters <[email protected]>
@scpeters
Use ignition as xml namespace
7ce81bd 
Signed-off-by: Steve Peters <[email protected]>
@scpeters
Merge branch 'mecanum_wheels_demo' of github.com:scpeters/ign-gazebo …
349714b 
…into mecanum_wheels_demo
@scpeters
Rename DiffDrive.* to MecanumDrive.*
a270912 
Signed-off-by: Steve Peters <[email protected]>
chapulina
chapulina reviewed Mar 22, 2021
View reviewed changes
examples/worlds/diff_drive_mecanum.sdf Outdated Show resolved Hide resolved
src/systems/mecanum_drive/MecanumDrive.hh Outdated Show resolved Hide resolved
src/systems/mecanum_drive/SpeedLimiter.cc Outdated Show resolved Hide resolved
@scpeters
Copy link
Member Author

ok, I just got a working joystick configuration with an ign-launch file in scpeters/ign-launch@ad0efb2

on my gamepad, the left analog stick commands longitudinal and lateral velocity, and the horizontal part of the right analog stick is used for turning

@scpeters scpeters marked this pull request as ready for review March 23, 2021 06:57
@scpeters
Copy link
Member Author

I think this is ready for review. It's missing the Odometry, but the controller works

azeey
azeey approved these changes Mar 24, 2021
View reviewed changes
Copy link
Contributor

@azeey azeey left a comment

Choose a reason for hiding this comment

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

This is so cool!! Left a few minor comments. If you don't have time to implement the odometry, I'd suggest removing the functionality completely and revisiting later.

examples/worlds/diff_drive_mecanum.sdf Outdated Show resolved Hide resolved
examples/worlds/mecanum_drive.sdf Show resolved Hide resolved
src/systems/mecanum_drive/MecanumDrive.cc
}

// If the joints haven't been identified yet, look for them
static std::set<std::string> warnedModels;
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure why warnedModels is a set. Can there be more than one model here?

src/systems/mecanum_drive/MecanumDrive.cc

// constant used in computing target velocities
const double angularLength = 0.5 * (this->wheelSeparation + this->wheelbase);
const double invWheelRadius = 1 / this->wheelRadius;
Copy link
Contributor

Choose a reason for hiding this comment

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

Can wheelRadius be 0?

chapulina
chapulina reviewed Mar 24, 2021
View reviewed changes
Copy link
Contributor

@chapulina chapulina left a comment

Choose a reason for hiding this comment

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

It works! Just some smaller comments from @azeey and me.

mecanum_lat

src/systems/mecanum_drive/MecanumDrive.cc
public: transport::Node node;

/// \brief Entity of the front left joint
public: std::vector<Entity> frontLeftJoints;
Copy link
Contributor

Choose a reason for hiding this comment

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

I've never seen a vehicle with more than 4 mecanum wheels, is that possible? If not, we probably should just store a single Entity for each joint.

src/systems/mecanum_drive/MecanumDrive.cc
{
double minVel = _sdf->Get<double>("min_velocity");
this->dataPtr->limiterLin->SetMinVelocity(minVel);
this->dataPtr->limiterAng->SetMinVelocity(minVel);
Copy link
Contributor

Choose a reason for hiding this comment

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

I realize that this is just following the pattern from other systems, but I'm wondering if we should split linear and angular components, considering that they have different units.

src/systems/mecanum_drive/MecanumDrive.cc

double linVel;
double latVel;
double angVel;
Copy link
Contributor

Choose a reason for hiding this comment

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

Small suggestion, instead of 3 vars, this could use a Commands object

examples/worlds/diff_drive_mecanum.sdf Outdated Show resolved Hide resolved
examples/worlds/mecanum_drive.sdf Outdated Show resolved Hide resolved
@scpeters
Copy link
Member Author

If you don't have time to implement the odometry, I'd suggest removing the functionality completely and revisiting later.

removed in 49eb8cb

@scpeters scpeters force-pushed the mecanum_wheels_demo branch from 53c2823 to bcfb881 Compare March 24, 2021 08:08
@scpeters
Copy link
Member Author

I haven't yet addressed some of the comments related to the code in MecanumDrive. I wasn't very thoughtful in architecting it, I just copied DiffDrive and added to it. I'm not sure I'll have time to address those concerns before code freeze given other higher priority pull requests. Maybe addressing the current set of comments is doable, but I'm sure there are more and more issues to be found here if reviews continue getting stricter and I'd rather focus elsewhere than polishing this one

chapulina
chapulina approved these changes Mar 24, 2021
View reviewed changes
Copy link
Contributor

@chapulina chapulina left a comment

Choose a reason for hiding this comment

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

LGTM as a first revision! I'll come back to this after the release and address the remaining comments, which should be backwards compatible.

@chapulina chapulina merged commit dfeef53 into gazebosim:main Mar 24, 2021
@scpeters scpeters deleted the mecanum_wheels_demo branch March 25, 2021 16:58
@scpeters scpeters mentioned this pull request May 17, 2021
Closed
This was referenced Aug 25, 2022
Closed
Merged
Closed
@scpeters scpeters mentioned this pull request Dec 10, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chapulina chapulina chapulina approved these changes

@azeey azeey azeey approved these changes

Assignees

@scpeters scpeters

Labels
beta Targeting beta release of upcoming collection 🏢 edifice Ignition Edifice enhancement New feature or request needs upstream release Blocked by a release of an upstream library
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@scpeters @azeey @chapulina