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

Rover simulation update #67

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Rover simulation update #67

wants to merge 3 commits into from

Conversation

chfriedrich98
Copy link
Contributor

@chfriedrich98 chfriedrich98 commented Nov 25, 2024 •
edited
Loading

This PR adresses the following points:

  • Increase the friction parameter in the rover world to reduce the amount of sliding of ground vehicles
  • Clean up the sdf of the r1_rover model: There is some code left over that doesn't actually do anything, fixed formatting errors and some of the naming is unnecessary complicated.
  • Remove the motor limits from the ackermann model. Motor limits in the sdf lead to weird stick input mapping (i.e. 70 - 100% throttle all lead to the same motor output). These limits are now handled with the SIM_GZ_WH_MIN /SIM_GZ_WH_MAX parameters instead.
  • Add mecanum model: Add a simulation model for the third rover type that has received its own module in PX4 (https://github.com/PX4/PX4-Autopilot/tree/main/src/modules/rover_mecanum)
    Screenshot from 2024-11-26 17-04-33

@chfriedrich98 chfriedrich98 self-assigned this Nov 25, 2024
Jaeyoung-Lim
Jaeyoung-Lim approved these changes Nov 26, 2024
View reviewed changes
Copy link
Member

@Jaeyoung-Lim Jaeyoung-Lim left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Thanks for the improvements and cleanup!

Mostly looks LGTM. Just added some minor comments.

Also, would be nice if you can add some screenshots/gifs here of the mecanuum model for better tracking what feature went in 😉

models/r1_rover_mecanum/model.config
<name>r1_rover</name>
<version>1.0</version>
<sdf version='1.5'>r1_rover.sdf</sdf>
<license>https://github.com/aionrobotics/aion_io/blob/master/LICENSE.txt</license>
Copy link
Member

Choose a reason for hiding this comment

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

This path doesn't seem to be valid. Can you recheck if this license is still applicable?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Seems like the aion robotics repo is now private.
Maybe @PerFrivik knows more about the license ?

models/rover_ackermann/model.sdf Outdated Show resolved Hide resolved
@chfriedrich98
Copy link
Contributor Author

Squashed some of the commits together to provide a cleaner history (no changes to the code).

@Jaeyoung-Lim Does the license issue block this PR or could we merge it?

@Jaeyoung-Lim
Copy link
Member

@chfriedrich98 The license issue is blocking. We cant merge things with licenses that are unclear.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Jaeyoung-Lim Jaeyoung-Lim Jaeyoung-Lim approved these changes

Assignees

@chfriedrich98 chfriedrich98

Labels
None yet
Projects
PX4 Rover
Status: 👀 In Review
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@chfriedrich98 @Jaeyoung-Lim