Skip to content

Conversation

haslinghuis
Copy link
Member

@haslinghuis haslinghuis commented Sep 28, 2025

For 2025.12.0

  • deprecates gyro_to_use
  • introduces gyro_enabled_bitmask

Summary by CodeRabbit

  • New Features

    • Added “Gyro enable mask” field (8-bit) in Hardware Options; displays decimal with binary mask, auto-populates from imported logs, and preserves raw value on save.
  • UI

    • Reordered gyro settings so “Hardware gyro LPF” now appears after “Gyro enable mask” for clearer layout.
    • Bitmask shown read-only with descriptive tooltip when available.

@haslinghuis haslinghuis added this to the 2025.12.0 milestone Sep 28, 2025
@haslinghuis haslinghuis self-assigned this Sep 28, 2025
Copy link

coderabbitai bot commented Sep 28, 2025

Walkthrough

Adds a new gyro_enabled_bitmask sysConfig field parsed from log headers, renders it in the header dialog via a new bitmask helper, and exposes a new UI cell in the HARDWARE OPTIONS row (inserted before the existing gyro_hardware_lpf). Also includes parser numeric/coercion refactors and firmware-version conditional adjustments.

Changes

Cohort / File(s) Summary
UI: Hardware options layout
index.html
Adds a new <td name="gyro_enabled_bitmask"> containing label "Gyro enable mask" and an input; moves existing <td name="gyro_hardware_lpf"> to follow the new cell (DOM reordering only).
Parser: SysConfig extension & header parsing + numeric coercions
src/flightlog_parser.js
Adds defaultSysConfigExtension.gyro_enabled_bitmask; recognizes header key "gyro_enabled_bitmask" and assigns parsed integer to sysConfig; replaces several `(value
Dialog rendering: Bitmask helper, rendering & save handling, version gating
src/header_dialog.js
Adds helper setBitmaskParameter(name, data, totalBits) to render bitmask values (numeric + binary), integrate into renderSysConfig for gyro_enabled_bitmask, ensure saving uses raw numeric values for bitmask fields (including array elements), update parameter version entries, and relax/adjust several firmware-version conditional thresholds and some DOM insertion calls.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant LH as Log Header
  participant FLP as flightlog_parser.js
  participant SC as sysConfig
  participant HD as header_dialog.js
  participant UI as index.html (DOM)

  LH->>FLP: header["gyro_enabled_bitmask"] = "42"
  FLP->>SC: sysConfig.gyro_enabled_bitmask = parseInt("42")
  note right of SC #E8F5E9: Stored as numeric bitmask

  HD->>SC: read sysConfig.gyro_enabled_bitmask
  HD->>HD: setBitmaskParameter("gyro_enabled_bitmask", 42, 8)
  HD->>UI: populate `td[name="gyro_enabled_bitmask"]` with "42 (0b00101010)"
  UI-->>User: displays "Gyro enable mask" before "Hardware gyro LPF"
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

  • RC filter update #852 — touches header dialog/version gating and UI/sysconfig parameter handling similar to this change.

Suggested reviewers

  • ctzsnooze
  • nerdCopter

Pre-merge checks and finishing touches

❌ Failed checks (3 warnings)
Check name Status Explanation Resolution
Title Check ⚠️ Warning The title states "Replace gyro_to_use with gyro_enabled_bitmask in header dialog" but the changeset shows that gyro_enabled_bitmask is being added as a new field across multiple files (parser, UI, and header dialog) without removing gyro_to_use. The word "replace" implies substitution, but the code changes demonstrate addition and extension of functionality. While the title references a real aspect of the change (introducing gyro_enabled_bitmask), it mischaracterizes the nature of the change and is overly narrow in scope by focusing only on the header dialog when changes affect the parser and core configuration structures as well. Consider revising the title to accurately reflect that gyro_enabled_bitmask is being added rather than replacing gyro_to_use. A more accurate title might be "Add gyro_enabled_bitmask parameter support" or "Introduce gyro_enabled_bitmask for 2025.12.0" which better captures the additive nature of the change without implying removal of existing functionality.
Description Check ⚠️ Warning The pull request description contains only two brief bullet points stating that it deprecates gyro_to_use and introduces gyro_enabled_bitmask for version 2025.12.0. The template requires discussion of coding style adherence, pull request scope, commit structure, readiness for review, CI considerations, and issue references where applicable. The provided description is largely incomplete as it lacks context about what the new parameter does, why the change is necessary, backward compatibility considerations, and does not address any of the template's required sections. Expand the description to include: an explanation of what gyro_enabled_bitmask does and how it differs from gyro_to_use, the rationale for the change, backward compatibility notes, confirmation that coding style guidelines were followed, and verification that CI builds are expected to pass. If this fixes any open issues, add the issue references using the "Fixes #issue_number" format.
Docstring Coverage ⚠️ Warning Docstring coverage is 27.27% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share
🧪 Early access (Sonnet 4.5): enabled

We are currently testing the Sonnet 4.5 model, which is expected to improve code review quality. However, this model may lead to increased noise levels in the review comments. Please disable the early access features if the noise level causes any inconvenience.

Note:

  • Public repositories are always opted into early access features.
  • You can enable or disable early access features from the CodeRabbit UI or by updating the CodeRabbit configuration file.

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 334ae08 and 6579188.

📒 Files selected for processing (3)
  • index.html (1 hunks)
  • src/flightlog_parser.js (2 hunks)
  • src/header_dialog.js (3 hunks)
🔇 Additional comments (1)
index.html (1)

1519-1522: Placement and structure LGTM.

The new gyro_enabled_bitmask field slots neatly into the hardware options row and matches the surrounding markup, so I’ve got no concerns here.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6579188 and 64686f5.

📒 Files selected for processing (1)
  • src/header_dialog.js (5 hunks)

coderabbitai[bot]
coderabbitai bot previously approved these changes Sep 28, 2025
@haslinghuis haslinghuis changed the title Add gyro_enabled_bitmask to header dialog Replace gyro_to_use with gyro_enabled_bitmask in header dialog Sep 28, 2025
Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
9.4% Duplication on New Code (required ≤ 3%)
B Maintainability Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

Copy link

Preview URL: https://pr862.betaflight-blackbox.pages.dev

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 71b5878 and 49356c6.

📒 Files selected for processing (3)
  • index.html (3 hunks)
  • src/flightlog_parser.js (10 hunks)
  • src/header_dialog.js (7 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • index.html
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-24T23:23:01.244Z
Learnt from: haslinghuis
PR: betaflight/blackbox-log-viewer#859
File: src/flightlog_fields_presenter.js:1315-1325
Timestamp: 2025-09-24T23:23:01.244Z
Learning: In the blackbox-log-viewer codebase, firmware version pre-release identifiers (like "-rc1", "-rc2") are stripped from the `firmwareVersion` property, so Betaflight 4.5.0-RC1 would be exposed as just "4.5.0" for version comparison purposes.

Applied to files:

  • src/flightlog_parser.js
🪛 Biome (2.1.2)
src/flightlog_parser.js

[error] 845-846: This case is falling through to the next case.

Add a break or return statement to the end of this case to prevent fallthrough.

(lint/suspicious/noFallthroughSwitchClause)

🔇 Additional comments (15)
src/flightlog_parser.js (9)

302-302: LGTM!

The addition of gyro_enabled_bitmask is properly initialized and follows the established pattern for configuration fields.


536-536: LGTM!

Simplifying the undefined check is appropriate here and more idiomatic.


747-747: LGTM!

The switch case properly handles the new gyro_enabled_bitmask header field using the same integer parsing pattern as similar fields.


826-826: LGTM!

The division simplifications (removing .0 suffix) are appropriate in JavaScript, where all numbers are floating-point by default.

Also applies to: 840-840, 846-846


987-987: LGTM!

The fallback assignment for unknown firmware versions is appropriate.


1088-1089: LGTM!

Using Math.max and Math.min is more idiomatic than manual comparisons for tracking field statistics.


1447-1447: LGTM!

Replacing bitwise | 0 coercion with Math.trunc improves readability and makes the intent explicit.

Also applies to: 1456-1456


1676-1676: LGTM!

Division simplifications are consistent with JavaScript's floating-point number handling.

Also applies to: 1681-1682


844-847: Intentional case fallthrough is correct here.

The static analysis tool flags the fallthrough from motor_idle to digitalIdleOffset, but this is intentional as both parameters share identical parsing logic (integer value divided by 100). This is a common and acceptable pattern for consolidating duplicate case logic.

src/header_dialog.js (6)

876-880: LGTM!

Minor formatting adjustment with no functional impact.


882-908: LGTM!

The setBitmaskParameter helper properly handles bitmask display and storage. The implementation correctly:

  • Clears all state when data is null (preventing stale data issues)
  • Stores the raw integer value for round-trip saving
  • Makes the field readonly to prevent editing of the formatted display
  • Provides user-friendly binary representation with tooltip

539-549: Verify the firmware version ranges for the gyro parameter transition.

The PR deprecates gyro_to_use (max: "4.5.9") and introduces gyro_enabled_bitmask (min: "2025.12.0"). Please confirm:

  1. Is there a firmware version gap between 4.5.9 and 2025.12.0 where neither parameter would display?
  2. Should gyro_to_use have a max closer to "2025.11.9" to ensure continuity?

Based on learnings: firmware pre-release identifiers are stripped, but the large version jump suggests a possible versioning scheme change that should be verified.

Based on learnings.


1974-1974: LGTM!

The setBitmaskParameter call is properly integrated into the rendering flow alongside the related gyro_to_use parameter.


2035-2038: LGTM!

The raw-value data attribute handling correctly enables round-trip saving of bitmask parameters, preventing the formatted display string from being coerced to 0/1. The implementation properly handles both array and scalar parameters.

Also applies to: 2046-2048


1750-1750: LGTM!

The DOM manipulation change from insertBefore() to before() is appropriate—both achieve the same result, with before() being the more modern jQuery syntax.

@haslinghuis haslinghuis merged commit 4f60b5e into betaflight:master Sep 29, 2025
4 of 5 checks passed
@haslinghuis haslinghuis deleted the gyro_enabled_bitmask branch September 29, 2025 20:47
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.

2 participants