Maintenance 9.x #2547
Merged
Maintenance 9.x #2547
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Decompiler now outputs whenChanged() to match the API definition - Use OPERAND_TYPE.LC from constants instead of hardcoded value - Add test for whenChanged() decompilation - Add test for OPERAND_TYPE.LC usage in handleNot()
…ilds The postPackage hook was using the wrong path for macOS app bundles, causing it to fail to find and remove non-native SITL binaries. Problem: - macOS DMG packages included Windows SITL binaries (cygwin1.dll, inav_SITL.exe) - These should have been removed by the postPackage hook - Hook was looking at outputPath/resources/sitl (incorrect for macOS) - Actual macOS location: <app>/Contents/Resources/sitl Root Cause: - macOS app bundles have structure: INAV Configurator.app/Contents/Resources/ - Windows/Linux packages: <outputPath>/resources/ - Hook used same path for all platforms (outputPath/resources/sitl) Fix: - Use different path construction based on platform: - macOS (darwin): <outputPath>/Contents/Resources/sitl - Windows/Linux: <outputPath>/resources/sitl - Added console.log statements for debugging build process Impact: - macOS DMG will now correctly contain only darwin SITL binaries - Reduces package size (no unnecessary Windows/Linux binaries) - Consistent with Windows/Linux builds which already work correctly Testing: - Requires macOS build to verify DMG no longer contains windows/ directory - Logic verified for all three platforms (darwin, win32, linux) - Console logging added to aid verification 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
The previous fix used the wrong path for macOS app bundles. It was missing the .app directory name in the path construction. Problem: - Was looking for: <outputPath>/Contents/Resources/sitl - Actual location: <outputPath>/<AppName>.app/Contents/Resources/sitl Fix: - Dynamically find the .app bundle in outputPath - Construct correct path with .app directory included This should now properly remove non-native SITL binaries from macOS builds.
This addresses two issues identified by Qodo bot code review: Issue #3: Conflict detection indexing bug - Previously only ifthen handlers got unique keys - All on.always() blocks collapsed to single key, causing: * False positives: assignments in different blocks flagged as conflicts * Disabled race detection: check requires multiple handlers but all collapsed to 1 - Fix: Give every handler a unique index regardless of type - Location: js/transpiler/transpiler/analyzer.js:528-530 Issue #5: Raw values not wrapped in Literal nodes - >= and <= optimization passed raw numbers instead of AST nodes - Fix: Wrap constValue±1 in {type: 'Literal', value: ...} objects - Maintains AST consistency throughout pipeline - Location: js/transpiler/transpiler/condition_generator.js:300,307 All 27 transpiler test suites pass with 0 failures.
…d-naming Fix decompiler to output whenChanged() instead of delta()
Fix transpiler conflict detection and AST consistency
This commit fixes three issues in the JavaScript Programming tab:
1. **Fix silent code dropping for invalid function calls**
- Previously, code like `inav.override.flightAxis(180);` was silently
discarded without any error message
- Now generates clear error: "Cannot call 'X' as a function"
- Added extractCalleeNameForError() helper to show full call path
- Location: js/transpiler/transpiler/parser.js
2. **Add comprehensive intermediate object detection**
- Detects when users try to use objects as if they were properties
- Catches: assignments, function calls, expressions, comparisons
- Examples caught:
* `inav.override.flightAxis.yaw = 180` (missing .angle)
* `gvar[0] = inav.flight + 1` (flight is an object)
* `inav.override.vtx(3)` (vtx is object, not function)
- Provides helpful suggestions showing available properties
- Locations:
* js/transpiler/transpiler/property_access_checker.js
* js/transpiler/transpiler/analyzer.js
3. **Fix missing i18n text in JavaScript Programming tab header**
- Added i18n.localize() calls in tab initialization
- Added CSS to fix paragraph spacing (margin-bottom: 10px)
- Locations:
* tabs/javascript_programming.js (lines 84, 91)
* tabs/javascript_programming.html (line 217-219)
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
- Changed addError to addWarning with type 'error' in parser - Made transpile() fail when parser errors are present - Prevents silent code dropping from succeeding compilation
This commit completes the transpiler validation improvements by: 1. Enhanced intermediate object detection (property_access_checker.js): - Now uses raw API definitions instead of processed structure - Simplified logic per user suggestion: just check typeof === 'object' - Catches 2-level objects (e.g., inav.override.vtx) - Catches 3-level objects (e.g., inav.override.flightAxis.yaw) - Catches category-only access (e.g., inav.flight) 2. Improved error messages (analyzer.js): - Shows available properties for intermediate objects - Differentiates simple vs deeply nested objects - Provides actionable suggestions 3. Comprehensive test coverage: - test_examples.mjs: All 22 examples still compile (no regressions) - test_validation_fixes.mjs: 8/8 validation tests pass All invalid code now generates helpful error messages instead of: - Being silently dropped - Failing later in codegen with cryptic messages - Passing through to generate invalid logic conditions 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
Address Qodo code review feedback: check result.warnings.errors instead of result.errors for semantic/validation errors.
Set tpa_rate = 80 for airplane presets ("Airplane with a Tail" and
"Airplane without a Tail"). This addresses feedback from PR #11222
that fixed-wing aircraft benefit from higher TPA values.
Multicopter presets retain their existing tpa_rate = 20 setting.
…y key Prevents generating error suggestions containing 'undefined' when nested.properties is an empty object. Addresses qodo-merge-pro bot suggestion on PR #2514. Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
The plotElevation() function was commented out in commit 8d5870b due to ESM module compatibility issues. This fix: - Adds Chart.js v4.4.1 with full ESM support to package.json - Imports Chart.js properly as ES module - Uncomments the plotElevation() function This restores the terrain elevation profile chart feature for mission planning safety checks.
The plotElevation() function calls `new Chart()` which expects Chart to be available in the global scope. While we correctly imported Chart.js as an ES module, it was only available within the module scope. This change exposes Chart on the window object so the elevation plotting function can access it.
The plotElevation() function had Chart.js code only for empty missions (dummy data). When missions had waypoints, it prepared elevation data but only had a commented-out Plotly.newPlot() call, so no chart rendered. Changes: - Add Chart.js rendering after elevation data preparation (line 4352+) - Convert Plotly trace data to Chart.js datasets format - Display WGS84 terrain elevation (orange, filled area) - Display mission altitude (blue line with waypoint markers) - Properly destroy existing chart before creating new one - Store chart instance globally for cleanup on subsequent calls Both empty and non-empty mission cases now create Chart.js charts.
Code review identified several issues that needed addressing: CRITICAL fixes: - Replace hardcoded test data in empty mission case with proper empty chart - Add canvas element validation before Chart instantiation - Remove all unused Plotly code structures (trace objects, layout vars) - Fix chart title to not depend on removed Plotly code IMPROVEMENTS: - Add try-catch error handling for async elevation data fetch - Add safe array checks for Math.min/max to prevent empty array errors - Change var to const for immutable values (modern JavaScript) - Clear chart instance reference (set to null) after destruction - Add console error messages for debugging The plotElevation() function is now a clean Chart.js-only implementation without any Plotly remnants, proper error handling, and validation.
…ctivity When waypoints are dragged, the elevation chart should update smoothly. The previous approach of destroying and recreating the chart on every update was causing the chart to not refresh when waypoints were moved. Chart.js update() method is designed for this use case: - Preserves the chart instance - Updates data and options - Triggers smooth re-render with animations - More efficient than destroy/recreate This fixes the issue where dragging a waypoint didn't update the elevation profile.
…cted Previously, plotElevation() was only called if the dragged waypoint matched selectedMarker. This meant dragging an unselected waypoint wouldn't update the chart. Now plotElevation() is called for all waypoint drags: - If waypoint is selected: updates elevation + runs full async logic - If waypoint is not selected: updates elevation chart only This ensures the elevation profile always reflects the current mission geometry after any waypoint is moved.
Per code review feedback, adds two important improvements:
1. Race condition protection:
- Track elevation update sequence number
- Ignore stale data from late-returning API calls
- Prevents chart showing outdated elevation when waypoints dragged rapidly
2. Disable chart animations during updates:
- Use chart.update('none') instead of chart.update()
- Improves performance during drag operations
- Prevents animation queueing and visual lag
These fixes ensure the chart always shows current data and responds
smoothly even with rapid waypoint dragging or slow network connections.
readme: Add debugging info
Update ru translation
…aries Fix postPackage hook to remove non-native SITL binaries from macOS builds
Fix transpiler validation and improve error messages
…aults Add TPA rate defaults to fixed-wing presets
…rt-esm Fix: Re-enable terrain elevation chart with Chart.js ESM support
- Linux x64 SITL: built locally with glibc 2.34 compatibility - Linux arm64, macOS, Windows SITL: from nightly v9.0.0-20260114.184 - Firmware commit: a7932b92eadd92ba7851a009a5c974b9cc24835d - All binaries match 9.0.0 final release candidate
removed old pre 9.0 values I missed
correcting vspeed defaults in presets
Remove 'set' prefix from nav_fw keys
Fix formatting of OSD element document
Update SITL binaries for 9.0.0 final release
In 8.0.0, addOnReceiveListener() called addOnReceiveCallback() which registered callbacks in both _onReceiveListeners and the BLE-specific _onCharateristicValueChangedListeners array. In 9.0.0 this was removed to "avoid duplicate push", but Serial's addOnReceiveCallback() pushes to _onReceiveListeners while BLE's pushes to its own separate array. This meant the byte counter listener (added via addOnReceiveListener) only went into _onReceiveListeners, but BLE's notification handler only called _onCharateristicValueChangedListeners - so the counter never incremented and MSP responses were never seen by the parser. Fix: align BLE with Serial by using _onReceiveListeners throughout, removing the now-unnecessary _onCharateristicValueChangedListeners array. Includes debug logging to aid diagnosis if further issues arise.
User confirmed BLE connection is working correctly after the byte counter fix, but reported slower performance compared to 8.0.1. Analysis showed 4 console.log() statements in the data path: - 2 logs on every BLE notification received (hot path) - 2 logs during listener add/remove (cold path) The hot path logs were causing performance degradation as they execute on every 20-byte BLE packet received. Removed all 4 debug logs as they were marked TODO for removal after testing. The core fix (using _onReceiveListeners array) remains intact.
Fix BLE byte counter regression introduced in 9.0.0
Branch Targeting SuggestionYou've targeted the
If This is an automated suggestion to help route contributions to the appropriate branch. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Bring master up to date with maintenance-9.x just in case someone tries to use it.