PID TPA Pitch Angle Related Refactor

[breadoven]

Refactors code for PID Throttle PID attenuation (TPA). General code clean up with notable changes as follows:

1. Function calculateTPAThtrottle renamed to tpaPitchThrottleAdjustment to better describe its purpose which has now changed so it only provides a pitch based TPA throttle adjustment/compensation value rather than a direct throttle output value. tpaPitchThrottleAdjustment simplified to remove redundant code. Comment added to better describe its purpose.
2. Pitch angle TPA throttle compensation now specifically only applicable to fixed wing (it was never used for multirotor).
3. tpa_rate for multirotor constrained 0 to 100 to prevent -ve TPA factors for settings > 100.
4. Fixed wing step change in tpaFactor at idle removed for tpa_rate < 100. There's no reason for a step change just because the throttle reduces from 1us above idle to idle. Seems to be a hangover from code going back some time that was never corrected after recent changes.

The settings.yaml description for tpa_pitch_compensation needs improving but hasn't been changed for this PR in light of #11904 and #11956.

HITL testing on fixed wing and multirotor testing shows the TPA factor values behave as expected.

[qodo-code-review]

ⓘ Qodo reviews are paused because the subscription is no longer active. Ask your workspace admin to reactivate the subscription to resume reviews. Manage billing

[qodo-free-for-open-source-projects]

PR Summary by Qodo

Refactor pitch-aware throttle PID attenuation

🐞 Bug fix ✨ Enhancement 📝 Documentation 🕐 20-40 Minutes

AI Description

• Restricts pitch-based throttle compensation to fixed-wing TPA and simplifies its calculation.
• Clamps multirotor TPA rates to prevent negative attenuation factors above 100%.
• Renames the TPA breakpoint field consistently and clarifies user-facing documentation.

Diagram

graph TD
    C["Control Profile"] --> U["PID Update"] --> M{"Aircraft Type"}
    M -->|Fixed wing| F["Pitch Adjustment"] --> W["Fixed-wing TPA"]
    M -->|Multirotor| R["Rate Clamp"] --> X["Multirotor TPA"]
    W --> G["Scaled PID Gains"]
    X --> G

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Share TPA calculations with transition handling

• ➕ Reduces duplicated fixed-wing and multirotor attenuation formulas.
• ➕ Keeps normal-flight and auto-transition bounds behavior synchronized.
• ➖ Requires parameterizing control profiles and separate filter state.
• ➖ Broadens the change into higher-risk transition-control behavior.

Recommendation: The PR's scoped refactor is preferable because it fixes normal PID TPA behavior without changing auto-transition filter ownership or MSP wire formats. A follow-up could consolidate shared formulas after adding focused tests for normal and transition TPA boundaries.

Files changed (11) +65 / -59

Bug fix (1) +46 / -40

pid.cSeparate fixed-wing pitch compensation from TPA throttle +46/-40

Separate fixed-wing pitch compensation from TPA throttle

• Replaces the general throttle-producing helper with a filtered fixed-wing pitch adjustment and applies it only during fixed-wing throttle-based TPA. It also clamps the normal multirotor TPA rate to 100, simplifies factor calculations, and adopts the renamed breakpoint field.

src/main/flight/pid.c

Refactor (7) +15 / -15

blackbox.cUse renamed TPA breakpoint field in blackbox headers +1/-1

Use renamed TPA breakpoint field in blackbox headers

• Updates blackbox system information output to read 'throttle.tpa_breakpoint'. The emitted 'tpa_breakpoint' header remains unchanged.

src/main/blackbox/blackbox.c

control_profile.cInitialize the renamed TPA breakpoint setting +1/-1

Initialize the renamed TPA breakpoint setting

• Updates control-profile defaults to initialize 'throttle.tpa_breakpoint' without changing its configured default value.

src/main/fc/control_profile.c

control_profile_config_struct.hRename the throttle breakpoint configuration field +3/-3

Rename the throttle breakpoint configuration field

• Renames 'pa_breakpoint' to 'tpa_breakpoint' for clearer domain terminology and aligns adjacent field comments.

src/main/fc/control_profile_config_struct.h

fc_msp.cPropagate TPA breakpoint naming through MSP handling +6/-6

Propagate TPA breakpoint naming through MSP handling

• Renames internal MSP packet members and control-profile accesses for the TPA breakpoint. Packet order, field widths, and packed structure sizes remain unchanged.

src/main/fc/fc_msp.c

rc_adjustments.cTarget renamed breakpoint during in-flight adjustment +1/-1

Target renamed breakpoint during in-flight adjustment

• Routes TPA breakpoint step adjustments to 'throttle.tpa_breakpoint' while retaining existing PWM limits.

src/main/fc/rc_adjustments.c

osd.cDisplay the renamed TPA breakpoint field +1/-1

Display the renamed TPA breakpoint field

• Updates the standard OSD breakpoint element to read the renamed control-profile member without changing display behavior.

src/main/io/osd.c

osd_dji_hd.cUse renamed breakpoint in DJI OSD and MSP output +2/-2

Use renamed breakpoint in DJI OSD and MSP output

• Updates DJI adjustment messages and MSP tuning responses to access 'throttle.tpa_breakpoint'. User-facing labels and serialized values remain unchanged.

src/main/io/osd_dji_hd.c

Documentation (1) +1 / -1

Settings.mdClarify generated TPA rate documentation +1/-1

Clarify generated TPA rate documentation

• Rewords the TPA rate description to distinguish multirotor PDFF scaling from fixed-wing PIDFF scaling and improve breakpoint behavior wording.

docs/Settings.md

Other (2) +3 / -3

settings.yamlRemap and clarify TPA configuration settings +2/-2

Remap and clarify TPA configuration settings

• Maps 'tpa_breakpoint' to the renamed control-profile field and revises the TPA rate description. Existing defaults and allowed ranges remain unchanged.

src/main/fc/settings.yaml

CMakeLists.txtNormalize target configuration line formatting +1/-1

Normalize target configuration line formatting

• Removes trailing whitespace from the SYNERDUINOH7 target declaration with no functional build change.

src/main/target/SYNERDUINOH7/CMakeLists.txt

[qodo-free-for-open-source-projects]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. Fixed-wing gains are falsely attenuated ✓ Resolved 🐞 Bug ≡ Correctness

Description

pidResetTPAFilter still seeds fixedWingTpaFilter with the idle throttle value, while
tpaPitchThrottleAdjustment now filters a zero-centered pitch adjustment and adds that result to
the current throttle. With pitch compensation and throttle-based TPA enabled, initialization and
repeated launch resets therefore begin near a +1000 adjustment, making the effective throttle
approach its maximum and reducing fixed-wing gains until the default two-second filter converges.

Code

src/main/flight/pid.c[1312]

+        tpaThrottleAdjustment = pt1FilterApply(&fixedWingTpaFilter, tpaThrottleAdjustment);

Evidence

The filter reset seeds state with getThrottleIdleValue(), whereas the changed function supplies an
adjustment centered on zero and then adds the filtered result to throttle. The PT1 implementation
begins from the prior state, launch handling repeatedly invokes this reset, and the configured
default time constant is 2000 ms, proving the erroneous positive adjustment persists rather than
disappearing immediately.

src/main/flight/pid.c[467-472]
src/main/common/filter.c[61-64]
src/main/flight/pid.c[1300-1331]
src/main/navigation/navigation_fw_launch.c[318-322]
src/main/fc/settings.yaml[1482-1488]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The fixed-wing TPA filter now processes a zero-centered pitch adjustment, but its lifecycle still initializes it with an absolute idle-throttle value and can preserve stale state while TPA is inactive.
## Fix Focus Areas
- src/main/flight/pid.c[467-472]
- src/main/flight/pid.c[1300-1331]
- src/main/navigation/navigation_fw_launch.c[318-322]
## Recommended Fix
Reset `fixedWingTpaFilter` to zero now that its input is an adjustment rather than absolute throttle. Ensure its state is also updated or reset while pitch TPA is inactive so reactivation cannot reuse an adjustment from an earlier attitude.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Protocol docs point to a removed field ✓ Resolved 🐞 Bug ⚙ Maintainability

Description

The profile member is renamed to throttle.tpa_breakpoint, but the protocol message definitions and
generated reference still describe it as throttle.pa_breakpoint. Developers following those
references encounter a field that no longer exists across several tuning request and response
definitions.

Code

src/main/fc/control_profile_config_struct.h[33]

+        uint16_t tpa_breakpoint;              // Breakpoint where TPA is activated

Evidence

The changed profile structure defines only tpa_breakpoint, and firmware serialization already uses
that member, while multiple protocol definitions still explicitly reference pa_breakpoint. This
makes the documented source mapping inconsistent with the actual profile API introduced by the PR.

src/main/fc/control_profile_config_struct.h[30-36]
src/main/fc/fc_msp.c[811-815]
docs/development/msp/msp_messages.json[4022-4027]
docs/development/msp/msp_messages.json[5127-5132]
docs/development/msp/README.md[1994-2000]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The protocol documentation retains references to the removed `throttle.pa_breakpoint` member after the profile field was renamed to `throttle.tpa_breakpoint`.
## Fix Focus Areas
- docs/development/msp/msp_messages.json[4022-4027]
- docs/development/msp/msp_messages.json[5127-5132]
- docs/development/msp/msp_messages.json[7447-7452]
- docs/development/msp/msp_messages.json[7541-7546]
- docs/development/msp/README.md[1994-2000]
- docs/development/msp/README.md[2421-2427]
- docs/development/msp/README.md[3315-3321]
- docs/development/msp/README.md[3336-3342]
## Recommended Fix
Replace every protocol-documentation reference to `throttle.pa_breakpoint` with `throttle.tpa_breakpoint`, then regenerate the rendered protocol README from the corrected message definitions.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

Tip of the day

💡 Did you know, you can turn on the rule miner and Qodo learns your standards from review history

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

[sensei-hacker]

163d7cc should resolve the issue Qodo pointed out, I think?

I think the merge resolution is just deleting calculateTPAThtrottle?

[breadoven]

@Jetrell Does it make sense to allow multirotor tpaFactor to go to zero at max throttle with tpa_rate = 100 given P, D and CD/FF also go to 0 ?

[Jetrell]

@breadoven When I read it the other day. I was actually left wondering why you did that. And you have tested it in HITL ?

I only had a quick skim through on the phone. I might have missed something.
But looking at the multirotor calculations, What if the user sets tpa_rate to MC max limit of 100.
At full throttle (throttle == getMaxThrottle()), becomes 1.0. With dynPID = 100, Doesn't that leave a tpaFactor of 0.0.

However I assume the I-term remains active to preserve attitude control . Even when the P, D, and Feedforward terms is scale down to 0.0 ? I think it could be more robust.

It would seem the path you have taken is to make it work with one setting.
I and the other guys talked about this in another PR recently.
In this case, we have -
0 - 100 for MC TPA_rate
0 - 200 for FW TPA_rate.
This use of the same variable for both platforms. Has even been a problem raised many times on Discord.
Because TPA_rate is set to zero by default, for MC safety. Which means the user has to load the configurator fixedwing platform default. Or FW TPA_rate remains at zero. Leading to control oscillation straight after launch, if the gains are even a little too high.

There are so many new AI driven commits being added with many of the settings they bring along not being completely needed.. But here we have a crucial flight setting, that would be better separated for multiple reasons, and not the least safety in setup... That's just my view anyway.

[breadoven]

It would seem the path you have taken is to make it work with one setting. I and the other guys talked about this in another PR recently. In this case, we have - 0 - 100 for MC TPA_rate 0 - 200 for FW TPA_rate. This use of the same variable for both platforms. Has even been a problem raised many times on Discord. Because TPA_rate is set to zero by default, for MC safety. Which means the user has to load the configurator fixedwing platform default. Or FW TPA_rate remains at zero. Leading to control oscillation straight after launch, if the gains are even a little too high.

I didn't change the existing settings or the basic TPA calculation method just the way it was coded to make it clearer (It always worked with only 1 setting for TPA_rate). The only exception to this is to do with multirotor where the TPA_rate setting is now constrained by this PR to 0 -> 100 always. The existing code has no constraint until max throttle then it suddenly gets constrained to a max of 100. This makes no sense because you could set TPA_rate to 200 and end up with tpaFactor = -1 being applied right before max throttle then have it suddenly flip to 0 at max throttle. It's a bug because I'd have thought -1 would do bad things. Probably never been an issue because nobody ever set TPA_rate to 200 for MR.

Fixed wing is constrained to a minimum tpaFactor of 0.3 so it seemed a bit odd that multirotor can go down to 0 with a TPA_rate of 100. This seems to be considered OK based on the existing code but then the existing code is buggy so is this the actual intention ? If it's not an issue then it can be left constrained to a max of 100 but the setting description needs clarifying that this is the limit for multirotor. Or you could add yet another setting I guess to split out fixed wing and multirotor but this isn't preferable unless it's causing real problems.

[Jetrell]

Your explanation makes sense. I must have missed one of those points last night..

Or you could add yet another setting I guess to split out fixed wing and multirotor but this isn't preferable unless it's causing real problems.

Well it actually has been causing problems as mentioned in my last post.
Users are often not loading the fixedwing platform default, when they migrate from one version to another. Like in the case of 8.1 to 9.0. Which TPA + PA first appeared in.
This meant fixedwing TPA_rate is left at zero.. I consider that a real issue... Many users have asked why they can't get rid of control surface oscillations.
After inspection of their Diff and Dump, that was the reason.
It's much safer and user friendly to have two setting broken out with their own typical defaults. e.g. mc_tpa_rate = 20 as it used to be. And fw_tpa_rate = 80 or even 100 as I have found.
Being this is an important setting related to flight.

[sensei-hacker]

Users are often not loading the fixedwing platform default, when they migrate from one version to another. Like in the case of 8.1 to 9.0. Which TPA + PA first appeared in.

That is a problem. And not just for this one setting, right? Platform defaults not happening on upgrade to the version where the setting is introduced seems like a wider issue. This isn't the only setting with platform defaults, eh?

I know @b14ckyy did some good work on the upgrade path for settings. Perhaps we can find a way to fix the broader problem, rather than just a bandaid for this one particular setting?

A thought is when platform defaults are important, we could do any of the following:

Configurator could check for a "never set" sentinel value and set the platform default. "Has never been set" would be the firmware default.

Alternatively:

1. Have the firmware default be invalid, and Configurator would flag it immediately upon connecting
   Or
2. Have the firmware handle setting the platform defaults

[Jetrell]

And not just for this one setting, right?

There are a couple. But its more a case of having certain functions enabled or disabled by default, for the benefit of the user.
Not so much like this. When not having it set on a FW platform will lead to control surface oscillations. Or having it set too high on a MC platform, will weaken the gains at high speed.. In either case it could lead to a crash, or at best, poor performance..
I can't think of any settings that are coupled, with the same negative effect this one can have upon flight performance if it's set incorrectly.
Most settings are not inter-platform related. They generally have a different setting, or setting name, to handle the difference in operation between a MC and FW platform.

Below is a sample of those.. A few could have been coupled in the early days. But now that we have mixer profiles for VTOL operation, it is imperative those settings have different identities. Because there tuning requirements are different in each mixer profile.

MC	FW
nav_mc_bank_angle	nav_fw_bank_angle
nav_mc_auto_climb_rate	nav_fw_auto_climb_rate
nav_mc_manual_climb_rate	nav_fw_manual_climb_rate
mc_p_pitch	fw_p_pitch
mc_i_pitch	fw_i_pitch
mc_d_pitch	fw_d_pitch
mc_p_yaw	fw_p_yaw
mc_i_yaw	fw_i_yaw
mc_d_yaw	fw_d_yaw
mc_p_level	fw_p_level
mc_i_level	fw_i_level
}mc_d_level	fw_d_level
nav_mc_pos_z_p	nav_fw_pos_z_p
nav_mc_pos_xy_p	nav_fw_pos_xy_p

Besides the above. There are more than double that amount of settings that are specific to each platform. And don't have a function to perform on the other... Like this random example - nav_fw_launch_timeout and nav_mc_braking_disengage_speed.

But getting back to this. The fact tpa_rate has two different setting ranges. And is now applied differently with pitch effecting FW TPA.. Its also requiring the code handling to be more 'creative'. I seen this back when Shota wrote it last year, and thought there should have been two settings.

I'm not unaware that setting resources are a limited. But I feel we are allowing simple settings to get through in some cases. While we are debating here over the need for an extra far more important setting.

2. Have the firmware handle setting the platform defaults

Do you mean that certain platform specific settings will be applied when a certain mixer platform type is selected ? But I'm not sure how that will work for a VTOL.

[b14ckyy]

If we have new settings with platform specific defaults, I can extend the settings conversion tool to read the platform type before applying an upgrade and then add the appropriate settings to it. So we just define them in the upgrade profile json. The only issue might be, if there are platform specific differences within one class for example for wings and planes with tail. Or vtol setups.

Configurator derives the platform type based on the mixer config but full custom mixes could be an issue.

Alternative we could add a follow up popup if the platform type is not 100% reliably detected so the user decides?

[breadoven]

For the record it seems tpa_rate max setting was increased from 100 to 200 in #11042. This added a 100 constraint on multirotor tpa_rate at max throttle but not below which I assume was an oversight because it allows tpaFactors < 0 for multirotor which would cause problems.

[github-actions]

RAM / Flash usage vs. base commit 884efc7 — commit e051de3

Using the nearest available size baseline — the PR's exact base commit has no stored baseline yet.

Target	Flash Δ	RAM Δ
MATEKF405 ⚠️	+3436 B (+0.49%)	-11700 B (-7.83%)
MATEKF722 ⚠️	+1968 B (+0.42%)	-11744 B (-9.36%)
MATEKF765 ⚠️	+2960 B (+0.40%)	-11752 B (-7.03%)
MATEKH743 ⚠️	+2624 B (+0.34%)	-11664 B (-6.83%)

See RAM/flash optimization guide for techniques to reduce usage.

[github-actions]

Test firmware build ready — commit e051de3

Download firmware for PR #11969

249 targets built. Find your board's .hex file by name on that page (e.g. MATEKF405SE.hex). Files are individually downloadable — no GitHub login required.

Development build for testing only. Use Full Chip Erase when flashing.