inav-configurator icon indicating copy to clipboard operation
inav-configurator copied to clipboard

Published 20 hours ago • verified publisher icon iNavFlight

Removed some unnecessary preset parameters for FW

Open b14ckyy opened this issue 11 months ago • 7 comments

Removed some Preset values for Fixed wings due to FW changes that will be good on defaults. Comes along with https://github.com/iNavFlight/inav/pull/10541

b14ckyy avatar Dec 19 '24 10:12 b14ckyy

Maybe instead of removing these, just adjust the airplane without a tail values as bredoven recommendations? We can look into removing this later, with more time for testing.

mmosca avatar Dec 19 '24 10:12 mmosca

If the controller now controls velocity and not position, I think the difference in wing load and pitch authority should not make a difference between them anymore. What is breadoven's recommendation? Could not see that in the 2 threads I follow. If they still act differently then yes I can add them back in with slightly different values.

b14ckyy avatar Dec 19 '24 11:12 b14ckyy

If the controller now controls velocity and not position, I think the difference in wing load and pitch authority should not make a difference between them anymore. What is breadoven's recommendation? Could not see that in the 2 threads I follow. If they still act differently then yes I can add them back in with slightly different values.

I don't see any reason why the velocity altitude control would affect the xy position control so these values should be left unchanged.

breadoven avatar Dec 19 '24 11:12 breadoven

@breadoven I removed the xy presets in that go as well since I noticed that somewhere in the 7.0 or 7.1 phase the value for flying wings with no tail was too low. I planned to change/remove them for a while and just remembered so I removed them now. Especially I noticed with Path tracking and Autoland, that Wings struggle to get a precise heading fast enough on 55 while the Tail Planes worked much better. 75 or more on wings was severely better in tracking.

b14ckyy avatar Dec 19 '24 13:12 b14ckyy

I have restored the settings and just changed the defaults slightly based on the recommendations.

b14ckyy avatar Jan 18 '25 18:01 b14ckyy

This missed the train for 8.0. What is the concensus? Is it still needed?

mmosca avatar Jan 21 '25 17:01 mmosca

Not mandatory

b14ckyy avatar Jan 21 '25 18:01 b14ckyy

PR Compliance Guide 🔍

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
⚪
🎫 No ticket provided
  • [ ] Create ticket/issue
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 logging context: The added changes adjust default parameter values without introducing or modifying any
logging of critical actions, making it unclear whether related updates are audited.

Referred Code 
            {
    key: "nav_fw_pos_z_p",
    value: 35
},
{
    key: "nav_fw_pos_z_i",
    value: 5
},
{
    key: "nav_fw_pos_z_d",
    value: 10
},
{
    key: "nav_fw_pos_xy_p",
    value: 55
},
{
    key: "fw_turn_assist_pitch_gain",
    value: 0.4
},
{


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

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

Status:
No error handling: The changes only modify static default values without any validation or error handling
additions, so it is unclear whether edge cases for these parameters are validated
elsewhere.

Referred Code 
            {
    key: "nav_fw_pos_z_p",
    value: 35
},
{
    key: "nav_fw_pos_z_i",
    value: 5
},
{
    key: "nav_fw_pos_z_d",
    value: 10
},
{
    key: "nav_fw_pos_xy_p",
    value: 55
},
{
    key: "fw_turn_assist_pitch_gain",
    value: 0.4
},
{


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

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

Status:
Validation unknown: Updating default numeric parameters does not include input validation or constraints here,
leaving uncertainty about safeguards against invalid or unsafe values elsewhere.

Referred Code 
            {
    key: "nav_fw_pos_z_p",
    value: 35
},
{
    key: "nav_fw_pos_z_i",
    value: 5
},
{
    key: "nav_fw_pos_z_d",
    value: 10
},
{
    key: "nav_fw_pos_xy_p",
    value: 55
},
{
    key: "fw_turn_assist_pitch_gain",
    value: 0.4
},
{


 ... (clipped 203 lines)
Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

qodo-code-review[bot] avatar Oct 31 '25 05:10 qodo-code-review[bot]