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

mecanum: new features #24036

Merged
merged 8 commits into from
Dec 2, 2024
Merged

mecanum: new features #24036

merged 8 commits into from
Dec 2, 2024

Conversation

chfriedrich98
Copy link
Contributor

Solved Problem

This PR adds the following new features to the mecanum module:

  1. Deprecate RM_MISS_SPD_DEF: This brings the module in line with the other 2 rover modules. The default mission speed is now simply the maximum speed RM_MAX_SPEED or specified with a speed waypoint.

  2. Deprecate RM_MAN_YAW_SCALE: This is also to bring the module in line with the other rover modules. This parameter is removed and its functionality is instead provided by RM_MAX_YAW_RATE and RM_MAX_THR_YAW_R (which already existed).

  3. Add slew rates for the yaw, yaw rate and speed setpoints. Same idea/implementation as with the differential module (differential: add slew rates for speed, yaw and yaw rate setpoints #23812). Adds 2 new parameters RM_MAX_DECEL and RM_MAX_YAW_ACCEL.

  4. Fix inverse kinematics: The yaw rate prioritization was implemented wrong. The prioritization of the yaw rate over the velocity caused the rover to completely disregard the velocity setpoint if for example the user was giving full forward throttle + some yaw rate input while keeping the lateral throttle at 0. This happend because of an if statement that checked the sign consistency of the original and reduced speed setpoints which could fail due to floating point precision.
    The updated inverse kinematics instead scales the requested normalized velocity vector to be withing a square that is scaled based on the control effort that is left after applying the required motor commands for the yaw rate.
    This way the desired yaw rate is achieved (prioritized over speed) and the velocity direction is maintained while ensuring that none of the normalized motor commands exceed [-1, 1].

  5. Adjust speed setpoints to always be feasible: Using the same calculation as the new inverse kinematics, the speed setpoint in closed loop speed control is adjusted based on the yaw rate setpoint s.t. the setpoint is always feasible.

  6. Update the mecanum SITL airframe with the new/deprecated parameters.

Test coverage

  • Tested in SITL
  • Tested on hardware

@chfriedrich98 chfriedrich98 added the Rover 🚙 Rovers and other UGV label Nov 25, 2024
@chfriedrich98 chfriedrich98 self-assigned this Nov 25, 2024
@mrpollo
Copy link
Contributor

mrpollo commented Nov 25, 2024

@chfriedrich98
Copy link
Contributor Author

@mrpollo Thanks for the heads up. It is caused by one of these commits (5640287 or e2d553c) which are actually part of this PR: #23931.
The reason they are in here is because my PR builds on this other one and the things I add are ready to be reviewed.
It will of course not go in like this, but rather after that other PR was fixed/merged which is why it is in draft.

@MaEtUgR
Copy link
Member

MaEtUgR commented Nov 26, 2024

The failing unit tests were my PRs fault sry 😇

sfuhrer
sfuhrer previously approved these changes Nov 27, 2024
Copy link
Contributor

@sfuhrer sfuhrer left a comment

Choose a reason for hiding this comment

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

Makes conceptually sense and looks clean in the implementation. Didn't check the code it too much detail but given that tested it well and it follows the same patterns for all rover type I'm okay with bringing it in like this.

src/modules/rover_mecanum/module.yaml Outdated Show resolved Hide resolved
@chfriedrich98 chfriedrich98 merged commit be2a3af into PX4:main Dec 2, 2024
53 of 59 checks passed
@chfriedrich98 chfriedrich98 deleted the mecanum_update branch December 2, 2024 09:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Rover 🚙 Rovers and other UGV
Projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

4 participants