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

Fix Bug causing Dynamic Idle to fail if RPM filtering is turned off #12270

Conversation

ctzsnooze
Copy link
Member

@ctzsnooze ctzsnooze commented Jan 31, 2023

This is a Bug Fix for an issue where Dynamic Idle went to maximum Idle boost, when activated, if RPM Filtering was switched off, even though DShot Telemetry was enabled.

Many thanks to Eddie Trejo for finding the bug and reporting it!

I think the bug originated here in PR #11765, where the initialisation of the various RPM Filter init values exited before calculating the erpmToHz value. erpmToHz is required to calculate and return minMotorFrequencyHz to Dynamic Idle via getMinMotorFrequency.

This PR just moves the calculation erpmToHz up, in rpmFilterInit, so that it is calculated before the return when RPM Filtering is not enabled.

This appears to solve the bug.

Fix for no RPM values sent to Dynamic Idle when RPM filtering was turned off
@ctzsnooze ctzsnooze changed the title Init erpmToHz even if RPM filtering is disabled Fix Bug causing Dynamic Idle to fail if RPM filtering is turned off Jan 31, 2023
@github-actions
Copy link

Do you want to test this code? Here you have an automated build:
Assets
WARNING: It may be unstable. Use only for testing! See: https://www.youtube.com/watch?v=I1uN9CN30gw for instructions for unified targets!

@ctzsnooze ctzsnooze self-assigned this Jan 31, 2023
@ctzsnooze ctzsnooze added this to the 4.4 milestone Jan 31, 2023
@blckmn
Copy link
Member

blckmn commented Jan 31, 2023

We should probably also put this to the 4.4-maintenance branch

@blckmn
Copy link
Member

blckmn commented Jan 31, 2023

AUTOMERGE: (FAIL)

  • github identifies PR as mergeable -> FAIL
  • assigned to a milestone -> PASS
  • cooling off period lapsed -> FAIL
  • commit count less or equal to three -> PASS
  • Don't merge label NOT found -> PASS
  • at least one RN: label found -> PASS
  • Tested label found -> FAIL
  • assigned to an approver -> PASS
  • approver count at least three -> FAIL

@klutvott123
Copy link
Member

I'm thinking the processing of rpm data should probably be done elsewhere since it's used not only by the rpm filters. We have had issues like this before, and I'm sure we'll have them again, but for now I think this is good as a bugfix 🙂

@KarateBrot
Copy link
Member

I'm thinking the processing of rpm data should probably be done elsewhere since it's used not only by the rpm filters. We have had issues like this before, and I'm sure we'll have them again, but for now I think this is good as a bugfix 🙂

That's on my TODO list for 4.5 👍

@KarateBrot KarateBrot merged commit 395e8e8 into betaflight:master Jan 31, 2023
@ctzsnooze ctzsnooze deleted the Fix-DShot-Telemetry-when-RPM-Filtering-is-Off branch March 11, 2023 08:54
davidbitton pushed a commit to davidbitton/betaflight that referenced this pull request Feb 5, 2024
…etaflight#12270)

Init erpmToHz even if RPM filtering is disabled

Fix for no RPM values sent to Dynamic Idle when RPM filtering was turned off
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: COMPLETED
Development

Successfully merging this pull request may close these issues.

None yet

4 participants