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 request: Support negative RPM for dynamic harmonic notch #28303

Closed
RickReeser opened this issue Oct 4, 2024 · 5 comments · Fixed by #28309
Closed

Feature request: Support negative RPM for dynamic harmonic notch #28303

RickReeser opened this issue Oct 4, 2024 · 5 comments · Fixed by #28309

Comments

@RickReeser
Copy link
Contributor

RickReeser commented Oct 4, 2024

Copter 4.5.6

Some specifications allow ESC telemetry to send negative RPM if the motor is spinning backwards (or just clockwise vs. counter-clockwise, as in my case), e.g. DroneCAN esc status message. RPM-tracking notches do not use negative RPM and they will just sit at the minimum frequency:
image

Suggestion:
I suppose the naive solution of using the absolute value of RPM would suffice?

Platform
[ ] All
[ ] AntennaTracker
[ x ] Copter
[ x ] Plane
[ ] Rover
[ ] Submarine

@andyp1per
Copy link
Collaborator

Certainly some judicious fabsf() would probably fix this and would be easy to do.

@andyp1per andyp1per linked a pull request Oct 4, 2024 that will close this issue
@andyp1per
Copy link
Collaborator

Please can you try

@RickReeser
Copy link
Contributor Author

Yeah, I will look into it today. I don't have any familiarity with the filtering classes, but I suppose this will be a good introduction.

@andyp1per
Copy link
Collaborator

Yeah, I will look into it today. I don't have any familiarity with the filtering classes, but I suppose this will be a good introduction.

Did you see the PR I did?

@RickReeser
Copy link
Contributor Author

Nope, just saw it. Will try your PR now.

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

Successfully merging a pull request may close this issue.

2 participants