RPM sensing from GPIO (PWM pin) pulses #24041
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
MaEtUgR
changed the title
|
We need to get better with rejecting outliers: |
@sfuhrer |
MaEtUgR
changed the title
🔎 FLASH Analysispx4_fmu-v5x [Total VM Diff: 148 byte (0.01 %)]px4_fmu-v6x [Total VM Diff: 40 byte (0 %)]Updated: 2024-12-12T14:45:05 |
|
I rebased on main, fixed the screw up with the unwanted, unused
Here's the plot from the small RC heli spinning the motor without blades open loop: |
oravla5
reviewed
Dec 11, 2024
| /** | ||
| * RPM Capture Enable | ||
| * | ||
| * Enables the RPM capture module on FMU channel 5. |
There was a problem hiding this comment.
Before merging, shall we check if we could use other channels different from 5? If so, we should update the comment accordingly.
There was a problem hiding this comment.
Yes definitely. I always used 5 so far because of that comment...
There was a problem hiding this comment.
I quickly asked the ST experts and the interrupt should be independent of timers and configuring the pin for input interrupt should be possible on most pins but probably depends on the board. So the assumption is it should work also on the other FMU PWM pins.
There was a problem hiding this comment.
@bkueng are you aware of other boards in which only channel 5 would work? Otherwise, would you be ok with removing the channel 5 part of the description?
There was a problem hiding this comment.
Channel 5 has history reasons. This is now more flexible as any pin can be configured, so the comment does not apply anymore.
oravla5
reviewed
Dec 11, 2024
|
I confrimed that RPM capture works on other AUX GPIO pins. It's not available in the MAIN pins. I also played around with the parameters of the alpha and medium filters:
Find attached the logs for:
|
bresch
requested changes
Dec 12, 2024
Such that we can continue development on this part. The implementation was already used before porting things into the RPM capture module.
Not timing out based on a random interval but based on the time after the last inerrupt.
Signed-off-by: Silvan Fuhrer <silvan@auterion.com>
…pe -> use module params
…utput channels and rpm filter time constant
…to work with all these drivers
|
@bresch Thanks for the thorough review 👍 I rebased on main, addressed all your comments in new commits (not one each but they're referenced) and added 375cd55 on top because I saw that was only necessary before 15fece7#diff-21b8564105b699e09127da22888e7b35b845e53be24c30a6f921ea1f9e82f821L84. |
bresch
approved these changes
Dec 16, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Solved Problem
When wokring on helicopter rpm control I found that RPM sensing is "easily" possible but requires work. @bkueng provided the nice configurable capture part for that effort and I used the output directly in the heli-specific controller. But now I think we should enable generic RPM capturing for use in control, log, UI reporting.
Solution
As a draft, I added the hardcoded logic we used to process the RPM.Note: This doesn't work well out of the box because it cannot detect a timeout if the interrupt is never called anymore.
The solution in the meantime has:
Thanks for all the help from @oravla5, @sfuhrer and @bresch behind the scenes.
Changelog Entry
Test coverage
This logic was used on a heli to control RPM before porting it to latest main and moving the processing to the rpm capture driver. EDIT: and changing it completely again.
We need to follow up with further testing, settling the interface, implementing the timeout... FYI @sfuhrerWe did testing on an RC helicopter with high pulse per revolution count + noisy signal and a combustion engine with outliers.