-
Notifications
You must be signed in to change notification settings - Fork 373
Ipf trig #2478
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: maintenance-10.x
Are you sure you want to change the base?
Ipf trig #2478
Conversation
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
magnetometer.js error logging
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.
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
Branch Targeting SuggestionYou've targeted the
If This is an automated suggestion to help route contributions to the appropriate branch. |
PR Compliance Guide 🔍All compliance sections have been disabled in the configurations. |
| abs: { | ||
| type: 'function', | ||
| desc: 'Return absolute value', | ||
| params: ['value'], | ||
| returns: 'number', | ||
| inavOperation: 32 // OPERATION.ABS | ||
| returns: 'number' | ||
| }, |
There was a problem hiding this comment.
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]
| 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 | |
| }, |
| 57: { | ||
| name: "Trigonometry: ACos", | ||
| operandType: "Maths", | ||
| hasOperand: [true, true], | ||
| output: "raw" | ||
| }, | ||
| 58: { | ||
| name: "Trigonometry: ASin", | ||
| operandType: "Maths", | ||
| hasOperand: [true, true], | ||
| output: "raw" | ||
| }, |
There was a problem hiding this comment.
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]
| 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" | |
| }, |
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
File Walkthrough
5 files
Add arc trig functions and refactor operation constantsImplement code generation for arc trigonometric functionsDefine new logic operators for arc trigonometryUpdate diagnostics to support arc trigonometric functionsAdd operation constants for arc trigonometric functions1 files
Add TypeScript type declarations for arc functions3 files