Skip to content

Conversation

@xznhj8129
Copy link

@xznhj8129 xznhj8129 commented Dec 14, 2025

User description

Add support for arc function IPF LCs (see iNavFlight/inav#11179)


PR Type

Enhancement, New Feature


Description

  • Add support for inverse trigonometric functions (acos, asin, atan2)

  • Replace hardcoded operation numbers with named constants for maintainability

  • Update error messages and diagnostics to reflect new supported functions

  • Define new logic operators 57, 58, 59 for arc trigonometry operations


Diagram Walkthrough

flowchart LR
  A["New Arc Trig Functions"] -->|acos, asin, atan2| B["Expression Generator"]
  B -->|Generate Logic Commands| C["Logic Operators 57-59"]
  C -->|Map to Operations| D["INAV Constants"]
  A -->|Update Type Definitions| E["TypeScript Declarations"]
  B -->|Update Diagnostics| F["Editor Validation"]
Loading

File Walkthrough

Relevant files
Enhancement
5 files
helpers.js
Add arc trig functions and refactor operation constants   
+42/-14 
expression_generator.js
Implement code generation for arc trigonometric functions
+26/-3   
logicConditionOperators.js
Define new logic operators for arc trigonometry                   
+19/-1   
diagnostics.js
Update diagnostics to support arc trigonometric functions
+3/-3     
inav_constants.js
Add operation constants for arc trigonometric functions   
+7/-0     
Documentation
1 files
types.js
Add TypeScript type declarations for arc functions             
+3/-0     
Additional files
3 files
inav_SITL [link]   
inav_SITL [link]   
inav_SITL [link]   

sensei-hacker and others added 30 commits November 30, 2025 02:34
This commit fixes issues in the transpiler API definitions where
the configurator did not match the actual INAV firmware implementation.

Issues Fixed:

1. waypoint.js (CRITICAL):
   - Fixed wrong operand type (was 5/GVAR, now 7/WAYPOINTS)
   - Removed 6 fabricated properties not in firmware (latitude, longitude,
     altitude, bearing, missionReached, missionValid)
   - Added 14 actual properties from firmware (isWaypointMission, number,
     action, nextAction, distance, distanceFromPrevious, user action flags)

2. gvar.js (HIGH):
   - Fixed wrong operand type (was 3/FLIGHT_MODE, now 5/GVAR)
   - Fixed wrong operation (was 19/GVAR_INC, now 18/GVAR_SET)

3. pid.js (MAJOR):
   - Removed fabricated properties not in firmware (setpoint, measurement,
     P/I/D/FF gains, enabled)
   - Kept only 'output' property which is actually exposed (operands 0-3)
   - Changed from i*10+0..7 mapping to direct i mapping

4. flight.js (MEDIUM):
   - Added missing wind parameters 46-49:
     minGroundSpeed, horizontalWindSpeed, windDirection, relativeWindOffset

5. inav_constants.js (MEDIUM):
   - Added missing FLIGHT_PARAM constants 46-49
   - Added missing OPERATION constant 56 (OVERRIDE_MIN_GROUND_SPEED)

6. codegen.js (LOW):
   - Fixed RC channel regex to support both rc[N] and rc[N].value syntax

Testing:
- Added 5 comprehensive test files (test_rc_channels, test_gvar, test_pid,
  test_waypoint, test_flight)
- All fixes verified against firmware source code:
  - src/main/programming/logic_condition.h
  - src/main/programming/logic_condition.c
  - src/main/programming/global_variables.h
  - src/main/programming/pid.h

Files verified correct (no changes needed):
- override.js (all 10 operations match firmware)
- rc.js (correctly handled by codegen)

Documentation:
- API_BUGS_FOUND.md: Initial issue analysis
- VERIFICATION_SUMMARY.md: Complete verification findings
- FIXES_COMPLETE.md: Final fix summary with cross-references

Breaking Changes:
- pid[N].configure(), pid[N].setpoint, pid[N].enabled no longer available
  (these never existed in firmware, only pid[N].output works)
- waypoint.latitude/longitude/altitude/bearing no longer available
  (these are not exposed through logic condition system)

Firmware References:
- logic_condition.h lines 92-102 (operand types)
- logic_condition.h lines 104-155 (flight parameters)
- logic_condition.h lines 177-192 (waypoint parameters)
- logic_condition.c lines 575-669 (waypoint implementation)
- logic_condition.c lines 1078-1082 (PID implementation)
Updated test documentation to reflect fixed state and improve code quality:

1. test_flight.js:
   - Removed outdated 'KNOWN ISSUE' header (params 46-49 are now present)
   - Updated to use FLIGHT_PARAM_NAMES constant instead of Object.keys().find()
   - Cleaner and more performant parameter name lookup

2. test_pid.js:
   - Replaced 'KNOWN BUG TO DETECT' with 'FIRMWARE DESIGN' explanation
   - Clarified that firmware intentionally exposes only PID outputs
   - Removed 'bug' language as this is intentional firmware design

These changes address feedback from PR review bot.
…fixes

Fix transpiler API definitions to match firmware implementation
…etch_to_esm

search.js: Update fetch to ESM import syntax
Removed gvar.js and override.js as they are not used by the transpiler.

Analysis shows:
- gvar.js: Completely bypassed by hardcoded gvar handling in codegen.js
  (lines 598-610) and action_generator.js (lines 98, 108, 118, 133, 141)
- override.js: Bypassed by hardcoded operation mapping in codegen.js
  (lines 705-719)

Both files contained only documentation that duplicated information already
hardcoded in the transpiler implementation. Removing them reduces maintenance
burden and eliminates confusion from misleading values (gvar.js had wrong
type and operation values that were never actually used).

Files modified:
- Deleted: js/transpiler/api/definitions/gvar.js
- Deleted: js/transpiler/api/definitions/override.js
- Updated: js/transpiler/api/definitions/index.js (removed imports/exports)
The previous commit removed override.js but forgot to update the import
statement in js/transpiler/index.js, causing build failures:
  "Could not resolve ./api/definitions/override.js"

Changes:
- Removed import of overrideDefinitions from index.js (line 23)
- Removed overrideDefinitions from export list (line 40)

Build verified successful with npm run make.
 JS Programming: Fix tab loading + Monaco editor  import
…pi-definitions

Remove unused API definition files
When saving a JavaScript program to the flight controller, previously-
occupied logic condition slots that are not part of the new transpiled
script are now properly cleared.

Problem:
- User has 20 logic conditions on FC (from previous save)
- User writes JavaScript that transpiles to 10 logic conditions
- Saves to FC
- BUG: FC had conditions 0-9 (new) PLUS 10-19 (old, stale data)

Root Cause:
saveToFC() only sent the new conditions to the FC, leaving old
conditions in their previously-occupied slots.

Solution:
1. Track which slots were occupied when loading from FC
2. When saving, identify slots that were occupied but are NOT in the
   new transpiled script
3. Send disabled/empty conditions for those slots to clear them

Changes:
- tabs/javascript_programming.js (onLogicConditionsLoaded):
  Track previously occupied slots in self.previouslyOccupiedSlots
- tabs/javascript_programming.js (saveToFC):
  Add empty conditions for previously-occupied slots not in new script

Testing:
- Build successful (npm run make)
- Configurator starts without errors
- Manual testing recommended: Create 15 conditions via Programming tab,
  then save 5-condition JavaScript program and verify only 5 remain

This fix prevents stale logic conditions from remaining active on the
flight controller, which could cause unexpected behavior during flight.
Bug iNavFlight#1: Update GPS example to use 'gpsSats' property
- The API property was renamed from 'gpsNumSat' to 'gpsSats'
- GPS Fix Check example was not updated, causing transpilation errors
- Fixed examples/index.js lines 120, 124

Bug iNavFlight#2: Fix waypoint example to use 'distance' property
- Example incorrectly used 'waypoint.distanceToHome'
- Correct property is 'waypoint.distance' (distance to current waypoint)
- Fixed examples/index.js lines 185, 189

Bug iNavFlight#3: Add null checks in property_access_checker.js
- Missing null check caused crash: "Cannot read properties of undefined (reading 'targets')"
- Affected "Altitude-based Stages" and other override examples
- Added defensive checks for apiObj, apiObj.targets, apiObj.nested
- Fixed property_access_checker.js lines 170-181

Fixes:
- "GPS Fix Check" example now transpiles successfully
- "Waypoint Arrival Detection" example now transpiles successfully
- "Altitude-based Stages" example now transpiles successfully
- All 15 examples validated

Users can now successfully use all built-in examples without errors.
The previous implementation used put() to append conditions sequentially,
which didn't correctly map array indices to FC slot numbers. This caused
empty conditions to be sent to the wrong slots when clearing stale data.

Fixed by:
- Building a Map of slot index -> condition from transpiler output
- Adding empty conditions for previously-occupied slots to the map
- Sorting by slot index before adding to FC.LOGIC_CONDITIONS array
- This ensures array position matches FC slot number for sendLogicConditions()

The sendLogicConditions() function uses array index as the FC slot number
(line 2469 in MSPHelper.js), so maintaining correct array indices is critical.

Addresses code review feedback from Qodo bot on PR iNavFlight#2452.
When code uses both direct (<, >, ==) and synthesized (>=, <=, !=)
comparison operators with the same operands, the transpiler was creating
duplicate conditions instead of reusing existing ones.

Example:
  if (x < 6) { ... }   // Creates condition 0: x < 6
  if (x >= 6) { ... }  // Was creating duplicate condition 2: x < 6
                       // Should reuse condition 0

Root cause:
The condition cache had separate namespaces for 'binary' (direct ops)
and 'binary_synth' (synthesized ops). When generating >= as NOT(x < 6),
the code didn't check if 'x < 6' already existed in the binary cache.

Fix:
In generateBinary(), when processing synthesized operators (>=, <=, !=),
check if the inverse comparison already exists in the cache before
generating a new one. If found, reuse the existing LC index.

Changes:
- js/transpiler/transpiler/condition_generator.js: Added cache lookup
  for inverse comparison before generating new condition
- Added comprehensive tests for duplicate detection

Testing:
- test_not_precedence.js: Verifies >= reuses existing <
- test_duplicate_detection_comprehensive.js: Tests >=, <=, != reuse
- All tests pass

Impact: Reduces wasted logic condition slots, prevents FC slot exhaustion
Fixes two issues reported by Scavanger on PR iNavFlight#2455:

1. types.js was referencing apiDefinitions.override which no longer
   exists after override.js was removed in commit 2ed9320
   - Removed line 44: generateInterfaceFromDefinition('override', ...)

2. tabs/javascript_programming.js was using editorWorker() without
   importing it
   - Added missing import on line 21

Both issues were oversights from the API definition cleanup.
…clear-unused-conditions

Fix JavaScript Programming: Clear unused logic conditions on save
…examples-bugs

Fix three bugs breaking JavaScript Programming examples
…not-operator-precedence

Fix duplicate condition bug in transpiler for synthesized operators
Resolves multiple bugs in the JavaScript Programming tab mentioned in iNavFlight#2453

1. IntelliSense contamination with browser APIs
   - Configured Monaco editor to use ES2020 lib only
   - Excludes DOM/browser APIs (navigator, document, etc.)
   - Preserves Math and standard JavaScript features

2. Override property validation errors
   - Restored override.js API definitions
   - Updated to use OPERATION constants from inav_constants.js
   - Enables proper IntelliSense for override.vtx.power, etc.

3. Unsaved changes warning behavior

Implementation details:
- monaco_loader.js: Added lib: ['es2020'] to compiler options
- override.js: Restored from git history, updated with OPERATION constants
- serial_backend.js: Check isDirty before disconnect, clear flag after confirm
- configurator_main.js: Check isDirty before tab switch
- javascript_programming.js: Simplified cleanup() to only dispose editor

Testing:
- IntelliSense no longer suggests browser APIs
- Override properties validate correctly
- Unsaved changes warning shows exactly once
- Cancel properly prevents disconnect/tab switch
- Subsequent actions work correctly after cancel

Files changed:
- js/transpiler/editor/monaco_loader.js
- js/transpiler/api/definitions/override.js (restored)
- js/transpiler/api/definitions/index.js
- js/transpiler/api/types.js
- js/serial_backend.js
- js/configurator_main.js
- tabs/javascript_programming.js
…programming-issues

Fix JavaScript Programming tab issues (iNavFlight#2453)
Implements full compiler and decompiler support for flight axis angle/rate overrides.

Syntax:
  override.flightAxis.{roll|pitch|yaw}.{angle|rate} = value

INAV operations:
- FLIGHT_AXIS_ANGLE_OVERRIDE (45): angle in degrees
- FLIGHT_AXIS_RATE_OVERRIDE (46): rate in deg/s

Changes:
- Added override.js with flightAxis API definitions
- Made TypeScript generation recursive for nested objects
- Encode axis (0=roll, 1=pitch, 2=yaw) in operandA
- Removed decompiler warnings
- Added missing operations: osdLayout, loiterRadius, minGroundSpeed

Testing:
- 130+ tests pass (116 existing + 14 new regression tests)
- No regressions found
Co-authored-by: qodo-code-review[bot] <151058649+qodo-code-review[bot]@users.noreply.github.com>
This changelog file was meant for internal tracking during development
and should not be included in the pull request.
sensei-hacker and others added 5 commits December 6, 2025 13:50
Moved FLIGHT_AXIS_OVERRIDE_IMPLEMENTATION.md and REGRESSION_TEST_SUMMARY.md
to claude/projects/flight-axis-override-implementation/ as these are
internal development documentation and should not be in the PR.
…rride-implementation

Add flight axis override support to JavaScript Programming
….0-RC3

Update SITL binaries for 9.0.0-RC3
@github-actions
Copy link

Branch Targeting Suggestion

You've targeted the master branch with this PR. Please consider if a version branch might be more appropriate:

  • maintenance-9.x - If your change is backward-compatible and won't create compatibility issues between INAV firmware and Configurator 9.x versions. This will allow your PR to be included in the next 9.x release.

  • maintenance-10.x - If your change introduces compatibility requirements between firmware and configurator that would break 9.x compatibility. This is for PRs which will be included in INAV 10.x

If master is the correct target for this change, no action is needed.


This is an automated suggestion to help route contributions to the appropriate branch.

@xznhj8129 xznhj8129 changed the title Ipf trig IPF acos/asin/atan2 Dec 14, 2025
@qodo-code-review qodo-code-review bot changed the title IPF acos/asin/atan2 Ipf trig Dec 14, 2025
@qodo-code-review
Copy link
Contributor

PR Compliance Guide 🔍

All compliance sections have been disabled in the configurations.

Comment on lines 32 to 37
abs: {
type: 'function',
desc: 'Return absolute value',
params: ['value'],
returns: 'number',
inavOperation: 32 // OPERATION.ABS
returns: 'number'
},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: Restore the inavOperation: OPERATION.ABS property for the abs function definition, as it was likely removed by mistake and is necessary for correct transpilation. [possible issue, importance: 9]

Suggested change
abs: {
type: 'function',
desc: 'Return absolute value',
params: ['value'],
returns: 'number',
inavOperation: 32 // OPERATION.ABS
returns: 'number'
},
abs: {
type: 'function',
desc: 'Return absolute value',
params: ['value'],
returns: 'number',
inavOperation: OPERATION.ABS
},

Comment on lines +264 to +275
57: {
name: "Trigonometry: ACos",
operandType: "Maths",
hasOperand: [true, true],
output: "raw"
},
58: {
name: "Trigonometry: ASin",
operandType: "Maths",
hasOperand: [true, true],
output: "raw"
},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: Update the hasOperand property for ACOS and ASIN in js/logicConditionOperators.js from [true, true] to [true, false] to correctly reflect that they are single-argument functions. [general, importance: 6]

Suggested change
57: {
name: "Trigonometry: ACos",
operandType: "Maths",
hasOperand: [true, true],
output: "raw"
},
58: {
name: "Trigonometry: ASin",
operandType: "Maths",
hasOperand: [true, true],
output: "raw"
},
57: {
name: "Trigonometry: ACos",
operandType: "Maths",
hasOperand: [true, false],
output: "raw"
},
58: {
name: "Trigonometry: ASin",
operandType: "Maths",
hasOperand: [true, false],
output: "raw"
},

@xznhj8129 xznhj8129 changed the base branch from master to maintenance-10.x January 28, 2026 17:07
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.

3 participants