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] Also reset heading target when resetting odometry. #1617

Closed
laurensvalk opened this issue Apr 28, 2024 · 8 comments
Closed

[Feature] Also reset heading target when resetting odometry. #1617

laurensvalk opened this issue Apr 28, 2024 · 8 comments
Labels
documentation Improvements or additions to documentation enhancement New feature or request software: pybricks-micropython Issues with Pybricks MicroPython firmware (or EV3 runtime) topic: control Issues involving control system algorithms topic: motors Issues involving motors

Comments

@laurensvalk
Copy link
Member

laurensvalk commented Apr 28, 2024

Is your feature request related to a problem? Please describe.
At the moment, calling drivebase.reset() resets the odometry offsets, but doesn't do anything to the controls. So if you are holding, you will keep holding.

Based on various discussions like https://github.com/orgs/pybricks/discussions/1616#discussioncomment-9251463, I think the user assumption is that this will also reset any internal controller states.

Describe the solution you'd like
I think it is reasonable to reset the states equivalent to calling stop(), so the user does not have to do this themselves, and so they don't forget about it.

drivebase.reset() is usually used when re-aligning the robot with the wall, so having a clean slate here makes sense, so it won't keep trying to maintain any previous targets in case it was holding.

I think it may also be reasonable to assume that the user will want to stop when calling drivebase.reset(), so maybe physically coasting isn't a bad idea. You could always immediately hold again if you wanted to.

When we do that, reset really does mean: reset back to the start, as if starting a new mission/program, which is probably what this function is going to be used for anyway.

This is a slightly breaking change though, so we'll want to clarify that in the documentation.

@laurensvalk laurensvalk added enhancement New feature or request topic: motors Issues involving motors topic: control Issues involving control system algorithms labels Apr 28, 2024
@laurensvalk
Copy link
Member Author

See also #1449

@laurensvalk
Copy link
Member Author

Tagging for feedback: @ggramlich, @afarago.

@laurensvalk laurensvalk added documentation Improvements or additions to documentation software: pybricks-micropython Issues with Pybricks MicroPython firmware (or EV3 runtime) labels Apr 28, 2024
@afarago
Copy link

afarago commented Apr 28, 2024

Sounds good and awesome.
My dream drivebase is global-orientation aware, therefore I would still be happy to see an optional parameter to use for the heading initial state.

robot.reset(angle: Number = 0)

This allows an FLL team to use reset between two missions still keeping the orientation global (N-W-S-E) no matter how they position the robot. FLL and WRO mats have the orientation N facing top and S on the bottom and so forth...
In some runs the bot starts facing north, while in others it faces west or east or even south.

This if the team starts the bot facing leftwards it would call robot.reset(-90) instead of the default robot.reset() thus robot.angle() still yielding -90 when facing to the west.

@ggramlich
Copy link

For Python, the method call as described by @afarago sounds good to me. This would also mean that there is no way to only reset the distance.

As my teams are using Block Coding, the name of the block "reset distance and angle" and the example given in https://www.patreon.com/posts/pybricks-3-5-is-102449785 is a bit misleading when you enable useGyro, if you don't know, that the actual reset of the heading is done by the "drive base stop coast" block.
If you used "drive base stop HOLD" there, you get the behavior demonstrated in this video:

https://photos.app.goo.gl/jdEdbWDaD6GCa9B99

So yes, my expectation is also that "reset distance and angle" would reset the heading-angle.

@afarago
Copy link

afarago commented Apr 28, 2024

Agree 100% - elementary age group will not handle global heading.

For block coding it would be so nice to have "expandable" blocks - gradually adding more (and more) params to set.

@laurensvalk
Copy link
Member Author

laurensvalk commented Sep 19, 2024

I think the course of action here is:

Change .reset() to .reset(angle=0, distance=0)
Both angle and distance can be worth resetting. As suggested by @afarago.

Decide what to do about maintaining hold mode
We can either include a .stop(). Or, maintain the newly reset angle.

Both should avoid the sitation in @ggramlich's video. Calling .stop() would be simpler but not entirely technically correct.

Augment and document the block
This is currently missing from the docs. Potentially add the above options for angle and distance.

Decide relation with hub.imu.heading()
This isn't decided yet. Should it also be reset? It is technically independent... But in practice, it is probably used synonymously with the drivebase angle. There is also the other way around. What should the drivebase do if you call hub.imu.reset_heading(0)? This came up in #1818

EDIT: We should probably make them always the same. This is the easiest to explain. Even if it isn't the "right" approach from how how the firmware is structured, we can still do it.

@laurensvalk
Copy link
Member Author

For these to be proper replacements for imu.heading(), these probably need to become floating point values too.

@laurensvalk
Copy link
Member Author

The fix is available for testing using these instructions to try the latest version

Please re-open if you experience any further issues or inconsistencies. Thank you!

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

No branches or pull requests

3 participants