Resolved missing DEBUG modes #4636
Conversation
WalkthroughThe PR updates src/js/debug.js by adding new debug modes and corresponding field name mappings, renaming/removing several existing mappings (including DUAL_GYRO_* to MULTI_GYRO_* and GPS_RESCUE_THROTTLE_PID), introducing new FFT/autopilot/optical flow/TPA entries, removing GYRO_SCALED, and extending the public enableFields list with Attitude and Servo. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/js/debug.js (1)
756-760: Bug: malformed key "debug[all" in SPAMissing closing bracket breaks the “all” label.
Apply this diff:
- SPA: { - "debug[all": "SPA", + SPA: { + "debug[all]": "SPA",
🧹 Nitpick comments (1)
src/js/debug.js (1)
851-854: Avoid duplicating CHIRP/FLASH_TEST_PRBS in modesThese entries are already present in the base
modesarray; adding them again risks duplicates ifaddArrayElementisn’t de-duping.Apply this diff to remove the redundant additions:
- addArrayElement(DEBUG.modes, "CHIRP"); - addArrayElement(DEBUG.modes, "FLASH_TEST_PRBS");If you prefer keeping them gated by API 1.47+, instead remove their base entries (Lines 104–106) and keep these two lines.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/js/debug.js(2 hunks)
🔇 Additional comments (8)
src/js/debug.js (8)
861-868: Good: safe DUAL→MULTI aliasing orderYou map
MULTI_*first, then deleteDUAL_*, preserving references. LGTM.Does firmware still expose
DUAL_GYRO_COMBINED? If renamed, consider providingMULTI_GYRO_COMBINEDalias too.
869-879: FFT_FREQ indices changed—confirm against firmwareIndices now place “Gyro Pre Dyn Notch” at [0] and add Notch 1–7 at [1]–[7]. Verify ordering matches API 1.47 telemetry to avoid mislabeled plots.
881-891: AUTOPILOT_ALTITUDE labels look consistentClear P/I/D/F and altitude fields; nice replacement for
GPS_RESCUE_THROTTLE_PID.
893-901: TPA: expanded labels OKOverriding older single-field TPA with richer 1.47 mapping is fine.
903-911: OPTICALFLOW mapping addedLooks good; consider clarifying units for “Delta time”.
913-923: AUTOPILOT_POSITION mapping addedLabels are clear and aligned with PID semantics.
925-927: enableFields additions: scope and idempotency
- “Attitude” is added only for 1.47+, but
ATTITUDElabels are defined for 1.46+; confirm this split is intentional.- Ensure
addArrayElementAfter/addArrayElementare de-duplicating to prevent repeated entries across multipleupdate()calls.
794-800: FLASH_TEST_PRBS gaps in indicesYou define [0], [1], [6], [7] only. If the UI expects contiguous indices for plotting, this may hide series [2]–[5]. Confirm firmware doesn’t output those, or add placeholders.
| "DEBUG_AUTOPILOT", | ||
| "CHIRP", | ||
| "FLASH_TEST_PRBS", | ||
| ], |
There was a problem hiding this comment.
🧩 Analysis chain
New modes added: missing labels for DEBUG_AUTOPILOT
DEBUG_AUTOPILOT has no fieldNames mapping, so the UI will show generic labels or break grouping.
Add a mapping (place alongside other 1.47+ mappings):
// e.g., after AUTOPILOT_POSITION in the 1.47 block
DEBUG.fieldNames.DEBUG_AUTOPILOT = {
"debug[all]": "Autopilot",
"debug[0]": "State",
"debug[1]": "Error",
"debug[2]": "P",
"debug[3]": "I",
"debug[4]": "D",
"debug[5]": "F",
"debug[6]": "Setpoint",
"debug[7]": "Measurement",
};Run to confirm coverage for the new modes:
🏁 Script executed:
#!/bin/bash
# Verify fieldNames exist for new modes
rg -n 'DEBUG_AUTOPILOT|CHIRP|FLASH_TEST_PRBS' src/js/debug.js
rg -n 'fieldNames\.(DEBUG_AUTOPILOT|CHIRP|FLASH_TEST_PRBS)|fieldNames\["(DEBUG_AUTOPILOT|CHIRP|FLASH_TEST_PRBS)"\]' src/js/debug.jsLength of output: 246
Add missing DEBUG_AUTOPILOT fieldNames mapping
DEBUG_AUTOPILOT is listed under DEBUG.modes but has no corresponding DEBUG.fieldNames.DEBUG_AUTOPILOT block, so UI labels will fall back to generic values. Add it alongside the other 1.47+ mappings, for example:
// after AUTOPILOT_POSITION in the 1.47 block
DEBUG.fieldNames.DEBUG_AUTOPILOT = {
"debug[all]": "Autopilot",
"debug[0]": "State",
"debug[1]": "Error",
"debug[2]": "P",
"debug[3]": "I",
"debug[4]": "D",
"debug[5]": "F",
"debug[6]": "Setpoint",
"debug[7]": "Measurement",
};🤖 Prompt for AI Agents
In src/js/debug.js around lines 104 to 107, DEBUG.modes includes
"DEBUG_AUTOPILOT" but there is no corresponding DEBUG.fieldNames.DEBUG_AUTOPILOT
mapping, so UI labels fall back to generic values; add a new
DEBUG.fieldNames.DEBUG_AUTOPILOT block in the 1.47+ mappings (place it after
AUTOPILOT_POSITION) with keys "debug[all]", "debug[0]" through "debug[7]" and
values "Autopilot", "State", "Error", "P", "I", "D", "F", "Setpoint", and
"Measurement" respectively to match other mode mappings.
|
@haslinghuis |
|
|
Preview URL: https://pr4636.betaflight-app.pages.dev |
|
:) Yes, this is very interested! |
|
@demvlad check the update function. There you can sniff the values more reliable. |
Sorry, i've forgot. For the new versions only:) |
2 participants
Resolved missing DEBUG modes:
"DEBUG_AUTOPILOT",
"CHIRP",
"FLASH_TEST_PRBS"
Summary by CodeRabbit
New Features
Changes