RC filter update #852
RC filter update #852
Conversation
WalkthroughAdds id attributes to two table cells in index.html and updates header_dialog.js to change the max firmware for rc_smoothing_active_cutoffs_ff and to gate visibility/initialization of feedforward-related controls by BetaFlight firmware version (different handling for >=4.6.0, >=4.3.0, and older). Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant HeaderDialog
participant renderSysConfig
participant DOM
User->>HeaderDialog: Open header dialog
HeaderDialog->>renderSysConfig: Render with sysConfig + firmwareType/version
renderSysConfig->>renderSysConfig: If firmwareType == BetaFlight
alt firmware >= 4.6.0
renderSysConfig->>DOM: Hide rcSmoothingFeedforwardHz, rcSmoothingActiveCutoffsFf
renderSysConfig->>DOM: Init rcSmoothingActiveCutoffsSp <- rc_smoothing_active_cutoffs_ff_sp_thr[0]
renderSysConfig->>DOM: Init rcSmoothingActiveCutoffsThr <- rc_smoothing_active_cutoffs_ff_sp_thr[1]
else if firmware >= 4.3.0
renderSysConfig->>DOM: Set rcSmoothingFeedforwardHz <- rc_smoothing_feedforward_hz
renderSysConfig->>DOM: Set rcSmoothingActiveCutoffsFf <- rc_smoothing_active_cutoffs_ff_sp_thr[0]
renderSysConfig->>DOM: Set rcSmoothingActiveCutoffsSp <- rc_smoothing_active_cutoffs_ff_sp_thr[1]
renderSysConfig->>DOM: Set rcSmoothingActiveCutoffsThr <- rc_smoothing_active_cutoffs_ff_sp_thr[2]
else
renderSysConfig->>DOM: Legacy handling (unchanged)
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
index.html(1 hunks)src/header_dialog.js(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/header_dialog.js (3)
src/graph_spectrum.js (1)
sysConfig(30-30)src/graph_config.js (1)
sysConfig(232-232)src/flightlog.js (1)
sysConfig(1438-1438)
🔇 Additional comments (1)
index.html (1)
1950-1953: LGTM: explicit id enables precise gating from JS.Adding id="rcSmoothingFeedforwardHz" is appropriate for the new visibility logic in header_dialog.js.
|
|
Preview URL: https://e1715b39.betaflight-blackbox.pages.dev |
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/header_dialog.js (1)
1575-1588: Guard against missing rc_smoothing_active_cutoffs_ff_sp_thr in BF 4.6+ logs (prevent TypeError).In the 4.6.0+ branch you index sysConfig.rc_smoothing_active_cutoffs_ff_sp_thr[0/1]. If BF 4.6 swapped the key to rc_smoothing_active_cutoffs_sp_thr (FF removed), this will be undefined and indexing will throw, breaking renderSysConfig. Use optional chaining and a fallback to the SP/THR-only key.
Apply this diff to make the block robust:
// RC Smoothing if (activeSysConfig.firmwareType === FIRMWARE_TYPE_BETAFLIGHT) { - if (semver.gte(activeSysConfig.firmwareVersion, "4.6.0")) { - $("#rcSmoothingFeedforwardHz").hide(); - $("#rcSmoothingActiveCutoffsFf").hide(); - setParameter("rcSmoothingActiveCutoffsSp", sysConfig.rc_smoothing_active_cutoffs_ff_sp_thr[0], 0); - setParameter("rcSmoothingActiveCutoffsThr", sysConfig.rc_smoothing_active_cutoffs_ff_sp_thr[1], 0); - } else if (semver.gte(activeSysConfig.firmwareVersion, "4.3.0")) { - setParameter("rcSmoothingFeedforwardHz", sysConfig.rc_smoothing_feedforward_hz, 0); - setParameter("rcSmoothingActiveCutoffsFf", sysConfig.rc_smoothing_active_cutoffs_ff_sp_thr[0], 0); - setParameter("rcSmoothingActiveCutoffsSp", sysConfig.rc_smoothing_active_cutoffs_ff_sp_thr[1], 0); - setParameter("rcSmoothingActiveCutoffsThr", sysConfig.rc_smoothing_active_cutoffs_ff_sp_thr[2], 0); - } + // Normalize 4.3–4.5 (FF+SP+THR) vs 4.6+ (SP+THR only) + const ffSpThr = sysConfig.rc_smoothing_active_cutoffs_ff_sp_thr; + const spThr = sysConfig.rc_smoothing_active_cutoffs_sp_thr; + if (semver.gte(activeSysConfig.firmwareVersion, "4.6.0")) { + $("#rcSmoothingFeedforwardHz").hide(); + $("#rcSmoothingActiveCutoffsFf").hide(); + const arr = Array.isArray(spThr) ? spThr : ffSpThr; // fallback if logs still use old key + setParameter("rcSmoothingActiveCutoffsSp", arr?.[0], 0); + setParameter("rcSmoothingActiveCutoffsThr", arr?.[1], 0); + } else if (semver.gte(activeSysConfig.firmwareVersion, "4.3.0")) { + setParameter("rcSmoothingFeedforwardHz", sysConfig.rc_smoothing_feedforward_hz, 0); + setParameter("rcSmoothingActiveCutoffsFf", ffSpThr?.[0], 0); + setParameter("rcSmoothingActiveCutoffsSp", ffSpThr?.[1], 0); + setParameter("rcSmoothingActiveCutoffsThr", ffSpThr?.[2], 0); + } }
🧹 Nitpick comments (1)
src/header_dialog.js (1)
623-627: Capping rc_smoothing_active_cutoffs_ff to 4.5.999 is correct; consider aligning related caps and normalizing name matching.
- Good update: BF 4.6+ removed FF active cutoff, so max "4.5.999" is appropriate.
- Optional: For consistency, also cap "rc_smoothing_feedforward_hz" at "4.5.999" in parameterVersion. You’re already hiding it in UI, but codifying the cap improves discoverability and reduces reliance on ad-hoc DOM gating.
- Optional: isParameterValid compares parameterVersion snake_case to DOM/caller camelCase (e.g., rcSmoothingActiveCutoffsFf), which makes the caps ineffective for UI-driven visibility. Consider normalizing names in isParameterValid (camelCase → snake_case) once, to make version gating universally reliable.
If you want the canonical fix for isParameterValid, I can prepare a follow-up patch that adds a tiny normalizeName helper and applies it inside isParameterValid.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
index.html(2 hunks)src/header_dialog.js(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- index.html
|
@haslinghuis - this definitely works. Happy for you to merge it when you are ready. Thank you! |
2 participants
Before:
After:
Log from 4.5:
Summary by CodeRabbit
Bug Fixes
Chores