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

[Feature] DriveBase cannot brake, deceleration is too lazy #881

Closed
davecparker opened this issue Dec 24, 2022 · 6 comments
Closed

[Feature] DriveBase cannot brake, deceleration is too lazy #881

davecparker opened this issue Dec 24, 2022 · 6 comments
Labels
enhancement New feature or request software: pybricks-micropython Issues with Pybricks MicroPython firmware (or EV3 runtime) topic: motors Issues involving motors

Comments

@davecparker
Copy link

DriveBase has a stop() method, but this appears to only coast and there is no option to brake. Also, although acceleration works well, the deceleration behavior is not good for remote control (control loop) applications.

When using remote control, it is desirable to have a slower and smoother start, and specifying a lower acceleration can achieve this. However, this also makes the robot decelerate slowly, so once you lift off of the button to stop, the robot overshoots by a lot.

Adding the ability to brake would be an easy way for a user program to fix this.
Also for advanced users, being able to have the acceleration and deceleration ramps be different would be nice, if brake is too sudden.

@davecparker davecparker added enhancement New feature or request triage Issues that have not been triaged yet labels Dec 24, 2022
@dlech dlech added topic: motors Issues involving motors software: pybricks-micropython Issues with Pybricks MicroPython firmware (or EV3 runtime) and removed triage Issues that have not been triaged yet labels Dec 24, 2022
@laurensvalk
Copy link
Member

laurensvalk commented Dec 24, 2022

Which of the following would you prefer? Or both?

  • Add brake() method.
  • Allow acceleration and deceleration to be specified separately via the settings() method.

The good news is that you can technically already do both, so you could try to see what works best.

To try brake, just call brake() on both motors separately.

To try a separate deceleration value, perhaps test it in a separate program with a single motor first. You can pass a tuple to the acceleration value.

Now, the DriveBase has two of such control objects too, so the same trick works to set deceleration for distance_control. Using these control objects directly is nonintuitive, which is why the .settings() method exists. So we could adapt this method to accept (acceleration, deceleration) tuples.

@davecparker
Copy link
Author

Since you already have brake on Motor, I like the simplicity of adding a matching DriveBase.brake(), so that seems like a simple and easy addition. You could also argue that stop() should brake and drive(0, 0) should decelerate, but that would change current behavior.

For deceleration, I did see mention of the tuple for motors but wasn't sure how this would apply to a DriveBase, and as you describe above it's more complex than I would want a DriveBase user to deal with...

Since DriveBase.settings() says it applies to the measured movements (but actually applies to drive() too?), perhaps adding a new method that applies specifically to drive() and stop(), since that is really a different situation, and also deserves different defaults. I think the default decel for stop() should be pretty steep. If the default is good, then most users won't have to deal with the settings, given the choice between stop() for a steep decel and brake().

@laurensvalk
Copy link
Member

laurensvalk commented Dec 24, 2022

You could also argue that stop() should brake and drive(0, 0) should decelerate

drive(0, 0) already decelerates. That is, it does what drive does; there is nothing special about (0, 0).

stop() behaves the same as on a single motor. It coasts.

With the possible change to settings, I meant that we could allow both:

  • straight_acceleration=500
  • straight_acceleration=(500, 2000).

Since DriveBase.settings() says it applies to the measured movements (but actually applies to drive() too?)

Thanks, this sounds like it needs fixing. Indeed, the settings method should work consistently for everything now, including drive.

laurensvalk added a commit to pybricks/pybricks-micropython that referenced this issue Mar 17, 2023
This makes the limits() implementation easier to follow and we will
be able to re-use it for DriveBase deceleration.

See pybricks/support#881
@laurensvalk
Copy link
Member

Deceleration can now be set just like with a single motor: an (acceleration, deceleration) tuple is now accepted.

@laurensvalk
Copy link
Member

brake is not implemented yet, so I'll reopen.

@laurensvalk
Copy link
Member

Deceleration and passive brake are now both available!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request software: pybricks-micropython Issues with Pybricks MicroPython firmware (or EV3 runtime) topic: motors Issues involving motors
Projects
None yet
Development

No branches or pull requests

3 participants