perf(maths): backport BF #14790 O(1) sin/cos range reduction - #1327
perf(maths): backport BF #14790 O(1) sin/cos range reduction#1327nerdCopter wants to merge 12 commits into
Conversation
Replaces while-loop range reduction in sin_approx/cos_approx with roundf-based O(1) quadrant split (range_reduce -> r in [-0.5, 0.5]). sincosf_approx shares one range reduction and one r^2 computation between sin_poly5_r and cos_poly6_r instead of two full sin_approx evaluations. Consolidates maths.h FAST_MATH/VERY_FAST_MATH into a single FAST_MATH guard, matching BF's post-#14790 maths.h; VERY_FAST_MATH was always defined in EF (no target undefines it), so this is a no-op for build output, only a define-name delta reduction vs BF. Removes the prior |x| > 32 rad clamp-to-zero guard; the new reduction has no such limit (mechanical consequence of dropping the while loop, not a design choice made here). Call sites (filter.c, imu.c, gps.c, sdft.c, rangefinder.c, fc_rc.c) are unchanged per scope; BF's own callsite migration to sincosf_approx in vector.c/imu.c/rc.c is left as optional follow-up, consistent with the source issue's own "Optional follow-up" framing. Extends maths_unittest.cc: sweeps sin_approx/cos_approx against sinf/ cosf at BF's own tolerance (3.2e-6 sin, 3.5e-6 cos, EF's prior cos tolerance retained), adds explicit edge-case checks (0, +-pi, +-pi/2, large multiples of 2*pi with a magnitude-scaled epsilon), and verifies sincosf_approx output matches independent sin_approx/cos_approx calls on the same angle. Source: betaflight/betaflight#14790 (ledvinap, merged 2026-03-04). Closes emuflight#1269. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
CodeRabbit finding: sinCombined/cosCombined were bare-declared before being passed to sincosf_approx(), inconsistent with repo's UNIT-TEST.instructions.md zero-init convention. Related major finding (unguarded (int)qf conversion on non-finite/huge input in sin_approx/cos_approx/sincosf_approx) not applied: matches betaflight/betaflight#14790's merged, reviewed implementation exactly (no guard present upstream either); adding one would diverge from the BF reference this is a mechanical backport of.
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThe FAST_MATH sine and cosine approximations now use constant-time quadrant range reduction and shared polynomial evaluation. Paired trigonometric callsites use ChangesFast trigonometry migration
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@coderabbitai two things:
|
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '=== Relevant files ==='
fd -a '^(maths\.(c|h)|vector\.c|imu\.c|rc\.c|fc_rc\.c|filter\.c)$' src || true
printf '%s\n' '=== maths.c structure ==='
ast-grep outline src/main/common/maths.c --items all --type function --match 'sin|cos|sincos' || true
printf '%s\n' '=== Trigonometric implementations and uses ==='
rg -n -C 5 -P '\b(sin_approx|cos_approx|sincosf_approx)\s*\(' src/main \
-g '*.{c,h}' || true
printf '%s\n' '=== Current PR diff for maths and candidate call sites ==='
git diff --find-renames --unified=40 'HEAD^' -- \
src/main/common/maths.c src/main/common/maths.h \
src/main/common/vector.c src/main/flight/imu.c \
src/main/fc/rc.c src/main/fc/fc_rc.c src/main/common/filter.c || true
printf '%s\n' '=== Upstream Betaflight `#14790` metadata and changed files ==='
gh pr view 14790 --repo betaflight/betaflight --json title,state,mergedAt,url,body,files \
--jq '{title, state, mergedAt, url, body, files: [.files[] | {path, additions, deletions}]}' || true
printf '%s\n' '=== Upstream Betaflight `#14790` patch (relevant paths) ==='
gh api -H 'Accept: application/vnd.github.diff' \
repos/betaflight/betaflight/pulls/14790 \
| awk '
/^diff --git / {show = ($0 ~ /(maths\.(c|h)|vector\.c|imu\.c|rc\.c)$/)}
show {print}
' || trueLength of output: 30010 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '=== Candidate function maps ==='
ast-grep outline src/main/common/maths.c src/main/common/filter.c src/main/fc/fc_rc.c src/main/flight/imu.c \
--items all --type function --view expanded || true
printf '%s\n' '=== Enclosing candidate implementations ==='
for spec in \
'src/main/common/maths.c:220:270' \
'src/main/common/filter.c:90:205' \
'src/main/fc/fc_rc.c:200:250' \
'src/main/flight/imu.c:200:245' \
'src/main/flight/imu.c:410:440'
do
file="${spec%%:*}"
rest="${spec#*:}"
start="${rest%%:*}"
end="${rest##*:}"
printf '\n--- %s, Lines %s-%s ---\n' "$file" "$start" "$end"
sed -n "${start},${end}p" "$file"
done
printf '%s\n' '=== Declarations and constraints for RC camera angle/configuration ==='
rg -n -C 4 -P '\b(cinematicYaw|fpvCamAngleDegrees|currentPitchAngle|lastFpvCamAngleDegrees)\b' \
src/main -g '*.{c,h}' || true
printf '%s\n' '=== Angle type/source constraints for rotation and IMU paths ==='
rg -n -C 4 -P '\b(buildRotationMatrix|fp_angles_t|courseOverGround|yawHalf|qpAttitude)\b' \
src/main -g '*.{c,h}' || true
printf '%s\n' '=== Existing maths tests and finite/non-finite coverage ==='
fd -a 'maths_unittest.cc' src/test \
-x sh -c 'echo "--- $1 ---"; rg -n -C 5 -P "\b(TestFastTrigonometry|TestSincosf|sin_approx|cos_approx|sincosf_approx|NAN|INFINITY)\b" "$1"' sh {} || true
printf '%s\n' '=== Compiler language standard / float-conversion diagnostic flags ==='
rg -n -C 2 -P '(std=|Wconversion|float-conversion|fsanitize)' Makefile src -g 'Makefile' -g '*.mk' -g '*.cmake' || trueLength of output: 56285
1.
|
| EmuFlight location | Equivalent / reason |
|---|---|
src/main/common/maths.c — buildRotationMatrix |
Local equivalent of Betaflight’s vector.c rotation-matrix work; three same-angle pairs. |
src/main/common/filter.c — biquadFilterUpdate |
Same omega feeds both calls; this is a worthwhile direct conversion. |
src/main/common/sdft.c |
Same phi feeds both calls during twiddle-factor initialization. |
src/main/fc/fc_rc.c — scaleRcCommandToFpvCamAngle |
Local equivalent of Betaflight’s rc.c; preserve the existing static cached factors and replace each paired calculation with one combined call. |
src/main/flight/imu.c — GPS course correction |
courseOverGround is already normalized, then used for both values. |
src/main/flight/imu.c — imuQuaternionHeadfreeOffsetSet |
yawHalf is passed separately to cosine and sine. |
src/main/common/filter.c’s SVF paths already use sincosf_approx, so they need no change.
Recommended PR expansion
- Migrate every existing local, paired same-angle
sin_approx/cos_approxcalculation tosincosf_approx. - Preserve variable ordering and existing cached/static behavior.
- Add focused regression coverage for the migrated math path; handle non-finite/oversized-input hardening separately if upstream parity remains a constraint.
- 📌 Create a pull request with these changes
…pprox Per CodeRabbit analysis on PR emuflight#1327 (comment #5095429276), completes BF #14790's callsite migration for EmuFlight's local equivalents: - maths.c buildRotationMatrix: roll/pitch/yaw pairs - filter.c biquadFilterUpdate: omega - sdft.c twiddle-factor init: phi - fc_rc.c scaleRcCommandToFpvCamAngle: both branches - imu.c applySensorCorrection: GPS courseOverGround - imu.c imuQuaternionHeadfreeOffsetSet: yawHalf Each site shares one range reduction and one r*r computation between sin/cos instead of two independent evaluations. No behavior change — sincosf_approx is verified equal to independent sin_approx+cos_approx calls in maths_unittest's TestSincosfApproxMatchesSeparateCalls. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
AI Generated comment Callsite migration (item 2) implemented in commit ebcdb65: Item 1 ( |
lastFpvCamAngleDegrees stayed uint8_t initialized to 0, coinciding with the field's own default value. Widened to int16_t initialized to -1 (outside uint8_t's full 0-255 range) so the cache-invalidation check can never accidentally match a real config value, guaranteeing the first post-boot call always recomputes cosFactor/sinFactor instead of relying on the hardcoded 1.0/0.0 defaults matching by coincidence. No functional change for the common case (angle 0): sincosf_approx(0) produces the same cos=1.0/sin=0.0 the old code assumed by default. Matches betaflight/betaflight#15498 (nerdCopter), which applies the same hardening to BF's analogous scaleRawSetpointToFpvCamAngle after betaflight/betaflight#14790 introduced the pattern. rxConfig()->fpvCamAngleDegrees itself (the MSP-facing config field) is untouched, still uint8_t. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
|
@coderabbitai analyze (not review) two things:
|
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '=== PR status and target commits ==='
git status --short
git show --no-patch --format='%H%n%P%n%s' 66afda72c
git show --no-patch --format='%H%n%P%n%s' ebcdb65e8
printf '%s\n' '=== Unit-test structure and reported region ==='
ast-grep outline src/test/unit/maths_unittest.cc --items all --type function --match 'TestFastTrigonometryEdgeCases|TestSincosfApproxMatchesSeparateCalls' --view expanded || true
nl -ba src/test/unit/maths_unittest.cc | sed -n '230,270p'
printf '%s\n' '=== Exact unit-test expressions involving k ==='
rg -n -C 4 -P '\bfor\s*\(\s*int\s+k\s*=|<<\s*k\b|\bk\s*<<|>>\s*k\b|\bk\s*>>' src/test/unit/maths_unittest.cc
printf '%s\n' '=== FPV function and cache identifier references ==='
ast-grep outline src/main/fc/fc_rc.c --items all --type function --match 'scaleRcCommandToFpvCamAngle' --view expanded || true
rg -n -C 12 -P '\bscaleRcCommandToFpvCamAngle\s*\(|\blastFpvCamAngleDegrees\b|\bfpvCamAngleDegrees\b|\bcinematicYaw\b' src/main/fc/fc_rc.c src/main -g '*.{c,h}'
printf '%s\n' '=== Follow-up commit diff ==='
git show --format=fuller --find-renames --unified=50 66afda72c -- src/main/fc/fc_rc.c
printf '%s\n' '=== Field declaration/default/reset paths ==='
rg -n -C 5 -P '\bfpvCamAngleDegrees\b' src/main -g '*.{c,h}'Length of output: 2191 🏁 Script executed: #!/bin/bash
set -u
printf '%s\n' '=== Reported test lines (numbered with awk) ==='
awk 'NR >= 230 && NR <= 270 { printf "%6d %s\n", NR, $0 }' src/test/unit/maths_unittest.cc
printf '%s\n' '=== Every syntactic occurrence relevant to k and shift operators in this test file ==='
rg -n -C 3 -P '\bfor\s*\(\s*int\s+k\s*=|<<\s*k\b|\bk\s*<<|>>\s*k\b|\bk\s*>>|\bk\b' \
src/test/unit/maths_unittest.cc || true
printf '%s\n' '=== FPV cache function ==='
awk '/static void scaleRcCommandToFpvCamAngle\(void\)/,/^}/ { printf "%6d %s\n", NR, $0 }' src/main/fc/fc_rc.c
printf '%s\n' '=== All references to cache function and relevant configuration fields ==='
rg -n -C 6 -P '\b(scaleRcCommandToFpvCamAngle|fpvCamAngleDegrees|cinematicYaw|lastFpvCamAngleDegrees)\b' \
src/main -g '*.{c,h}' || true
printf '%s\n' '=== Exact follow-up change ==='
git diff 66afda72c^ 66afda72c -- src/main/fc/fc_rc.c || true
printf '%s\n' '=== Cache-field type/range/default metadata ==='
rg -n -C 8 -P '\bfpvCamAngleDegrees\b' src/main -g '*.{c,h}' || trueLength of output: 65971
|
|
@coderabbitai analyze (not review) the full functionality of this PR at current HEAD (66afda7) — the complete accumulated diff from base, not just the two narrow points in the prior comment. Specifically: does the combination of (a) the O(1) sin/cos range-reduction rewrite in maths.c/maths.h, (b) the six callsite migrations to sincosf_approx (buildRotationMatrix, biquadFilterUpdate, sdft.c twiddle-init, scaleRcCommandToFpvCamAngle both branches, applySensorCorrection GPS course, imuQuaternionHeadfreeOffsetSet), and (c) the fc_rc.c lastFpvCamAngleDegrees cache-sentinel widening hold together correctly as a whole? Look for anything across those combined changes — argument-order mistakes, sign errors, missed callers, interaction effects between commits — not just each commit in isolation. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
target="66afda72c"
base="$(git merge-base "$target" origin/master 2>/dev/null || git merge-base "$target" HEAD^)"
printf '=== Base and accumulated commits ===\n'
printf 'base=%s\ntarget=%s\n' "$base" "$target"
git log --oneline --reverse "${base}..${target}"
printf '\n=== Complete changed-file summary ===\n'
git diff --stat "$base" "$target"
git diff --name-status "$base" "$target"
printf '\n=== Aggregate diff: relevant implementation and tests ===\n'
git diff --find-renames --unified=35 "$base" "$target" -- \
src/main/common/maths.c src/main/common/maths.h \
src/main/common/filter.c src/main/common/sdft.c \
src/main/fc/fc_rc.c src/main/flight/imu.c \
src/test/unit/maths_unittest.cc
printf '\n=== Function maps ===\n'
ast-grep outline src/main/common/maths.c src/main/common/filter.c src/main/common/sdft.c \
src/main/fc/fc_rc.c src/main/flight/imu.c src/test/unit/maths_unittest.cc \
--items all --type function --view expanded || true
printf '\n=== Current approximate-trig definitions and all callsites ===\n'
rg -n -C 5 -P '\b(sin_approx|cos_approx|sincosf_approx)\s*\(' \
src/main src/test -g '*.{c,cc,h}' || true
printf '\n=== Possible remaining adjacent same-angle sin/cos pairs ===\n'
rg -n -U -P '(?s)\b(?:sin_approx|cos_approx)\s*\(\s*([^)\n]+)\s*\)\s*;.{0,300}\b(?:sin_approx|cos_approx)\s*\(\s*\1\s*\)\s*;' \
src/main -g '*.{c,h}' || true
printf '\n=== FPV cache, config range/default, and callers ===\n'
rg -n -C 8 -P '\b(scaleRcCommandToFpvCamAngle|lastFpvCamAngleDegrees|fpvCamAngleDegrees|cinematicYaw)\b' \
src/main -g '*.{c,h}' || true
printf '\n=== FAST_MATH / VERY_FAST_MATH references and trig declarations ===\n'
rg -n -C 3 -P '\b(FAST_MATH|VERY_FAST_MATH|INV_PIO2|sin_approx|cos_approx|sincosf_approx)\b' \
src/main src/test -g '*.{c,cc,h}' || trueLength of output: 119472 🏁 Script executed: #!/bin/bash
set -euo pipefail
target="66afda72c"
base="$(git merge-base "$target" origin/master 2>/dev/null || git merge-base "$target" HEAD^)"
printf '%s\n' '=== Exact per-commit changes in the accumulated series ==='
for commit in b3a6f6b5b bee1b0efe ebcdb65e8 66afda72c; do
git show --format='%H %s' --stat --oneline "$commit"
done
printf '%s\n' '=== All VERY_FAST_MATH references (repository-wide) ==='
rg -n -C 3 -P '\bVERY_FAST_MATH\b' . -g '!**/.git/**' || true
printf '%s\n' '=== All approximate trig call expressions, grouped by source file ==='
rg -n -P '\b(?:sin_approx|cos_approx|sincosf_approx)\s*\(' src/main -g '*.{c,h}' || true
printf '%s\n' '=== FPV cache state-relevant history ==='
git log -p -S'lastFpvCamAngleDegrees' -- src/main/fc/fc_rc.c | sed -n '1,260p'
printf '%s\n' '=== Upstream source comparison: Betaflight PRs ==='
gh api -H 'Accept: application/vnd.github.diff' repos/betaflight/betaflight/pulls/14790 \
| awk '/^diff --git a\\/src\\/main\\/common\\/maths\\.(c|h)/,/^diff --git / { print }' || true
gh api -H 'Accept: application/vnd.github.diff' repos/betaflight/betaflight/pulls/15498 \
| awk '/^diff --git a\\/src\\/main\\/fc\\/rc\\.c/,/^diff --git / { print }' || true
printf '%s\n' '=== Static float32 model of quadrant selection against libm ==='
python3 - <<'PY'
import ctypes
import math
import random
import struct
f32 = lambda x: struct.unpack('<f', struct.pack('<f', x))[0]
pio2 = f32(math.pi / 2.0)
inv_pio2 = f32(2.0 / f32(math.pi))
def roundf_away(x):
return math.floor(x + 0.5) if x >= 0 else math.ceil(x - 0.5)
def sin_poly(r):
c0 = f32(float.fromhex('0x1.921f1cp0'))
c1 = f32(-float.fromhex('0x1.4a974p-1'))
c2 = f32(float.fromhex('0x1.3db294p-4'))
s = f32(r * r)
return f32(r * f32(f32(f32(c2*s) + c1)*s + c0))
def cos_poly(r):
d1 = f32(-float.fromhex('0x1.3bd39cp0'))
d2 = f32(float.fromhex('0x1.03bp-2'))
d3 = f32(-float.fromhex('0x1.4e5eecp-6'))
s = f32(r * r)
return f32(f32(f32(f32(d3*s) + d2)*s + d1)*s + 1.0)
def approx(x):
t = f32(x * inv_pio2)
qf = f32(roundf_away(t))
q = int(qf)
r = f32(t - qf)
sb, cb = sin_poly(r), cos_poly(r)
s = cb if q & 1 else sb
c = -sb if q & 1 else cb
if q & 2:
s, c = -s, -c
return f32(s), f32(c)
angles = [f32(k * math.pi / 4) for k in range(-32, 33)]
angles += [f32(random.uniform(-1000.0, 1000.0)) for _ in range(10000)]
max_s = max_c = 0.0
for x in angles:
s, c = approx(x)
max_s = max(max_s, abs(s - f32(math.sin(x))))
max_c = max(max_c, abs(c - f32(math.cos(x))))
print(f'finite samples={len(angles)} max_abs_sin_error={max_s:.9g} max_abs_cos_error={max_c:.9g}')
for k in range(-4, 5):
x = f32(k * math.pi / 2)
s, c = approx(x)
print(f'k={k:+d}: sin={s:+.7f}, cos={c:+.7f}')
PY
printf '%s\n' '=== Diff completeness check ==='
git diff --check "$base" "$target"Length of output: 7544
Overall conclusionThe three parts hold together correctly:
I found no argument-order swap, sign inversion, missed paired approximate-trig caller, or new cross-commit interaction defect. Fast-trig implementationThe shared reduction is consistent across t = x * INV_PIO2;
qf = roundf(t);
q = (int)qf;
r = t - qf;The quadrant code then has the expected results for both positive and negative quadrants:
The combined implementation uses the same sincosf_approx(angle, &sine, &cosine);The known non-finite/out-of-range One expected numerical characteristic of simple float range reduction remains: error increases for very large finite angles because float precision loses low-order range-reduction information. That is separate from the integer-conversion UB threshold and does not affect the operational ranges of the migrated paths; it is also inherent to the selected upstream implementation. Callsite migration verificationAll migrated uses preserve the old value flow:
A repository-wide scan found no remaining paired same-angle FPV cache sentinel interactionThe sentinel change is correct in the combined series: static int16_t lastFpvCamAngleDegrees = -1;
It does not alter configuration layout, persistence, MSP serialization, or the function’s externally observable API; the widened value is local static state only. The previously noted behavior remains pre-existing: after a non-cinematic angle has been cached, enabling cinematic yaw recalculates from pitch without updating the non-cinematic cache key; disabling cinematic yaw with the same configured angle can retain pitch-derived factors until the setting changes. The new sentinel neither introduces nor worsens that behavior. It does improve the startup/cinematic-first transition because Verification boundary
|
|
@coderabbitai, AI Generated comment Follow-up on the cinematicYaw cache-staleness observation in this comment (#1327 (comment)). Filed as #1351, then closed not-planned: the finding's precondition requires |
|
Tip For best results, initiate chat on the files or code changes.
Therefore, the required cinematic-yaw transition cannot retain pitch-derived factors in the non-cinematic path. No change is needed in this PR. ✏️ Learnings added
You are interacting with an AI system. |
|
AI Generated comment Claude deemed safe to merge. Reasoning:
|
|
AI Generated comment Status update: real-flight test flown on FOXEERF722V4 ( PR description updated with this result plus: the cinematicYaw finding's disposition (#1351, closed not-planned — code-flow correct but the precondition isn't reachable given the field's persistent, reboot-on-save lifecycle), a correction that D-term dynamic notch runs through SVF not Overall: PASS. |
1 participant
AI Generated pull-request
Summary
Backports betaflight/betaflight#14790 (ledvinap, merged 2026-03-04) to EmuFlight's
src/main/common/maths.c/maths.h. Closes #1269.sin_approx/cos_approx: replaces the while-loop range reduction with an O(1)roundf-based quadrant split (t = x * INV_PIO2,qf = roundf(t),r = t - qfin[-0.5, 0.5]), matching BF's merged implementation.sincosf_approx: now shares one range reduction and oner*rcomputation between a 5th-order sin polynomial and a 6th-order cos polynomial, instead of two full independentsin_approx/cos_approxevaluations.maths.h: consolidatesFAST_MATH/VERY_FAST_MATHinto a singleFAST_MATHguard, matching BF's post-#14790 header. EF had both always defined with no target ever undefining either, so this is a no-op for build output — a define-name delta reduction only.Callsite migration
BF's own PR #14790 also migrated several same-angle sin/cos callsites (
vector.c,imu.c,rc.c) to the combinedsincosf_approx. This PR now includes the EmuFlight-local equivalents (upstream filenames don't map 1:1 to EF's structure):src/main/common/maths.cbuildRotationMatrix— roll/pitch/yaw pairssrc/main/common/filter.cbiquadFilterUpdate—omegasrc/main/common/sdft.ctwiddle-factor init —phisrc/main/fc/fc_rc.cscaleRcCommandToFpvCamAngle— both branches (cachedcosFactor/sinFactorstatic storage preserved)src/main/flight/imu.capplySensorCorrection— GPScourseOverGroundsrc/main/flight/imu.cimuQuaternionHeadfreeOffsetSet—yawHalffilter.c's SVF paths (svfLowpassFilterUpdate/svfNotchUpdate) already calledsincosf_approx, no change needed there. Remainingsin_approx/cos_approxcall sites (gps.c,imu.csmall-angle/throttle-angle,pid.c,rangefinder.c,acos_approxinternals) are single-use or not same-angle pairs — verified and left untouched.No behavior change: each migrated site now shares one range reduction and one
r*rbetween sin/cos instead of two independent evaluations;sincosf_approxis verified equal to independentsin_approx+cos_approxcalls inTestSincosfApproxMatchesSeparateCalls.Behavior note
The prior implementation clamped
|x| > 32rad (5*360 deg) to0.0f. The new O(1) reduction has no such clamp (mechanical consequence of removing the while loop, matching BF exactly) — grep acrosssrc/main/found no caller passing angles anywhere near that magnitude, so this is not expected to change any real behavior.Numerical verification
src/test/unit/maths_unittest.ccextended:TestFastTrigonometrySinCos: sweep[-10*pi, 10*pi)steppi/300againstsinf/cosf— measured max error: sin 3.173947e-06, cos 3.241003e-06 (tolerances: sin 3.2e-6 matching BF's own updated tolerance, cos 3.5e-6 retaining EF's prior tolerance headroom).TestFastTrigonometryEdgeCases: exact0,+-pi,+-pi/2, and large multiples of2*pi(kin-50..50step 10) against libm, with a magnitude-scaled epsilon for the large-x cases (float32 mantissa precision loss in representingxitself dominates at that magnitude — same property applies to any float32 trig approximation, not specific to this algorithm).TestSincosfApproxMatchesSeparateCalls: verifiessincosf_approxoutput matches independentsin_approx+cos_approxcalls on the same angle across[-20*pi, 20*pi).Build/test verification
make clean_test && CCACHE_DISABLE=1 make test: full suite green, 0 failures (39 test binaries, includestest_maths_unittest,test_common_filter_unittest,test_flight_imu_unittest— all PASS post-callsite-migration).CCACHE_DISABLE=1 make HELIOSPRING(real-hardware F4 bench target): build succeeded, FLASH1 35.97%, RAM 73.07%, CCM 26.06% — small flash increase (35.76% pre-migration) from additional inlinedsincosf_approxcall sites, no RAM/CCM change.Independent verification (SITL + real hardware)
9b144d436) and this branch (ebcdb65e8) asTARGET=SITL, forced non-default config (non-standard board alignment, D-term dynamic notch on) to exercise otherwise-conditional migrated sites, fed both an identical deterministic synthetic gyro/accel stream.buildRotationMatrix's output rotation matrix matched between builds to a max abs element diff of 8.34e-7 — an order of magnitude under the 3.2e-6 unit-test tolerance, confirming the migration is numerically equivalent rather than a behavior change. (sdft.c/biquadFilterUpdatesites weren't reached in this SITL session's observation window — a property of SITL's own init/calibration sequencing, not chased further given the genericsincosf_approxequivalence proof already covers the same code path.)9b144d4) vs this branch (ebcdb65) — no NaN/Inf in either blackbox log, no wild divergence or clipping ingyroADC/axisP/axisI/axisD/accSmooth/motor[]; differences attributable to the two takes' differing physical motion, not the code.rc.cFPV-cam migration (loststaticcaching oncosFactor/sinFactor, and a missing degrees→radians conversion), both fixed before BF merged. Verified EF'sfc_rc.cequivalent (scaleRcCommandToFpvCamAngle) has neither bug —staticstorage and the pre-existing* RADconversion were both preserved by this migration.CI status
a96e75f67(synced withupstream/master, no further changes to this PR's own diff since66afda72c): full build matrix (12 target groups) and bothtestjobs (Linux + macOS) PASS.sitlshowed a transient GitHub Actions infrastructure failure (Failed to resolve action download info: Service Unavailable, during environment setup, before any EmuFlight build/test step ran) — not a code or test failure; re-run triggered.src/test/unit/maths_unittest.cc:252-253("Shifting by a negative value is undefined behaviour"). Those lines areEXPECT_NEAR(...) << "k=" << k << " x=" << x;— googletest's stream-insertion operator, not a bitwise shift;k(afor (int k = -50; k <= 50; k += 10)loop variable) is never used in an actual bit-shift anywhere in this function. Confirmed as a cppcheck false positive both by inspection and by CodeRabbit analysis (perf(maths): backport BF #14790 O(1) sin/cos range reduction #1327 (comment)), consistent with this repo's known Codacy/cppcheck false-positive pattern on similar constructs (see e.g. PR fix(board-info): remove colliding flags, wire up BOARD_NAME + USB string #1299, PR chore: suppress cppcheck missingIncludeSystem false positive (Codacy) #1156).Real-flight verification
Flown 2026-08-06 on FOXEERF722V4 (primary F7 hardware test rig), firmware built from this exact
branch tip (
a96e75f67).debug_mode = FFT_FREQ(watches the SDFT dynamic-notch center-frequencypath this PR's
sdft.ctwiddle-factor-init migration touches; raw/unfiltered gyro no longer needsa debug-mode slot since it's now an always-on blackbox field per #1331). Log: 67,480 rows,
dterm_dyn_notch_enable=1,dynamic_gyro_notch_count=2, bounds150–400Hz.debug[0]/debug[1]): 167–370 Hz and 187–400 Hz, both withinthe configured bounds throughout the flight.
gyroADC/gyroUnfilttrack closely;motor[0..3]stay in normal DSHOT range with no sustainedclipping;
axisDnormal. No regression signal.Follow-up hardening: fc_rc.c cache sentinel
Associated with betaflight/betaflight#15498 (nerdCopter, still OPEN upstream) — closes betaflight/betaflight#15495, a real bug in BF's own
scaleRawSetpointToFpvCamAngle(loststaticcaching, cross-checked and confirmed absent from EF above). BF's fix also widens its cache-invalidation sentinel from a value inside the config field's own domain to one outside it, so the first post-boot call is guaranteed to recompute rather than relying on coincidence. Applied the same hardening here:src/main/fc/fc_rc.clastFpvCamAngleDegrees—uint8_t(init0) →int16_t(init-1, outsideuint8_t's full 0-255 domain). No behavior change for the common case (fpvCamAngleDegrees = 0, the default):sincosf_approx(0)producescos=1.0f/sin=0.0f, identical to the hardcoded static initializers the old skip-path relied on.rxConfig()->fpvCamAngleDegreesitself — the MSP-facing config field — is untouched, stilluint8_t; no wire-format impact. Not gated on betaflight/betaflight#15498 merging — the fix is independently verified via BF's own added regression test (rc_unittest.cc, confirmed fails pre-fix/passes post-fix) plus this PR's local test suite.CodeRabbit findings
(int)qfconversion on non-finite/huge input insin_approx/cos_approx/sincosf_approx. Analyzed further per review discussion — confirmed genuine undefined behavior on NaN/Inf/out-of-int-range input, matches faster cos and sin method betaflight/betaflight#14790's merged, reviewed implementation exactly (no guard present upstream either). No credible flight-path trigger identified (GPS course normalized to[-pi,pi], FPV camera angle constrained to[0,90]degrees, board-alignment inputs are integer degrees, IMU-derived angles bounded in normal operation). Not applied — adding a guard would diverge from the BF reference this change mechanically ports; would need to be a separately justified hardening commit if pursued.sinCombined/cosCombinedlocals in the newsincosf_approxconsistency test were bare-declared. Fixed — zero-initialized perUNIT-TEST.instructions.mdconvention.fc_rc.cscaleRcCommandToFpvCamAnglecache-staleness observation. Filed as FPV cam-angle factors can go stale after disabling cinematicYaw #1351, then closed not-planned: the finding's precondition needscinematicYawto toggle at runtime without a reboot, but it's a persistent config field changed only via save+reboot, which reinitializes the cache. Code-flow was correct; the precondition isn't reachable in normal use.biquadFilterUpdateis no longer the D-term dynamic notch's call site on current master — refactor(filter): replace D-term dynamic notch biquad with SVF notch #1329 (merged) replaced it withsvfNotchUpdate/svfNotchApplyinpid.c, which already calledsincosf_approxpre-perf(maths): backport BF #14790 O(1) sin/cos range reduction #1327 with zero source change. An earlier revision of a PR comment referenced the old call site; corrected in place.Upstream findings (BF-side only, no EF impact)
A completeness cross-check of this PR against BF #14790/#15498 surfaced two BF-only issues, filed as betaflight/betaflight#15526 (no fix applied here — neither has an EmuFlight equivalent):
imu.c imuComputeQuaternionFromRPY, introduced by #14790 itself: the pre-#14790 code negatedinitialYawbefore both the sin and cos calls; the migration to a singlesincosf_approxcall dropped that negation, silently flippingsinYaw's sign (cosine's evenness masked it). Confirmed still present in current BF master. EF has no equivalent function.src/main/common/sdft.c:45left unmigrated in BF's own #14790 (still separatecos_approx+sin_approx), the same callsite this PR'ssdft.cmigration covers on the EF side.Credit
Source: betaflight/betaflight#14790, opened by @Quick-Flash implementing @ledvinap's proposed algorithm. Polynomial coefficients and range-reduction technique originate from that work, ported mechanically here.
Test plan
make clean && make test— 0 failuresmake HELIOSPRING— build succeeds, no size regressionfc_rc.ccache-sentinel hardening re-verified:make clean_test && make test39/39 PASS,make HELIOSPRINGclean (+36 bytes, consistent with widening one static local by 1 byte)debug_mode=FFT_FREQ) — PASS, no regression, see Real-flight verification sectionOverall status: PASS.
reviewDecision: APPROVED,mergeable: MERGEABLE, zero open review threads. No merge-blocking items; Codacy false positive is the only non-green check.Summary by CodeRabbit
Performance Improvements
Bug Fixes
Tests