Flight axis override test + idiot-proof mag alignment handling for FCs without a mag #2463
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
This commit addresses valid feedback from the PR iNavFlight#2462 bot review. **Fix #1: Test counter logic (test_override_regressions.js)** The failed counter was never incremented, causing misleading test reports. Changed from `passed += runTest(...)` to proper if/else logic that correctly tracks both passed and failed test counts. Before: Failed tests showed "Passed: 0, Failed: 0" After: Correctly shows "Passed: X, Failed: Y" All 14 tests pass with accurate counting. **Fix #2: Magnetometer improvements (magnetometer.js)** - Fixed typo: "settting" → "setting" (3 occurrences) - Improved promise handling: explicit `return Promise.resolve()` in null guard - Added error handling: `.catch()` blocks for better error logging - Changed console.log → console.warn for null data warnings These changes improve code clarity and error handling without changing functional behavior.
The test_flight_axis_override.js file had multiple bugs preventing tests from running correctly: **Fix #1: Missing Decompiler import** - Added `import { Decompiler } from '../decompiler.js';` - Changed `transpiler.decompile()` to proper `new Decompiler()` usage - Error was: "transpiler.decompile is not a function" **Fix #2: Invalid property name in Test 4** - Fixed `flight.armed` → `flight.isArmed` - Error was: "Unknown property 'armed' in 'flight.armed'" - The correct property is `isArmed` per the flight API **Fix #3: Error message handling** - Fixed `result.errors` → `result.error || result.errors` - Fixed `decompiled.errors` → `decompiled.error || decompiled.errors` - Handles both singular and plural error field names **Test Results:** Before: 3 decompile errors, 1 compilation failure After: All 4 tests pass ✅ - Test 1: Roll angle override ✅ - Test 2: Pitch rate override ✅ - Test 3: Yaw angle override ✅ - Test 4: Multiple axis overrides ✅
Branch Targeting SuggestionYou've targeted the
If This is an automated suggestion to help route contributions to the appropriate branch. |
Contributor
PR Compliance Guide 🔍All compliance sections have been disabled in the configurations. |
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
3 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
User description
Addresses bot comments
PR Type
Bug fix, Tests
Description
Fix test counter logic in regression tests to properly track passed/failed counts
Correct flight axis override test imports and API property references
Handle both singular and plural error field names in test assertions
Improve magnetometer promise handling and error logging with proper null guards
Diagram Walkthrough
File Walkthrough
test_flight_axis_override.js
Fix decompiler import and flight API property namesjs/transpiler/transpiler/tests/test_flight_axis_override.js
Decompilerimport from decompiler.js moduleflight.armedtoflight.isArmednew Decompiler()instantiation
erroranderrorsproperties
test_override_regressions.js
Fix test counter logic for accurate pass/fail trackingjs/transpiler/transpiler/tests/test_override_regressions.js
passed += runTest()pattern with proper if/else logicfor all 14 tests
passedandfailedcounters based on testresults
Failed: 0"
magnetometer.js
Improve magnetometer promise handling and error loggingtabs/magnetometer.js
messages
return Promise.resolve()in null data guards for properpromise chaining
.catch()error handlers for all three magnetometer alignmentsettings calls
console.logtoconsole.warnfor null data warnings and addedconsole.errorfor failures