[sensei-hacker]

No description provided.

[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

Fix flight-control edge cases and reduce firmware footprint

🐞 Bug fix ✨ Enhancement 🧪 Tests 📝 Documentation 🕐 40+ Minutes

AI Description

• Reject duplicate SBUS assignments; preserve servo transitions and safe-home geozones.
• Fade opposing fixed-wing I-terms and add regression tests for the controller.
• Reduce firmware size with lighter string searches, sorting, math, and formatting.

Diagram

graph TD
  CLI["Serial CLI"] --> Config["Serial config"] --> SBUS["SBUS output"]
  Stick["Stick demand"] --> PID["Fixed-wing PID"] --> Servos["Servo output"]
  SafeHome["Safe-home config"] --> Zones["Runtime geozones"]

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Validate duplicate functions in all serial configurations

• ➕ Would also catch duplicate assignments loaded outside the CLI.
• ➖ Could invalidate saved configurations that currently operate using their first assigned port.

2. Retain libc and exact math implementations

• ➕ Preserves library behavior and trigonometric precision without local replacements.
• ➖ Retains the linked code that this maintenance work aims to remove from constrained firmware builds.

Recommendation: Keep the CLI-scoped SBUS check to prevent new ineffective assignments without rejecting existing saved configurations. The small local replacements suit the firmware-size goal, but reviewers should verify their boundary behavior and the precision trade-off of approximate trigonometry.

Files changed (21) +402 / -74

Enhancement (11) +55 / -49

string_light.cAdd compact substring searches +26/-0

Add compact substring searches

• Implements bounded and unbounded plain-scan substring searches for firmware call sites.

src/main/common/string_light.c

string_light.hExpose lightweight substring search functions +3/-0

Expose lightweight substring search functions

• Declares the new search functions and records the code-size reason for avoiding newlib's search machinery.

src/main/common/string_light.h

pitotmeter_dlvr_l10d.cRemove float formatting from pitot diagnostics +1/-1

Remove float formatting from pitot diagnostics

• Reports out-of-range pressure as an integer scaled to thousandths of an inch of water, avoiding floating-point log formatting.

src/main/drivers/pitotmeter/pitotmeter_dlvr_l10d.c

settings.cUse lightweight search for setting names +1/-1

Use lightweight search for setting names

• Switches setting-name substring matching to the local search implementation.

src/main/fc/settings.c

wind_estimator.cUse approximate trigonometry in wind estimation +3/-3

Use approximate trigonometry in wind estimation

• Replaces standard atan2, sine, and cosine calls with existing approximate math functions in the wind-estimation calculation.

src/main/flight/wind_estimator.c

gps_ublox.cUse bounded lightweight search for UBX capabilities +5/-4

Use bounded lightweight search for UBX capabilities

• Searches UBX capability fields for constellation and protocol markers with the new bounded substring function.

src/main/io/gps_ublox.c

osd.cUse approximate elevation math for telemetry +2/-2

Use approximate elevation math for telemetry

• Computes telemetry elevation with approximate atan2 and rounds the resulting angle with lrintf.

src/main/io/osd.c

navigation_private.hPack navigation FSM descriptors more tightly +4/-3

Pack navigation FSM descriptors more tightly

• Reorders descriptor fields to group word-sized members before smaller members and reduce padding in the constant state table.

src/main/navigation/navigation_private.h

battery.cStably sort battery profiles without qsort +10/-12

Stably sort battery profiles without qsort

• Uses insertion sort for the small profile array, preserving index order for equal voltage limits while avoiding the libc sort dependency.

src/main/sensors/battery.c

target.cRemove SITL-specific bounded search implementation +0/-22

Remove SITL-specific bounded search implementation

• Deletes the local strnstr implementation now that callers use the shared lightweight search function.

src/main/target/SITL/target.c

target.hRemove obsolete SITL search declaration +0/-1

Remove obsolete SITL search declaration

• Removes the declaration for the deleted SITL-specific strnstr function.

src/main/target/SITL/target.h

Bug fix (6) +67 / -25

cli.cReject duplicate SBUS assignments and lighten CLI formatting +12/-6

Reject duplicate SBUS assignments and lighten CLI formatting

• Checks single-port function conflicts before saving a serial command. Uses lighter substring and character searches and prints GPS status values with integer fixed-point formatting.

src/main/fc/cli.c

pid.cFade opposing fixed-wing I-term during large stick inputs +21/-2

Fade opposing fixed-wing I-term during large stick inputs

• Adds an attenuation-scaled fade for locked I-terms that oppose large roll or pitch rate targets in both fixed-wing controller paths. Leaves helping I-terms, small corrections, and yaw trim unaffected.

src/main/flight/pid.c

servos.cPreserve helper-rule speed limits across mixer switches +2/-2

Preserve helper-rule speed limits across mixer switches

• Continues scanning past mixer-switch helper rules when carrying over decaying speed-limit filter state. This prevents an early switch back from dropping remaining state and causing a servo jump.

src/main/flight/servos.c

serial.cDetect duplicate single-port serial functions +17/-0

Detect duplicate single-port serial functions

• Adds a helper that compares a proposed port's functions with other configured ports and reports duplicate SERVO_SERIAL assignments.

src/main/io/serial.c

serial.hDeclare duplicate serial-function check +1/-0

Declare duplicate serial-function check

• Exposes the serial configuration helper used by the CLI before accepting a port assignment.

src/main/io/serial.h

navigation_geozone.cEnable safe-home zones and stably order active zones +14/-15

Enable safe-home zones and stably order active zones

• Marks the appended nearest-safe-home zone enabled. Replaces qsort with an in-place stable partition that keeps enabled zones ahead of disabled zones without linking the sort routine.

src/main/navigation/navigation_geozone.c

Tests (3) +279 / -0

CMakeLists.txtRegister fixed-wing I-term fade unit test +5/-0

Register fixed-wing I-term fade unit test

• Adds the new test's math and filter dependencies and its PID access shim to the unit-test build.

src/test/unit/CMakeLists.txt

pid_fw_iterm_lock_fade_access.cExpose fixed-wing PID internals to tests +46/-0

Expose fixed-wing PID internals to tests

• Includes the production PID implementation in a test-only shim and provides helpers to set axis state, step the controller, and inspect its I-term and output.

src/test/unit/pid_fw_iterm_lock_fade_access.c

pid_fw_iterm_lock_fade_unittest.ccTest opposing I-term fade and trim preservation +228/-0

Test opposing I-term fade and trim preservation

• Exercises the real fixed-wing controller against an opposing-I-term scenario. Covers fade timing, helping trim, centred and small stick inputs, partial attenuation, and the yaw exclusion.

src/test/unit/pid_fw_iterm_lock_fade_unittest.cc

Documentation (1) +1 / -0

Serial.mdDocument the single-port SBUS output constraint +1/-0

Document the single-port SBUS output constraint

• Explains that the CLI refuses a second SERVO_SERIAL assignment and that existing duplicate configurations output only on the first listed port.

docs/Serial.md

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

Code Review by Qodo

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

1. Legacy backups lose serial settings 🔗 Cross-repo conflict ≡ Correctness

Description

cliSerial checks the entire resulting mask against the current saved configuration, so it rejects
both exported legacy duplicate SERVO_SERIAL assignments and edits to a port that already has the
function. When inav-configurator replays a backup without defaults, a rejected command leaves that
port’s function mask and baud rates unrestored; moving the function to an earlier-listed port can
also fail before a later command clears the old port.

Code

src/main/fc/cli.c[R947-950]

+    const uint32_t duplicatedFunctions = serialGetDuplicatedSinglePortFunctions(&portConfig);
+    if (duplicatedFunctions) {
+        cliPrintErrorLinef("Function %d is already assigned to another port", (int)duplicatedFunctions);
+        return;

Evidence

printSerial exports each port’s full mask in index order, including legacy duplicate
SERVO_SERIAL assignments, and configurator replays those commands individually while recording
errors and continuing. The guard compares each command with serialConfig()->portConfigs as it
exists at that moment, so it cannot account for a later line clearing the old assignment; because
portConfig starts as a copy of currentConfig, the guard also treats an existing assignment as a
conflict when only baud rates change.

inav -> inav-configurator
src/main/fc/cli.c[824-854]
src/main/fc/cli.c[947-954]
src/main/io/serial.c[316-331]
docs/Serial.md[51-51]
src/main/io/serial.c[319-331]
src/main/fc/cli.c[880-954]
External repo: iNavFlight/inav-configurator, js/backup_restore.js [187-196]
External repo: iNavFlight/inav-configurator, js/backup_restore.js [282-314]

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 `serial` CLI guard prevents some exported backups from being replayed: it rejects duplicate `SERVO_SERIAL` assignments, moves to an earlier-listed port while the old assignment still exists, and edits to a port that already has the function. Configurator continues after a rejected command, leaving that port’s mask and baud rates unrestored.
## Fix Focus Areas
- src/main/fc/cli.c[824-854]
- src/main/fc/cli.c[947-954]
- src/main/io/serial.c[319-331]
- /cross_repos/inav-configurator/js/backup_restore.js[282-314]
## Recommended Fix
Check only functions newly added by a `serial` command, using `(portConfig.functionMask & ~currentConfig->functionMask)` for the duplicate check, so edits to an existing assignment remain possible. Coordinate with inav-configurator to normalize legacy backup commands before replay: keep the SBUS bit only on the first assigned port while applying every port’s remaining mask and baud rates. Account for restores that move SBUS to an earlier-listed port before the old port is cleared; consider moving the function automatically by clearing it from the other port and printing a warning instead of rejecting the command. Verify that restoring a backup with duplicate assignments completes without a rejected `serial` command.

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

---

2. CLI error shows a raw bitmask, not the function number 🐞 Bug ⚙ Maintainability

Description

The rejection message prints duplicatedFunctions, which is the bitmask, so a user sees "Function
4194304". The serial command's +N/-N syntax takes the bit index (22), so the number shown
cannot be typed back to fix the problem.

Code

src/main/fc/cli.c[949]

+        cliPrintErrorLinef("Function %d is already assigned to another port", (int)duplicatedFunctions);

Evidence

FUNCTION_SERVO_SERIAL = (1 << 22). The CLI's +/- handling uses 1 << val, so users think in
bit indices.

src/main/io/serial.h[56-56]
src/main/fc/cli.c[886-897]

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 error message prints the function bitmask instead of the bit index or name.
## Fix Focus Areas
- src/main/fc/cli.c[949-949]
## Recommended Fix
Print the bit index, e.g. `__builtin_ctz(duplicatedFunctions)`, or a readable name such as "SERVO_SERIAL".

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

---

3. Configurator can still put SBUS output on two ports 🐞 Bug ≡ Correctness

Description

The single-port check is only called from cliSerial. The MSP serial-config handler in fc_msp.c
still writes functionMask straight from the packet, and isSerialConfigValid does not check for
this, so the Configurator Ports tab can still create the duplicate this change is meant to prevent
(#11430).

Code

src/main/io/serial.c[R319-331]

+uint32_t serialGetDuplicatedSinglePortFunctions(const serialPortConfig_t *portConfigToCheck)
+{
+    uint32_t functionMaskOfOtherPorts = 0;
+
+    for (int index = 0; index < SERIAL_PORT_COUNT; index++) {
+        const serialPortConfig_t *portConfig = &serialConfig()->portConfigs[index];
+        if (portConfig->identifier != portConfigToCheck->identifier) {
+            functionMaskOfOtherPorts |= portConfig->functionMask;
+        }
+    }
+
+    return portConfigToCheck->functionMask & functionMaskOfOtherPorts & SERIAL_SINGLE_PORT_FUNCTIONS;
+}

Evidence

grep finds serialGetDuplicatedSinglePortFunctions used only in cli.c. fc_msp.c line 3603 assigns
functionMask = sbufReadU32(src) without any check.

src/main/fc/fc_msp.c[3603-3603]
src/main/fc/cli.c[947-951]

Agent prompt

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

## Issue description
Only the CLI rejects SBUS output on a second port. MSP writes are not checked.
## Fix Focus Areas
- src/main/fc/fc_msp.c[3603-3603]
- src/main/io/serial.c[271-331]
## Recommended Fix
Either reject or strip duplicate single-port functions in the MSP serial-config handler, or add a check to `isSerialConfigValid` so all write paths are covered.

ⓘ 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 reply 'qodo' on any finding to push back, ask questions, or dig deeper

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

[github-actions]

RAM / Flash usage vs. base commit 01a7aa3 — commit c9893bb

Using the nearest available size baseline: this build contains base commit 6cbfc64, which has no stored baseline yet, so the deltas also include what was merged to the base since 01a7aa3.

Target	Flash Δ	RAM Δ
MATEKF405 ⚠️	-2280 B (-0.31%)	CCM: +512 B (+1.57%) RAM: +1648 B (+1.58%)
MATEKF722 ⚠️	-8048 B (-1.67%)	ITCM_RAM: -928 B (-7.40%) RAM: +56 B (+0.06%) TCM: +512 B (+2.02%)
MATEKF765	-1168 B (-0.15%)	DTCM_RAM: +512 B (+1.79%) SRAM1: +64 B (+0.05%)
MATEKH743 ⚠️	+4764 B (+0.60%)	D2_RAM: +256 B (+7.84%) DTCM_RAM: +512 B (+3.95%) ITCM_RAM: -888 B (-5.41%) RAM: +1948 B (+1.37%)

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

[github-actions]

Test firmware build ready — commit c9893bb

Download firmware for PR #12207

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