src/main/flight/imu.c (1)

200-208: Apply __attribute__((unused)) conditionally to avoid misleading compiler diagnostics.

Both applyAccError and applySensorCorrection are actively called on lines 346 and 348 when the code at line 331 (#if defined(SIMULATOR_BUILD) && defined(SKIP_IMU_CALC)) evaluates to false—which is the case in most build configurations. They only become unused in the specific scenario where both SIMULATOR_BUILD and SKIP_IMU_CALC are defined. Applying the attribute unconditionally suppresses legitimate compiler warnings in typical builds.

Consider either:

1. Conditionally defining the attribute: #if defined(SIMULATOR_BUILD) && defined(SKIP_IMU_CALC) #defineMAYBE_UNUSED __attribute__((unused))#endif``
2. Wrapping the function definitions in conditional compilation: #if !defined(SIMULATOR_BUILD) || !defined(SKIP_IMU_CALC)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/main/flight/imu.c` around lines 200 - 208, The functions applyAccError
and applySensorCorrection are annotated unconditionally with
__attribute__((unused)) which hides real-use diagnostics in normal builds;
change this so the attribute is only present when both SIMULATOR_BUILD and
SKIP_IMU_CALC are defined (or omit the attribute in builds where the functions
are used). Implement either a conditional macro (e.g., define MAYBE_UNUSED as
__attribute__((unused)) only when SIMULATOR_BUILD && SKIP_IMU_CALC) and apply
MAYBE_UNUSED to applyAccError/applySensorCorrection, or wrap the function
definitions themselves in `#if` !defined(SIMULATOR_BUILD) ||
!defined(SKIP_IMU_CALC) ... `#endif` so the functions are compiled normally when
they are actually referenced.

src/main/target/SITL/target.c (1)

58-60: Prefer integer literals over floating-point for integer constants.

Using 1e6 (a double literal) for an integer constant is non-idiomatic and may trigger -Wfloat-conversion warnings with stricter compiler settings. The static initialization also makes the redundant assignment at line 192 unnecessary.

♻️ Suggested improvement

 // System clock for SITL (fake 500MHz)
-uint32_t SystemCoreClock = 500 * 1e6;
+uint32_t SystemCoreClock = 500000000U;

And optionally remove the redundant assignment at line 192:

     clock_gettime(CLOCK_MONOTONIC, &start_time);
     printf("[system]Init...\n");
-    SystemCoreClock = 500 * 1e6; // fake 500MHz

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/main/target/SITL/target.c` around lines 58 - 60, Replace the
floating-point literal used to initialize SystemCoreClock with an integer
literal (e.g., use 500000000u or 500U * 1000000U) to avoid double-to-int
conversion and -Wfloat-conversion warnings; also remove the redundant later
assignment to SystemCoreClock (the duplicate initialization in the same
translation unit) so the static initialization is the single source of truth.
Ensure you update the symbol SystemCoreClock and eliminate the redundant
assignment that resets it elsewhere in this file.

src/main/common/kalman.c (1)

22-31: Remove the unused SITL sqrt fallback or use it consistently.

The helper added at Line 28 is never called because the non-ARM path uses sqrtf directly at Line 79. Keeping both paths adds dead code and drift risk.

♻️ Proposed cleanup

 `#ifdef` USE_ARM_MATH
 `#include` "arm_math.h"
-#else
-// Fallback for SITL and non-ARM targets
-static inline void arm_sqrt_f32(float x, float *out) {
-    *out = sqrtf(x);
-}
 `#endif`

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/main/common/kalman.c` around lines 22 - 31, The fallback helper
arm_sqrt_f32 is unused while code directly calls sqrtf; either remove the unused
arm_sqrt_f32 function or replace all direct sqrtf usages with the wrapper
(arm_sqrt_f32) so both ARM and non-ARM paths use the same API. Locate the
fallback definition arm_sqrt_f32 and the direct sqrtf call(s) (e.g., the sqrtf
at the Kalman computation site) and either delete the fallback block and keep
sqrtf everywhere, or change the sqrtf calls to call arm_sqrt_f32(out) and ensure
the wrapper signature matches existing usage.

src/main/common/kalman.c (1)

24-31: Add negative-input handling to fallback arm_sqrt_f32 for semantic consistency with CMSIS.

The fallback at lines 28–30 returns void whereas CMSIS arm_sqrt_f32 returns arm_status and handles negative inputs. More importantly, in the same file, the #ifdef USE_ARM_MATH branch (line 77) calls the fallback directly without pre-validation, while the #else branch (line 79) explicitly guards against negative values with fmaxf(value, 0.0f). This inconsistency should be reconciled.

Proposed refactor

 `#ifdef` USE_ARM_MATH
 `#include` "arm_math.h"
 `#else`
 // Fallback for SITL and non-ARM targets
-static inline void arm_sqrt_f32(float x, float *out) {
-    *out = sqrtf(x);
+typedef enum {
+    ARM_MATH_SUCCESS = 0,
+    ARM_MATH_ARGUMENT_ERROR = -1
+} arm_status;
+
+static inline arm_status arm_sqrt_f32(float x, float *out) {
+    if (x < 0.0f) {
+        *out = 0.0f;
+        return ARM_MATH_ARGUMENT_ERROR;
+    }
+    *out = sqrtf(x);
+    return ARM_MATH_SUCCESS;
 }
 `#endif`

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/main/common/kalman.c` around lines 24 - 31, The fallback arm_sqrt_f32
currently has a void signature and doesn't mirror CMSIS behavior; change it to
static inline arm_status arm_sqrt_f32(float x, float *out) and implement
CMSIS-like negative input handling: if x < 0 set *out = 0.0f and return
ARM_MATH_ARGUMENT_ERROR, otherwise set *out = sqrtf(x) and return
ARM_MATH_SUCCESS; update callers in this file that call arm_sqrt_f32 (the branch
that invokes it without pre-validation) to handle the returned arm_status if
needed, ensuring behavior is now consistent with the USE_ARM_MATH branch which
guards with fmaxf(value, 0.0f).

src/main/common/kalman.c (1)

24-31: Align non-ARM arm_sqrt_f32 fallback with CMSIS-DSP API contract for future compatibility.

The fallback implementation returns void, while the actual CMSIS-DSP arm_sqrt_f32 returns arm_status. Although current callsites at src/main/common/maths.c:107 and src/main/common/kalman.c:78 ignore the return value, matching the signature prevents potential portability issues if future code checks the status.

Proposed compatibility refactor

 `#ifdef` USE_ARM_MATH
 `#include` "arm_math.h"
 `#else`
 // Fallback for SITL and non-ARM targets
-static inline void arm_sqrt_f32(float x, float *out) {
-    *out = sqrtf(x);
+typedef enum {
+    ARM_MATH_SUCCESS = 0,
+    ARM_MATH_ARGUMENT_ERROR = -1
+} arm_status;
+
+static inline arm_status arm_sqrt_f32(float x, float *out) {
+    if (!out) {
+        return ARM_MATH_ARGUMENT_ERROR;
+    }
+    if (x < 0.0f) {
+        *out = 0.0f;
+        return ARM_MATH_ARGUMENT_ERROR;
+    }
+    *out = sqrtf(x);
+    return ARM_MATH_SUCCESS;
 }
 `#endif`

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/main/common/kalman.c` around lines 24 - 31, The fallback arm_sqrt_f32
currently returns void; change it to match the CMSIS-DSP API by returning
arm_status (use the same arm_status type or typedef it if missing) and return
ARM_MATH_SUCCESS after computing *out = sqrtf(x); update the fallback definition
(symbol: arm_sqrt_f32) so its signature and return value match CMSIS-DSP and
avoid future portability issues with callsites that may check the status.

src/main/target/SITL/target.c (1)

192-192: Comment wording is inaccurate about linkage.

Line 192 says “initialized as static,” but SystemCoreClock is not static (it has external linkage). Please reword to avoid confusion.

✏️ Suggested wording update

-    // SystemCoreClock initialized as static at declaration
+    // SystemCoreClock is initialized at declaration in this translation unit

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/main/target/SITL/target.c` at line 192, The comment "SystemCoreClock
initialized as static at declaration" is incorrect about linkage; update the
comment near the SystemCoreClock declaration in target.c to state that
SystemCoreClock is initialized at declaration and has external linkage (not
declared static), e.g., rephrase to "SystemCoreClock initialized at declaration
(external linkage)" or similar to avoid implying it is a static symbol.

src/main/flight/mixer.c (1)

1062-1064: LGTM! Float literals correctly eliminate implicit conversion warnings.

The change to use 1.0f and 0.0f instead of double literals aligns with the PR objective of eliminating compiler warnings.

For complete consistency, consider also updating the initializations at lines 1037-1038 which still use double literals:

Optional consistency improvement

-    float maxMotor = -1000.0;
-    float minMotor = 1000.0;
+    float maxMotor = -1000.0f;
+    float minMotor = 1000.0f;

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/main/flight/mixer.c` around lines 1062 - 1064, The float-literal change
around maxMotor, motorCorrection, and minMotor is correct; make the remaining
initializations in the same scope consistent by replacing any double literals
(e.g., 1.0, 0.0) used when initializing related variables (such as maxMotor,
minMotor, motorCorrection or nearby variables declared just above this logic)
with float literals (1.0f, 0.0f) so all floating-point constants in that
function use the 'f' suffix.

src/main/flight/mixer.c (1)

1062-1064: Good fix for float literal consistency.

The 1.0f and 0.0f suffixes correctly avoid implicit double-to-float conversions. However, the initializations at lines 1037-1038 still use double literals:

float maxMotor = -1000.0;
float minMotor = 1000.0;

Consider updating these for full consistency.

♻️ Proposed fix for consistency

-    float maxMotor = -1000.0;
-    float minMotor = 1000.0;
+    float maxMotor = -1000.0f;
+    float minMotor = 1000.0f;

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/main/flight/mixer.c` around lines 1062 - 1064, The float initializations
for maxMotor and minMotor use double literals; change their initializers to
float literals (e.g., use -1000.0f and 1000.0f) so the declarations float
maxMotor and float minMotor avoid implicit double-to-float conversions and match
the existing 1.0f/0.0f usage in the same function in mixer.c.

src/main/flight/pid.c (1)

753-753: Good fix for type consistency, but line 404 has the same pattern unfixed.

The 0.003f suffix correctly ensures float-precision arithmetic. However, the identical calculation at line 404 for dtermBoostMultiplier still uses 0.003 (double literal):

dtermBoostMultiplier = (pidProfile->dtermBoost * pidProfile->dtermBoost / 1000000) * 0.003;

Consider applying the same fix there for consistency.

♻️ Proposed fix for consistency

-    dtermBoostMultiplier = (pidProfile->dtermBoost * pidProfile->dtermBoost / 1000000) * 0.003;
+    dtermBoostMultiplier = (pidProfile->dtermBoost * pidProfile->dtermBoost / 1000000) * 0.003f;

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/main/flight/pid.c` at line 753, The multiplication for
dtermBoostMultiplier uses a double literal (0.003) causing mixed-precision;
update the expression setting dtermBoostMultiplier that uses
pidProfile->dtermBoost to use a float literal (0.003f) to match the float
arithmetic used for errorMultiplier, ensuring consistent float-precision
calculations for dtermBoostMultiplier.

src/main/sensors/voltage.c (1)

132-143: Consider conditional compilation instead of __attribute__((unused)).

While the attribute suppresses the warning effectively, wrapping the array definition in #ifdef USE_ADC would be architecturally cleaner since the array is only referenced within that guard (line 157).

♻️ Alternative approach using conditional compilation

+#ifdef USE_ADC
 static const uint8_t voltageMeterAdcChannelMap[] __attribute__((unused)) = {
     ADC_BATTERY,
 `#ifdef` ADC_POWER_12V
     ADC_POWER_12V,
 `#endif`
 `#ifdef` ADC_POWER_9V
     ADC_POWER_9V,
 `#endif`
 `#ifdef` ADC_POWER_5V
     ADC_POWER_5V,
 `#endif`
 };
+#endif

Then remove the __attribute__((unused)) since the array would only be compiled when actually used.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/main/sensors/voltage.c` around lines 132 - 143, The array
voltageMeterAdcChannelMap is marked with __attribute__((unused)) to silence
warnings but is only needed when the USE_ADC conditional is active; remove the
__attribute__((unused)) and instead wrap the entire voltageMeterAdcChannelMap
definition in the same `#ifdef` USE_ADC / `#endif` conditional used where the array
is referenced so it is compiled only when used (keep the ADC_* entries
unchanged), ensuring no unused-warning and cleaner architecture.

src/main/common/kalman.c (1)

86-93: Consider moving the #include <assert.h> to the top of the file.

The variance clamping and debug assertion correctly address the prior review feedback about NaN prevention. However, placing #include <assert.h> inside a function body (line 89) is unconventional and may confuse readers or cause issues with some static analyzers.

Suggested refactor

 `#include` <string.h>
 `#include` <math.h>
+#if defined(UNIT_TEST) || defined(SIMULATOR_BUILD)
+#include <assert.h>
+#endif

 `#ifdef` USE_ARM_MATH

Then at line 88-92:

 `#if` defined(UNIT_TEST) || defined(SIMULATOR_BUILD)
-#include <assert.h>
     // Debug: catch unexpectedly large negative variance (suggests calculation bug)
     assert(kalmanFilterStateRate[axis].axisVar > -1e-6f);
 `#endif`

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/main/common/kalman.c` around lines 86 - 93, Move the conditional include
for assert.h out of the function body and place it with the other includes at
the top of the file, keeping the same conditional guards (i.e., `#if`
defined(UNIT_TEST) || defined(SIMULATOR_BUILD) / `#include` <assert.h> / `#endif`)
so the debug assertion in the kalman variance block (where clampedVar is
computed from kalmanFilterStateRate[axis].axisVar and arm_sqrt_f32 is called to
set squirt) still compiles and runs unchanged; this keeps the assert available
for the runtime check without nesting an include inside the code block.

src/main/flight/imu.c (1)

200-200: Add explanatory comments for the __attribute__((unused)) annotations.

Both applyAccError (line 200) and applySensorCorrection (line 208) are actively called in normal builds (lines 346, 348), but these call sites are compiled out when SIMULATOR_BUILD && SKIP_IMU_CALC are both defined. The attributes correctly suppress warnings in that configuration, but marking actively-used functions as "unused" without explanation could confuse future maintainers.

Suggested clarifying comments

+// Note: marked unused to suppress warnings in SIMULATOR_BUILD+SKIP_IMU_CALC configs where call sites are compiled out
 static void __attribute__((unused)) applyAccError(quaternion *vAcc, quaternion *vError) {

+// Note: marked unused to suppress warnings in SIMULATOR_BUILD+SKIP_IMU_CALC configs where call sites are compiled out
 static void __attribute__((unused)) applySensorCorrection(quaternion *vError) {

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/main/flight/imu.c` at line 200, Add a short explanatory comment
immediately above the function declarations for applyAccError and
applySensorCorrection that explains why the __attribute__((unused)) is present:
note that these functions are invoked in normal builds but their call sites are
conditionally compiled out when SIMULATOR_BUILD && SKIP_IMU_CALC are defined, so
the attribute suppresses unused-function warnings in that configuration; include
the conditional macro names and mention the normal call sites (the runtime IMU
flow) so future maintainers understand the attribute’s purpose and can safely
remove or change it if the build logic changes.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes