Skip to content

Adds MecanumControllerCommand to Commands2. #35

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

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

Conversation

lospugs
Copy link
Contributor

@lospugs lospugs commented Dec 6, 2023

#28

Copy link
Member

@virtuald virtuald left a comment

Choose a reason for hiding this comment

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

mypy failed

ChassisSpeeds(initialXVelocity, initialYVelocity, 0.0)
)
self.timer.restart()
self.prevTime = 0
Copy link
Member

Choose a reason for hiding this comment

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

prevTime isn't reset in Java?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It isn't reset in Java but I guess I just sort of did that because that's how I teach the students to write commands when I teach them.

Not really a bug per se in Java, but I think it should probably be reset to 0 in initialize in Java. I removed it here for parity.

Comment on lines +92 to +99
feedforward
or frontLeftController
or rearLeftController
or frontRightController
or rearRightController
or currentWheelSpeedsSupplier is None
):
raise RuntimeError(
Copy link
Member

Choose a reason for hiding this comment

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

If any of

  • feedforward
  • frontLeftController
  • rearLeftController
  • frontRightController
  • rearRightController

are provided (i.e. not None), won't we always get here? This doesn't seem right.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants