Skip to content

Conversation

@Patronics
Copy link
Contributor

@Patronics Patronics commented Nov 25, 2025

The default configuration originally graphed the roll setpoint twice, rather than the pitch and roll setpoint and gyro values paired together as intended.

Summary by CodeRabbit

  • Bug Fixes
    • Updated graph labels to display specific titles (Roll, Pitch) instead of generic labels for improved clarity.
    • Corrected command field references to ensure proper data alignment across graph channels.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link

coderabbitai bot commented Nov 25, 2025

Walkthrough

Updates to a configuration file with graph label replacements from "Custom graph" to specific axis titles ("Roll", "Pitch", "Kiss Roll"/"Kiss Pitch") and corrected field references from rcCommands[0] to rcCommands[1] in one graph definition.

Changes

Cohort / File(s) Summary
Graph Configuration Labels
src/ws_supafly.json
Updated graph display labels from generic "Custom graph" to specific axis names and corrected command channel reference for the second graph

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Verify label changes accurately reflect intended axis mappings (Roll, Pitch, Kiss Roll, Kiss Pitch)
  • Confirm rcCommands[1] reference is the correct channel for the updated graph position
  • Ensure JSON syntax and structure remain valid after string replacements

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the main change: correcting an error in the Supafly/GyroSetpoint default configuration related to duplicate roll setpoint graphing.
Description check ✅ Passed The PR description explains the issue (roll setpoint graphed twice instead of pitch/roll paired with gyro), but does not follow the required template structure or include context about PR guidelines.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent 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 a3253ec and 5499ccf.

📒 Files selected for processing (1)
  • src/ws_supafly.json (2 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: ctzsnooze
Repo: betaflight/blackbox-log-viewer PR: 851
File: src/flightlog_fields_presenter.js:0-0
Timestamp: 2025-08-14T02:50:17.745Z
Learning: In the Betaflight blackbox log viewer, PT1K debug fields should use 0.xxx format with /1000 scaling for proper chart display, as confirmed by ctzsnooze for the FEEDFORWARD and FEEDFORWARD_LIMIT debug modes.
📚 Learning: 2025-05-26T16:18:25.863Z
Learnt from: demvlad
Repo: betaflight/blackbox-log-viewer PR: 833
File: src/graph_spectrum.js:0-0
Timestamp: 2025-05-26T16:18:25.863Z
Learning: In src/graph_spectrum_calc.js, the user created dedicated PSD functions `dataLoadPowerSpectralDensityVsThrottle()` and `dataLoadPowerSpectralDensityVsRpm()` instead of using boolean parameters, following a consistent naming pattern for spectrum data loading methods.

Applied to files:

  • src/ws_supafly.json
🔇 Additional comments (1)
src/ws_supafly.json (1)

858-858: Correctly fixes the duplicate roll setpoint issue in GyroSetpoint graph.

The changes properly pair gyro and setpoint data:

  • Line 858: First graph labeled "Roll" with gyroADC[0] + rcCommands[0] (index 0)
  • Line 891: Second graph now correctly references rcCommands[1] instead of the duplicate rcCommands[0]
  • Line 896: Second graph labeled "Pitch" with gyroADC[1] + rcCommands[1] (index 1)

This resolves the reported bug where the roll setpoint was being graphed twice. The axis indexing now matches the pattern used elsewhere in the file (e.g., lines 40 & 78 in the "Kiss Step" section), ensuring consistent configuration across presets.

Also applies to: 891-891, 896-896

Tip

📝 Customizable high-level summaries are now available in beta!

You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.

  • Provide your own instructions using the high_level_summary_instructions setting.
  • Format the summary however you like (bullet lists, tables, multi-section layouts, contributor stats, etc.).
  • Use high_level_summary_in_walkthrough to move the summary from the description to the walkthrough section.

Example instruction:

"Divide the high-level summary into five sections:

  1. 📝 Description — Summarize the main change in 50–60 words, explaining what was done.
  2. 📓 References — List relevant issues, discussions, documentation, or related PRs.
  3. 📦 Dependencies & Requirements — Mention any new/updated dependencies, environment variable changes, or configuration updates.
  4. 📊 Contributor Summary — Include a Markdown table showing contributions:
    | Contributor | Lines Added | Lines Removed | Files Changed |
  5. ✔️ Additional Notes — Add any extra reviewer context.
    Keep each section concise (under 200 words) and use bullet or numbered lists for clarity."

Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later.


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

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

@sonarqubecloud
Copy link

@github-actions
Copy link

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

@SupaflyFPV
Copy link

not sure what's going on here as this isn't my template setup...

@nerdCopter nerdCopter self-requested a review November 25, 2025 21:46
@Patronics
Copy link
Contributor Author

Patronics commented Nov 25, 2025

not sure what's going on here as this isn't my template setup...

Was it mislabeled when originally added in commit 248003e? This is the current version available by default in the blackbox log viewer under the name "SupaflyFPV", when you select a default workspace with shift+w. The only new change in this pull request is swapping a single item which was graphed, and adding 2 new labels to better describe the graphs.

@Patronics
Copy link
Contributor Author

Patronics commented Nov 25, 2025

@SupaflyFPV at a quick glance at least, it does seem to match the config file you uploaded here: #734 (comment)

Edit: confirmed by git diff --no-index -w, the file is identical to the one you uploaded aside from whitespace (which was changed by autoformatters in commit a01caf5, and the 3 lines of fixes I've implemented.

@nerdCopter
Copy link
Member

@demvlad ^^

@Patronics
Copy link
Contributor Author

Patronics commented Nov 25, 2025

@SupaflyFPV I think I figured out the source of confusion: When you first open the preview URL linked above, it may not default to opening your template, but instead defaults to the other template from ctzsnooze. The changes I made are to your presets, which you can access by pressing shift+W, then selecting the SupaflyFPV option from the dropdown.

@demvlad
Copy link
Contributor

demvlad commented Nov 26, 2025

@Patronics
I used some shared JSON file while was doing this setup.
Thank's for improvement.

@haslinghuis haslinghuis merged commit 7de89d7 into betaflight:master Nov 26, 2025
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants