-
-
Notifications
You must be signed in to change notification settings - Fork 987
Include buildKey in tracking information #4561
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- this is to help identify last use of a build key for storage analysis
WalkthroughThe Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant serial_backend.js
Caller->>serial_backend.js: processUid()
serial_backend.js->>serial_backend.js: processBuildConfiguration() (await)
serial_backend.js->>serial_backend.js: sendConfigTracking() (await, includes buildKey)
Estimated code review effort🎯 2 (Simple) | ⏱️ ~7 minutes Possibly related PRs
Suggested reviewers
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ 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. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
|
Preview URL: https://41c46135.betaflight-configurator.pages.dev |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/js/serial_backend.js (1)
95-95
: Fix trailing comma formatting issue.The addition of
buildKey
to the tracking payload aligns perfectly with the PR objectives to assist in storage analysis. However, there's a missing trailing comma that should be addressed for consistency.- buildKey: FC.CONFIG.buildKey + buildKey: FC.CONFIG.buildKey,
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/js/serial_backend.js
(2 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: DavidAnson
PR: betaflight/betaflight-configurator#4450
File: src/js/tabs/setup.js:0-0
Timestamp: 2025-05-01T05:41:03.282Z
Learning: In the Betaflight Configurator, FC.CONFIG.buildKey is defaulted to an empty string when not present, making the length check (FC.CONFIG.buildKey.length === 32) safe without additional type checking.
Learnt from: mituritsyn
PR: betaflight/betaflight-configurator#4526
File: src/js/gui.js:43-43
Timestamp: 2025-06-20T12:35:49.283Z
Learning: In the Betaflight Configurator codebase, tabs in `defaultCloudBuildTabOptions` are conditionally displayed based on firmware build options. The logic in `serial_backend.js` checks `FC.CONFIG.buildOptions` and only adds tabs to `GUI.allowedTabs` if the firmware was built with support for that feature.
📚 Learning: in the betaflight configurator codebase, tabs in `defaultcloudbuildtaboptions` are conditionally dis...
Learnt from: mituritsyn
PR: betaflight/betaflight-configurator#4526
File: src/js/gui.js:43-43
Timestamp: 2025-06-20T12:35:49.283Z
Learning: In the Betaflight Configurator codebase, tabs in `defaultCloudBuildTabOptions` are conditionally displayed based on firmware build options. The logic in `serial_backend.js` checks `FC.CONFIG.buildOptions` and only adds tabs to `GUI.allowedTabs` if the firmware was built with support for that feature.
Applied to files:
src/js/serial_backend.js
📚 Learning: in the betaflight configurator, fc.config.buildkey is defaulted to an empty string when not present,...
Learnt from: DavidAnson
PR: betaflight/betaflight-configurator#4450
File: src/js/tabs/setup.js:0-0
Timestamp: 2025-05-01T05:41:03.282Z
Learning: In the Betaflight Configurator, FC.CONFIG.buildKey is defaulted to an empty string when not present, making the length check (FC.CONFIG.buildKey.length === 32) safe without additional type checking.
Applied to files:
src/js/serial_backend.js
🧬 Code Graph Analysis (1)
src/js/serial_backend.js (1)
src/js/fc.js (1)
FC
(130-993)
🪛 ESLint
src/js/serial_backend.js
[error] 95-96: Missing trailing comma.
(comma-dangle)
🔇 Additional comments (1)
src/js/serial_backend.js (1)
533-533
: Sequencing Confirmed: buildKey Populated Before Tracking
processBuildConfiguration()
issues an MSP2_GET_TEXT BUILD_KEY request (in serial_backend.js) and setsFC.CONFIG.buildKey
in MSPHelper.js (line 920). Placingawait sendConfigTracking()
immediately afterward guarantees the tracking payload carries the correctbuildKey
. LGTM.
Summary by CodeRabbit