Skip to content

change getVelocity() to remember velocity #195

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.

Sign up for GitHub

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

Jump to bottom
Merged
runger1101001 merged 1 commit into from
Jun 23, 2022
Merged

change getVelocity() to remember velocity #195

runger1101001 merged 1 commit into from
Jun 23, 2022

Conversation

@runger1101001
Copy link
Member

@runger1101001 runger1101001 commented Jun 19, 2022

Change in the way getVelocity() works.
It now "remembers" the last value calculated, and re-calculates it only when the delta-T is large enough (controllable through a new member field, default value is 100µs
float minDeltaT = 0.000100; // default is 100 microseconds, or 10kHz

This will, I think, more elegantly handle the edge cases:

  • getVelocity() called too often
  • microseconds wrap

It does however introduce a kind of low-pass filter based on minDeltaT - one that was implicitly present in some form anyway, but is now explicit.

Please take a look and see if you agree with this change @askuric - we discussed it, but this might not be the way you imagined it?

@runger1101001 runger1101001 requested a review from askuric June 19, 2022 19:03
@runger1101001
Copy link
Member Author

runger1101001 commented Jun 19, 2022

Note: tested with gimbal motors on STM32G4 and SAMD21

@runger1101001
Copy link
Member Author

runger1101001 commented Jun 19, 2022

Oh, and also important to mention: the change also implicitly, through its deltaT check, computes the velocity at most once per call to update() - since the timestamp is only updated during update().
So getVelocity() now has the same behaviour as getAngle(), where the value remains stable within one loop iteration, i.e. requiring a call to update() before the value changes.

@askuric
Copy link
Member

askuric commented Jun 20, 2022 •
edited
Loading

I love it, its so simple and effective. That is more or less the solution I had in mind also. The only thing is that we might define this minDeltaT differently for different architectures. Like teensy/esp32/stm32g4/stm32f4 maybe go to 10us and the rest to 100us.
But exposing this variable is probably the best way to resolve this, leave it to the user to find the best value.

Im in for merging this :D

@runger1101001 runger1101001 merged commit ab8d239 into simplefoc:dev Jun 23, 2022
@runger1101001
Copy link
Member Author

runger1101001 commented Jun 23, 2022

Ok then lets do it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@askuric askuric Awaiting requested review from askuric

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@runger1101001 @askuric