Skip to content

FW autotrim: replace servo_autotrim_iterm_rate_limit setting with hardcoded constant#11617

Merged
sensei-hacker merged 2 commits into
iNavFlight:release/9.1from
sensei-hacker:fix/remove-autotrim-iterm-rate-limit-setting
Jun 4, 2026
Merged

FW autotrim: replace servo_autotrim_iterm_rate_limit setting with hardcoded constant#11617
sensei-hacker merged 2 commits into
iNavFlight:release/9.1from
sensei-hacker:fix/remove-autotrim-iterm-rate-limit-setting

Conversation

@sensei-hacker
Copy link
Copy Markdown
Member

@sensei-hacker sensei-hacker commented Jun 4, 2026

Summary

Removes the servo_autotrim_iterm_rate_limit CLI setting introduced in #11215 and replaces it with the hardcoded constant SERVO_AUTOTRIM_ITERM_RATE_LIMIT 30.

Blackbox analysis of real flight i-term data shows:

  • Median rate during stable cruise: 4–9 units/s
  • 90th percentile: ~30 units/s
  • The planeIsFlyingStraight gyro guard already rejects most unsettled post-maneuver windows, the rate check also protects against turbulence-driven i-term spikes during otherwise-quiet cruise

A value of 30 (the ~90th percentile) gives appropriate protection without exposing a tuning knob that pilots cannot meaningfully calibrate.

Changes

  • src/main/flight/servos.c: Add #define SERVO_AUTOTRIM_ITERM_RATE_LIMIT 30 grouped with the other autotrim defines; replace servoConfig()->servo_autotrim_iterm_rate_limit with the constant
  • src/main/flight/servos.h: Remove uint8_t servo_autotrim_iterm_rate_limit from servoConfig_t
  • src/main/fc/settings.yaml: Remove the servo_autotrim_iterm_rate_limit setting entry

No PG version bump required because the tunable setting was for testing only and never released. Also field was at the end of servoConfig_t, so existing EEPROM blocks may load safely anyway.

Thanks @error414 !

…dcoded constant

The configurable CLI setting added in iNavFlight#11215 was based on a default of 2
with no empirical basis. Blackbox analysis of real flight data shows i-term
rates of 4–9 units/s (median) and up to 30 units/s (90th percentile) during
settled cruise flight. A hardcoded constant of 30 gives reasonable protection
against turbulence-driven i-term spikes without exposing a tuning knob that
pilots cannot meaningfully calibrate.
@qodo-code-review
Copy link
Copy Markdown
Contributor

Review Summary by Qodo

Replace servo autotrim i-term rate limit with hardcoded constant

✨ Enhancement

Grey Divider

Walkthroughs

Description
• Replace configurable servo_autotrim_iterm_rate_limit setting with hardcoded constant
• Set constant to 30 based on blackbox analysis of real flight data
• Remove CLI setting entry from configuration system
• Simplify autotrim feature by eliminating unnecessary tuning knob
Diagram
flowchart LR
  A["Configurable Setting<br/>servo_autotrim_iterm_rate_limit"] -->|"Replace with"| B["Hardcoded Constant<br/>SERVO_AUTOTRIM_ITERM_RATE_LIMIT = 30"]
  B -->|"Based on"| C["Blackbox Analysis<br/>90th percentile = 30 units/s"]
  B -->|"Applied in"| D["processContinuousServoAutotrim()"]

Loading

Grey Divider

File Changes

1. src/main/flight/servos.c ✨ Enhancement +2/-1

Add hardcoded constant and replace config reference

• Add #define SERVO_AUTOTRIM_ITERM_RATE_LIMIT 30 with comment referencing blackbox analysis
• Replace servoConfig()->servo_autotrim_iterm_rate_limit with the hardcoded constant in autotrim
 logic
• Constant positioned with other autotrim-related defines for consistency

src/main/flight/servos.c


2. src/main/flight/servos.h ✨ Enhancement +0/-1

Remove configurable field from servo config struct

• Remove uint8_t servo_autotrim_iterm_rate_limit field from servoConfig_t structure
• No PG version bump required as field was at end of struct

src/main/flight/servos.h


3. src/main/fc/settings.yaml ⚙️ Configuration changes +0/-5

Remove autotrim i-term rate limit setting entry

• Remove servo_autotrim_iterm_rate_limit setting entry from configuration
• Eliminates CLI tuning knob that lacked empirical basis

src/main/fc/settings.yaml


Grey Divider

ⓘ You are approaching your monthly quota for Qodo. Upgrade your plan

Qodo Logo

@qodo-code-review
Copy link
Copy Markdown
Contributor

qodo-code-review Bot commented Jun 4, 2026

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0)

Grey Divider


Action required

1. Settings.md out of date ✓ Resolved 🐞 Bug ☼ Reliability
Description
The PR removes servo_autotrim_iterm_rate_limit from settings.yaml but does not regenerate
docs/Settings.md, and CI explicitly checks that Settings.md matches the generated output. This
will fail the docs.yml workflow whenever settings.yaml changes.
Code

src/main/fc/settings.yaml[L1364-1368]

-      - name: servo_autotrim_iterm_rate_limit
-        description: "Maximum I-term rate of change (units/sec) for autotrim to be applied. Prevents trim updates during maneuver transitions when I-term is changing rapidly. Only applies when using `feature FW_AUTOTRIM`."
-        default_value: 2
-        min: 0
-        max: 50
Evidence
CI’s docs workflow is triggered by changes to settings.yaml and fails if docs/Settings.md
differs from the regenerated file; docs/Settings.md still contains an entry for the removed
setting, so the diff check will fail.

.github/workflows/docs.yml[2-27]
docs/Settings.md[6055-6062]
src/main/fc/settings.yaml[1323-1365]

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

## Issue description
`docs/Settings.md` is generated from `src/main/fc/settings.yaml`, and CI fails if they are out of sync. This PR updates `settings.yaml` but leaves `docs/Settings.md` still documenting the removed setting.

## Issue Context
The GitHub workflow `docs.yml` runs `python3 src/utils/update_cli_docs.py` and diffs the result against the committed `docs/Settings.md`.

## Fix
1. Run `python3 src/utils/update_cli_docs.py`.
2. Commit the updated `docs/Settings.md`.

## Fix Focus Areas
- .github/workflows/docs.yml[2-27]
- src/utils/update_cli_docs.py[35-102]
- docs/Settings.md[6055-6062]

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



Remediation recommended

2. Old dumps block save 🐞 Bug ☼ Reliability
Description
Removing servo_autotrim_iterm_rate_limit makes older CLI dumps/config restores that still contain
set servo_autotrim_iterm_rate_limit = ... fail with Invalid name; when run under batch start
(used by dump output), any error blocks save. This breaks restore workflows for users upgrading
from builds that included this setting.
Code

src/main/flight/servos.h[174]

-    uint8_t servo_autotrim_iterm_rate_limit; // Max I-term rate of change (units/sec) to apply autotrim
Evidence
The CLI’s batch mode marks any error as a batch error and then refuses to save. Since dump
output enables batch mode, a removed setting name in an older dump will trigger an error and prevent
saving/restoring unless manually edited.

src/main/fc/cli.c[298-320]
src/main/fc/cli.c[3867-3888]
src/main/fc/cli.c[3952-4066]
src/main/fc/cli.c[4475-4483]

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

## Issue description
Older CLI dumps may include `set servo_autotrim_iterm_rate_limit = ...`. After this PR removes the setting, CLI treats it as an invalid name; in batch mode, errors prevent `save`, blocking restores.

## Issue Context
- `dump` output starts with `batch start`.
- In batch mode, any CLI error sets `commandBatchError`.
- `save` refuses to proceed if `commandBatchError` is set.

## Fix (one of)
- Add a backwards-compatibility shim in CLI: detect `servo_autotrim_iterm_rate_limit` in `cliSet()` and accept/ignore it (optionally print a deprecation warning) without flagging a batch error.
- Alternatively, keep the setting as a deprecated no-op for one release cycle (so old dumps restore cleanly), while still using the hardcoded constant in autotrim logic.

## Fix Focus Areas
- src/main/fc/cli.c[298-320]
- src/main/fc/cli.c[3867-3888]
- src/main/fc/cli.c[3952-4066]
- src/main/fc/cli.c[4475-4483]

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


Grey Divider

Qodo Logo

Comment thread src/main/fc/settings.yaml
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Jun 4, 2026

Test firmware build ready — commit 1b167ee

Download firmware for PR #11617

238 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.

@sensei-hacker sensei-hacker merged commit 45e14a9 into iNavFlight:release/9.1 Jun 4, 2026
24 checks passed
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 this pull request may close these issues.

1 participant