Skip to content

Conversation

@MrD-RC
Copy link
Member

@MrD-RC MrD-RC commented Nov 2, 2025

User description

Fixes #2420

I have found a bug that stopped FW level trim being shown as a float, even if set to a float. It would have affected any units that didn't require a conversion.

I did look at adding a slider. But there was an issue with it having a number with decimal places. It would show them when you used the slider. But not when you entered the page. I will take a look at some point. But, to be honest, it's getting late and this fixed the decimal places issue raised.

With regard to the original issue

  • I have set it to 3 decimal places, to match the CLI.
  • I will look in to getting the slider working, probably in another PR. This fix may affect other data, so should go in straight away.
  • History and logging changes is something that you'd have to do yourself. It's not in the scope of INAV to maintain history of changes to parameters. It would just waste a lot of memory. Potentially if SD card drivers are improved. Logging changes to parameter could happen on there. But I think that's a fair way off.

PR Type

Bug fix


Description

  • Fix FW level trim displaying as integer instead of float

  • Add explicit decimal step attribute to trim input field

  • Ensure decimal places preserved when no unit conversion applied

  • Standardize numeric literals to float format in conversion tables


Diagram Walkthrough

flowchart LR
  A["FW Level Trim Input"] -->|Add data-step attribute| B["Preserve Decimal Places"]
  C["Unit Conversion Logic"] -->|Check multiplier == 1| D["Apply countDecimals to step"]
  E["Numeric Conversion Tables"] -->|Standardize to float| F["Consistent decimal handling"]
  B --> G["Display as float with 3 decimals"]
  D --> G
  F --> G
Loading

File Walkthrough

Relevant files
Bug fix
settings.js
Decimal handling and unit conversion table standardization

js/settings.js

  • Fixed decimal place handling when multiplier equals 1 by calling
    countDecimals(step)
  • Changed step attribute lookup to prioritize data("step") over
    attr('step')
  • Standardized numeric literals in unitRatioTable to float format (e.g.,
    100 to 100.0)
  • Updated variable name from deg to decimals in countDecimals function
    for clarity
  • Removed unnecessary whitespace in conditional blocks
+20/-21 
pid_tuning.html
Add number type and decimal step to trim input                     

tabs/pid_tuning.html

  • Added type="number" attribute to FW level trim input element
  • Added data-step="0.001" attribute to enforce 3 decimal place
    granularity
  • Ensures trim value displays and accepts float values with proper
    precision
+1/-1     

@MrD-RC MrD-RC added this to the 9.0 milestone Nov 2, 2025
@qodo-code-review
Copy link
Contributor

qodo-code-review bot commented Nov 2, 2025

PR Compliance Guide 🔍

(Compliance updated until commit c79c943)

Below is a summary of compliance checks for this PR:

Security Compliance
🟢
No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
🟡
🎫 #2420
🟢 Level Trim [deg] input should allow manual changes with 2 decimal place precision.
🔴 Consider adding a slider or trim/slider pot control to fine-tune by +/-0.25 in 0.05
increments.
Provide a more obvious display/history of Auto Level changes to previous level settings
(change history log).
Level Trim [deg] input should display values with 2 decimal places (and not round to
integer).
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
🟢
Generic: Meaningful Naming and Self-Documenting Code

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status: Passed

Generic: Secure Error Handling

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

Status: Passed

Generic: Secure Logging Practices

Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.

Status: Passed

Generic: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status:
No audit logs: The new logic updating numeric input handling and unit conversions does not include any
logging of critical actions, but it is unclear if such logging is required for this UI
setting change.

Referred Code
const multiObj = getUnitMultiplier();

const multiplier = multiObj.multiplier;
const unitName = multiObj.unitName;

let decimalPlaces = 0;
// Update the step, min, and max; as we have the multiplier here.
if (element.attr('type') == 'number') {
    let step = parseFloat(element.data("step")) || parseFloat(element.attr('step')) || 1;

    if (multiplier !== 1) { 
        decimalPlaces = Math.min(Math.ceil(multiplier / 100), 3);
        // Add extra decimal place for non-integer conversions.
        if (multiplier % 1 != 0 && decimalPlaces < 3) {
            decimalPlaces++;
        }
        step = 1 / Math.pow(10, decimalPlaces);
    } else { 
        decimalPlaces = this.countDecimals(step);
    }
    element.attr('step', step.toFixed(decimalPlaces));


 ... (clipped 4 lines)
Generic: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status:
Missing validation: New code parses numeric attributes (e.g., step, min, max) and handles multipliers without
explicit error handling for invalid or missing values, which may rely on upstream
guarantees not visible in the diff.

Referred Code
const multiObj = getUnitMultiplier();

const multiplier = multiObj.multiplier;
const unitName = multiObj.unitName;

let decimalPlaces = 0;
// Update the step, min, and max; as we have the multiplier here.
if (element.attr('type') == 'number') {
    let step = parseFloat(element.data("step")) || parseFloat(element.attr('step')) || 1;

    if (multiplier !== 1) { 
        decimalPlaces = Math.min(Math.ceil(multiplier / 100), 3);
        // Add extra decimal place for non-integer conversions.
        if (multiplier % 1 != 0 && decimalPlaces < 3) {
            decimalPlaces++;
        }
        step = 1 / Math.pow(10, decimalPlaces);
    } else { 
        decimalPlaces = this.countDecimals(step);
    }
    element.attr('step', step.toFixed(decimalPlaces));


 ... (clipped 4 lines)
Generic: Security-First Input Validation and Data Handling

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

Status:
Input step exposure: The new numeric input with data-step and type=number could accept user-provided values but
the diff does not show client-side validation beyond step/min/max and it is unclear
whether server-side validation is enforced.

Referred Code
<div class="pidTuning_number"><input id="fwLevelTrim" type="number" class="rate-tpa_input" data-setting="fw_level_pitch_trim" data-unit="deg" data-step="0.001" /></div>
<div for="fwLevelTrim" class="helpicon cf_tip" data-i18n_title="pidTuning_fw_level_pitch_trim_help"></div>
Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

Previous compliance checks

Compliance check up to commit c79c943
Security Compliance
Client-side validation bypass

Description: Reliance on client-provided data attributes (data-step/min/max) without strict validation
could allow crafted DOM manipulation to bypass expected numeric constraints in the UI,
potentially leading to unintended values being sent if server-side validation is weak.
settings.js [483-505]

Referred Code
const multiObj = getUnitMultiplier();

const multiplier = multiObj.multiplier;
const unitName = multiObj.unitName;

let decimalPlaces = 0;
// Update the step, min, and max; as we have the multiplier here.
if (element.attr('type') == 'number') {
    let step = parseFloat(element.data("step")) || parseFloat(element.attr('step')) || 1;

    if (multiplier !== 1) { 
        decimalPlaces = Math.min(Math.ceil(multiplier / 100), 3);
        // Add extra decimal place for non-integer conversions.
        if (multiplier % 1 != 0 && decimalPlaces < 3) {
            decimalPlaces++;
        }
        step = 1 / Math.pow(10, decimalPlaces);
    } else { 
        decimalPlaces = this.countDecimals(step);
    }
    element.attr('step', step.toFixed(decimalPlaces));


 ... (clipped 2 lines)
Ticket Compliance
🟡
🎫 #2420
🟢 Level Trim [deg] input should display decimal values and allow manual changes to 2 decimal
places.
🔴 Auto Level fine-tuning should be more obviously displayed, with change history/log of
level settings.
Allow assigning the fine-tuning value to a trim/slider pot to adjust +/- 0.00 to 0.25 in
0.05 increments.
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
🟢
Generic: Meaningful Naming and Self-Documenting Code

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status: Passed

Generic: Secure Error Handling

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

Status: Passed

Generic: Secure Logging Practices

Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.

Status: Passed

Generic: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status:
No audit logs: The new logic updating numeric inputs and unit conversions does not introduce or modify
any audit logging for critical actions, but it may not be applicable to front-end UI
changes.

Referred Code
const multiObj = getUnitMultiplier();

const multiplier = multiObj.multiplier;
const unitName = multiObj.unitName;

let decimalPlaces = 0;
// Update the step, min, and max; as we have the multiplier here.
if (element.attr('type') == 'number') {
    let step = parseFloat(element.data("step")) || parseFloat(element.attr('step')) || 1;

    if (multiplier !== 1) { 
        decimalPlaces = Math.min(Math.ceil(multiplier / 100), 3);
        // Add extra decimal place for non-integer conversions.
        if (multiplier % 1 != 0 && decimalPlaces < 3) {
            decimalPlaces++;
        }
        step = 1 / Math.pow(10, decimalPlaces);
    } else { 
        decimalPlaces = this.countDecimals(step);
    }
    element.attr('step', step.toFixed(decimalPlaces));


 ... (clipped 4 lines)
Generic: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status:
Missing validation: Newly added parsing of step, min, and max relies on attributes and data without explicit
error handling for malformed values or NaN cases in the updated number handling.

Referred Code
let step = parseFloat(element.data("step")) || parseFloat(element.attr('step')) || 1;

if (multiplier !== 1) { 
    decimalPlaces = Math.min(Math.ceil(multiplier / 100), 3);
    // Add extra decimal place for non-integer conversions.
    if (multiplier % 1 != 0 && decimalPlaces < 3) {
        decimalPlaces++;
    }
    step = 1 / Math.pow(10, decimalPlaces);
} else { 
    decimalPlaces = this.countDecimals(step);
}
element.attr('step', step.toFixed(decimalPlaces));

if (multiplier !== 'FAHREN' && multiplier !== 'TZHOURS' && multiplier !== 1) {
    element.data('default-min', element.attr('min'));
    element.data('default-max', element.attr('max'));
Generic: Security-First Input Validation and Data Handling

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

Status:
Input constraints: The new number input for fwLevelTrim adds data-step but does not add min/max or explicit
client-side validation, leaving edge validation to unseen code.

Referred Code
<div class="pidTuning_number"><input id="fwLevelTrim" type="number" class="rate-tpa_input" data-setting="fw_level_pitch_trim" data-unit="deg" data-step="0.001" /></div>
<div for="fwLevelTrim" class="helpicon cf_tip" data-i18n_title="pidTuning_fw_level_pitch_trim_help"></div>

@qodo-code-review
Copy link
Contributor

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
General
Improve decimal counting for floats

Refactor the countDecimals function to more robustly handle floating-point
numbers and avoid issues with imprecise string representations.

js/settings.js [600-613]

 self.countDecimals = function(value) {
-    let text = value.toString()
-    // verify if number 0.000005 is represented as "5e-6"
-    if (text.indexOf('e-') > -1) {
-      let [base, trail] = text.split('e-');
-      let decimals = parseInt(trail, 10);
-      return decimals;
-    }
-    // count decimals for number in representation like "0.123456"
-    if (Math.floor(value) !== value) {
-      return value.toString().split(".")[1].length || 0;
+    if ((value % 1) !== 0) {
+        const valueStr = String(value);
+        if (valueStr.indexOf('e-') > -1) {
+            const parts = valueStr.split('e-');
+            return parseInt(parts[1], 10);
+        }
+        const decimalPart = valueStr.split('.')[1];
+        return decimalPart ? decimalPart.length : 0;
     }
     return 0;
 }

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 6

__

Why: The suggestion correctly identifies a potential floating-point precision issue in countDecimals and proposes a more robust implementation, which improves the reliability of decimal place counting.

Low
  • More

@MrD-RC MrD-RC merged commit e8899d9 into master Nov 3, 2025
6 checks passed
@MrD-RC MrD-RC deleted the MrD_Fix-granularity-of-FW-level-trim branch November 3, 2025 07:44
@flashkiwi2025
Copy link

I noticed that the iNav OSD display for fw_level_pitch_trim currently only displays 1 decimal point. Will this fix also pass onto the OSD display automatically?

@MrD-RC
Copy link
Member Author

MrD-RC commented Nov 8, 2025

No, Configurator is completely separate to the firmware. When it was implemented. I guess 1/10th of a degree was considered fine enough adjustment. I'd tend to agree. Perhaps configurator should be changed to 0.1 degree steps to match.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Level Trim [deg] input & dispay increase to 2 decimal points

3 participants