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] Change definition of stalled #1069

Open
laurensvalk opened this issue May 11, 2023 · 6 comments
Open

[Feature] Change definition of stalled #1069

laurensvalk opened this issue May 11, 2023 · 6 comments
Labels
enhancement New feature or request topic: control Issues involving control system algorithms topic: motors Issues involving motors

Comments

@laurensvalk
Copy link
Member

laurensvalk commented May 11, 2023

Current implementation

Currently stalled is true if:

  1. The proportional part of PID exceeds the (configurable) limit and;
  2. The motor is stopped, i.e. speed below (configurable) limit and;
  3. it has been like this for some (configurable) time.

Suggested improvement: Drop condition 2

Stall detection is mostly used to detect over load in order to stop. We don't need to wait until we are physically stopped to test this.

Doing this makes the stall detection more sensitive, and it makes run_until_stalled work better. We can use a configurable torque limit (test 1 above) instead of a duty limit. This means you can run at any speed and still safely detect soft endpoints. With duty_limit today, this doesn't work because capping it at say 30% means you can't run fast.

I think it is still consistent with our current definition:

image

Drawbacks
Maybe this is no longer called "stalled" this way, since it may not be fully stopped?

It would also raise the stall flag way sooner when running very fast. This is not technically an issue since all other maneuvers don't look at this flag but it is a bit odd.

Maybe loaded is a better term. Not sure if we need both though.

@laurensvalk laurensvalk added enhancement New feature or request topic: motors Issues involving motors topic: control Issues involving control system algorithms labels May 11, 2023
@dlech
Copy link
Member

dlech commented May 11, 2023

Instead of changing the definition, can we just add a new run_until_overload(speed, torque_limit, then) -> int and overload() -> bool and deprecate stalled?

I think an overload condition is also useful for discovering that you are trying to run at a speed that the motor is not physically capable of.

@laurensvalk
Copy link
Member Author

Part of this is getting mixed in with pybricks/pybricks-micropython#166 to make some progress.

I need to pull out the motor servo stuff, then rebase that PR.

@laurensvalk
Copy link
Member Author

Yes, as mentioned something like overload could work, I'm just not sure what to do with stalled as having both would be somewhat costly.

I'm wondering if it is acceptable to deprecate stalled by just aliasing it to the new variant. Old scripts would run, but with slightly different behavior.

Although pbio-derivatives will probably want to keep stalled available as well...

laurensvalk added a commit to pybricks/pybricks-micropython that referenced this issue May 22, 2023
This is an alternate way to use temporary limits in pbio (no nlr buf)
but without a breaking change in the definition of duty limit or stall.

See pybricks/support#1069
laurensvalk added a commit to pybricks/pybricks-micropython that referenced this issue May 22, 2023
This is an alternate way to use temporary limits in pbio (no nlr buf)
but without a breaking change in the definition of duty limit or stall.

See pybricks/support#1069
laurensvalk added a commit to pybricks/pybricks-micropython that referenced this issue May 26, 2023
This is an alternate way to use temporary limits in pbio (no nlr buf)
but without a breaking change in the definition of duty limit or stall.

See pybricks/support#1069
laurensvalk added a commit to pybricks/pybricks-micropython that referenced this issue May 26, 2023
This is an alternate way to use temporary limits in pbio (no nlr buf)
but without a breaking change in the definition of duty limit or stall.

See pybricks/support#1069
@BertLindeman
Copy link

Maybe not 100% on this item, but as a simple user I would expect True as the result of motor.stalled() after running motor.run_until_stalled(500).

Currently running this on ('primehub', '3.3.0b8', 'v1.20.0-23-g6c633a8dd on 2023-07-07')

    print("angle before closing", gripper.angle())
    gripper.run_until_stalled(500)
    print("gripper motor stalled ", gripper.stalled())
    print("angle after ", gripper.angle())

which shows:

angle before closing 83
gripper motor stalled  False
angle after  478

@laurensvalk
Copy link
Member Author

By default, run_until_stalled uses Stop.COAST. When it coasts, it is no longer stalled.

If you change it to Stop.HOLD, you should find it still stalled.

@BertLindeman
Copy link

It does not.
Not with V3.2.3 and not with v3.3.0b8.
Should I make another discussion to not pollute this one further?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request topic: control Issues involving control system algorithms topic: motors Issues involving motors
Projects
None yet
Development

No branches or pull requests

3 participants