Add RPM filter configuration

[McGiverGim]

To go with betaflight/betaflight#8817

Adds configuration for the:

• Bidirectional DShot
• Motor poles
• RPM Filter (harmonics and min hz)

Don't merge by the moment, the save action in the PIDs tab is giving some strange MSP error that I'm not sure if it is culprit of this code or is another kind of bug.

Fixes #1631.

[McGiverGim]

It seems it happens when we "deactivate" the RPM filtering: gyro_rpm_notch_harmonics = 0 something is wrong in the firmware because the FC stops to answer the MSP commands (timeout).
I can enable the filter when disabled without problem, but if a disable it when enabled the FC hangs.
Maybe something is wrong in the RPM Filter code or must I try to force a reboot (I'm not too sure if it is possible, the MSP seems not to accept any command)?

[McGiverGim]

The problem is fixed with betaflight/betaflight#8826

Pushed some cleaning on the code and is ready to be reviewed and merged.

[bizmar]

Can we please refer to motor poles as "motor bell magnets", or anything that is clear that they have to count the number of magnets, not the number of stators. Since 4.0 I've had to clarify this at least 20 times.

Notch Q and number of harmonics are adjusted frequently.
Min freq. is adjusted very rarely.

[McGiverGim]

It is explained in the tooltip, but I can change the tag too, it don't hurts. About the adjustment of Q, I can't say. If it is usually changed, it must be added to the MSP first. @joelucid what do you think? Is a value that must be changed by the standard user or is better to let it CLI only?

[joelucid]

I suggest we add the explanation "requires supposed ESC firmware" to the label of "bidirectional dshot". This makes it clear the user has to do something to make it work and will encourage to look at the tooltip where we have info on what to do:

[McGiverGim]

@mikeller what do you think about adding the extra text to the label like @joelucid suggests? We usually don't do it in this way, we have the explanation in the tooltip, but maybe is clear for the user and will led to less support requests.

[bizmar]

It is explained in the tooltip, but I can change the tag too, it don't hurt

Please change the label too. It's explained pretty clearly in the wiki, yet people somehow don't read past "motor poles". Getting the poles wrong "silently" makes the filter completely ineffective. So it's really important to get them right.

For the tooltip: (feel free to re-word)
Motors from 3" to brushless "whoop", usually have 12 poles.

RPM notch Q is not a "must" to change.. like all other RPM filter parameters, it can be changed if you want to tune it, for your quad. From my experience, setting it up numerous times since January. Q is the only value a "casual" tuner should change. (slowly up the number, look at the gyro noise plot of a flight log. Increase until the motor noise band starts "bleeding through", and back down a little.) Other values have more complex interactions and require more in depth knowledge.

Tooltip suggestion:
Number of harmonics per motor. A value of 3 (recommended for most quads) will generate 3 notch filters, per motor for each axis, totaling 36 notches. One at the base motor frequency and two "harmonics" at multiples of that base frequency.

[McGiverGim]

Thanks for the text, my English level sometimes don't let me elaborate the phrases :P

I will change them, I'm only waiting for the comments of the bidirectional dshot, to make all changes at one.

[mikeller]

@McGiverGim: I think in this case it makes sense to add the extra text (n.b. it's 'supported', not 'supposed').

But we absolutely should not start inventing new terms for things that are already well defined, and 'motor pole count' or 'magnetic motor pole count' is well defined and translators can translate them into the correct version in their language, and users can look it up - if we start inventing new terms, and then get them translated, users will be left with a meaningless jumble of words that will give them random results when they google them.

[McGiverGim]

Ok, pushed changes to the texts as per reviews.

[spatzengr]

Problem solved?:

A casual user may not realize the stator is not magnets, so do think we need to emphasize magnets on bell (bold maybe?).

[mikeller]

@spatzengr: Yes, I think that makes sense.

@McGiverGim: What do you think about adding the text as @spatzengr has proposed?

[McGiverGim]

This text is on the tooltip, but if you think is valuable add it to the label I can do it. We must go with caution to not show too much text in the tabs, it is less cleaner to the user and can made him not to read.

[bizmar]

eRPM needs to be converted into RPM correctly otherwise ALL calculations down the line will be wrong, and all RPM features will report as working, but will produce wrong results.

For somebody not familiar with the whole system, it may be hard figure out the reason why the motor noise band is still visible when RPM filter is enabled. They may conclude RPM filter is not working for them, or that they may set a wider notch Q (which may solve the problem in the wrong way)

I cant state strongly enough how important it is that "motor poles" are set correctly by the user. The only thing we can do about it, is to ensure we communicate that as successfully as we can.

[bizmar]

Here's the progression of the relevant part of the wiki article about Motor Poles.

Motor Poles

The escs report erpm which needs to be converted to rpm using the number of poles (magnets) of the motors. Regular 5" motors have 14 poles and that's the default setting. Smaller motors have less poles, often 12. Count them or look up the motor specs and configure using:

set motor_poles=14

Motor Poles

The ESCs report eRPM, which needs to be converted to RPM using the number of poles (magnets) of the motors. These are found on the bell of the motor, not the stator magnets where the windings are located. Typical 5" motors have 14 poles, so that is the default setting. Smaller motors have fewer poles, often 12. Count them or look up the motor specs and configure using:

set motor_poles=14

Motor Magnets

The ESCs report eRPM, which needs to be converted to RPM using the number of magnets of the motors. These are found on the bell of the motor, not the stator magnets where the windings are located. Typical 5" motors have 14 magnets, so that is the default setting. Smaller motors have fewer magnets, often 12. Count them or look up the motor specs and configure using the following command in the CLI if you don't have 14 magnets:

set motor_poles=xx where xx is the number of magnets you counted.

I'm sorry to keep pushing this, it would be insane to mek to make this mistake again, when we have just learned it.

[mikeller]

@bizmar: Please stop advocating for the invention of fantasy terminology.

[ctzsnooze]

Motor poles has always confused the end user. They confused me. They confused LOTs of people on Facebook. Google searches were often unhelpful. The wiki got more and more blunt about the need to count the magnets in the bell, precisely to avoid the problems with 'pole' count.

If it was up to me I would, with the benefit of hindsight, have used, in the CLI:

set motor_bell_magnet_count = 14

That's good, simple, clear end-user terminology, and in the beginning the CLI was all we had :-)

In the GUI I'd use the plain English and put the technical term in brackets.

14 Number of magnets on bell (motor pole count).

[Technology-Wizard]

Problem solved?:

A casual user may not realize the stator is not magnets, so do think we need to emphasize magnets on bell (bold maybe?).

Again with the logic man!!

Please show your wife!!

[Technology-Wizard]

I just rebased this onto 10.6.0 on my local and it does not pull the Gyro RPM filter data to populate the Fields..

Test board is all setup for BBDshot already and the Configuration tab pulls fields correctly.

If I set the fields and turn it on from toggle it saves but will not populate the fields.

I am sure I merged the pid_tuning.js correctly am I missing something?

[McGiverGim]

Give me some minutes, I'm rebasing it now and changing some texts...

[Technology-Wizard]

Give me some minutes, I'm rebasing it now and changing some texts...

No rush and no worries at all; as long as I didnt do anything wrong :)

[McGiverGim]

Rebased and changed some texts, ready to be reviewed/merged.

@quadwizard are you on latest master of the firmware? 4.1.0 RC1 had a bug in the RPM Filter that produced this behaviour when disabled the switch.

[Technology-Wizard]

I see I was on RC1, sorry

100% working now; great work.. I went off the development line to test RC1 to try to emulate a normal user... that didnt work :(

I note that gyro_rpm_notch_q is not included; would this be added?