Skip to content

Conversation

@sensei-hacker
Copy link
Member

@sensei-hacker sensei-hacker commented Jan 12, 2026

User description

Summary

Re-enables the terrain elevation chart feature in Mission Control that was disabled in December 2024 due to ESM module compatibility issues during the Vite migration.

Yarn rebuilt the yarn.lock file, reorganizing everything, when I added a dependency. Is that expected and correct?

Root Cause

The plotElevation() function was commented out in commit 8d5870b0ae because the charting libraries (Chart.js/Plotly.js) were not compatible with the new ESM module system used by Vite.

Changes

  • Added Chart.js v4.4.1 with full ESM support to package.json
  • Imported Chart.js as ES module with proper registration of components
  • Uncommented and refactored plotElevation() function in tabs/mission_control.js
  • Removed all unused Plotly.js code structures
  • Added proper error handling and canvas validation
  • Fixed chart lifecycle management (destroy/recreate pattern)

Technical Details

  • Chart.js v4.4.1 provides native ESM support compatible with Vite
  • Global window.Chart exposure ensures compatibility with existing code patterns
  • Chart instance stored globally for proper cleanup on subsequent calls
  • Safe array operations to prevent errors with empty datasets

Testing

  • Built configurator successfully with yarn start
  • Connected to flight controller in Demo mode
  • Created mission with multiple waypoints at varying altitudes
  • Clicked mountain icon to display elevation profile
  • Verified chart displays:
    • ✅ WGS84 terrain elevation (orange filled area)
    • ✅ Mission altitude with waypoint markers (blue line)
    • ✅ Proper axis labels and scaling
    • ✅ Chart updates when waypoints change
  • Tested empty mission case - displays blank chart appropriately
  • No JavaScript errors in console
  • Verified existing Mission Control functionality unchanged

Code Review

Reviewed with inav-code-review agent. All critical and important issues addressed:

  • Removed hardcoded test data
  • Added canvas validation
  • Cleaned up unused Plotly remnants
  • Added error handling
  • Improved code quality with const declarations

Impact

Restores mission planning safety feature that shows terrain clearance along flight paths, helping prevent ground collisions.

Fixes terrain elevation data not loading issue.


PR Type

Bug fix, Enhancement


Description

This description is generated by an AI tool. It may have inaccuracies

  • Re-enables terrain elevation chart with Chart.js ESM support

    • Added Chart.js v4.4.1 dependency with proper ESM module registration
    • Uncommented and refactored plotElevation() function with improved error handling
    • Implemented chart lifecycle management with destroy/recreate pattern
  • Enhances JavaScript transpiler validation to catch previously silent errors

    • Detects invalid function calls and intermediate object usage
    • Provides helpful error messages with available property suggestions
    • Prevents code from silently dropping during compilation
  • Improves error reporting in JavaScript Programming tab

    • Adds i18n localization for tab header text
    • Fixes CSS spacing issues in note sections
    • Updates beta warning message
  • Adds regression test scripts for validation and examples


Diagram Walkthrough

flowchart LR
  A["Chart.js v4.4.1<br/>ESM Module"] -->|"Import & Register"| B["mission_control.js<br/>plotElevation()"]
  B -->|"Render Chart"| C["Elevation Profile<br/>Visualization"]
  D["JavaScript Code"] -->|"Parse & Analyze"| E["Enhanced Transpiler<br/>Validation"]
  E -->|"Detect Errors"| F["Helpful Error<br/>Messages"]
  F -->|"Prevent Silent<br/>Failures"| G["Improved Code<br/>Quality"]
Loading

File Walkthrough

Relevant files
Dependencies
1 files
package.json
Add Chart.js v4.4.1 dependency                                                     
+1/-0     
Bug fix
2 files
mission_control.js
Re-enable elevation chart with Chart.js                                   
+159/-107
index.js
Fail transpilation on parser errors                                           
+12/-1   
Enhancement
4 files
parser.js
Detect invalid function calls with error messages               
+37/-0   
property_access_checker.js
Detect intermediate objects and provide suggestions           
+126/-6 
analyzer.js
Generate improved writability error messages                         
+113/-3 
javascript_programming.js
Add i18n localization for tab header                                         
+5/-0     
Formatting
1 files
javascript_programming.html
Fix CSS spacing in note sections                                                 
+4/-0     
Documentation
1 files
messages.json
Update JavaScript beta warning message                                     
+1/-1     
Tests
2 files
test_validation_fixes.mjs
Add regression tests for validation fixes                               
+117/-0 
test_examples.mjs
Add regression tests for example compilation                         
+61/-0   

@qodo-code-review
Copy link
Contributor

qodo-code-review bot commented Jan 12, 2026

PR Compliance Guide 🔍

All compliance sections have been disabled in the configurations.

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.
@sensei-hacker sensei-hacker force-pushed the fix/terrain-elevation-chart-esm branch from a0e1950 to bef73fc Compare January 12, 2026 19:58
@sensei-hacker sensei-hacker added this to the 9.0 milestone Jan 12, 2026
@sensei-hacker sensei-hacker merged commit aed1893 into iNavFlight:maintenance-9.x Jan 13, 2026
6 checks passed
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.

1 participant