Skip to content

Plot 8 columns of debug data #631

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

Merged
merged 1 commit into from
Mar 17, 2023

Conversation

bw1129
Copy link
Contributor

@bw1129 bw1129 commented Mar 3, 2023

This PR allows Blackbox explorer to properly plot up to 8 columns of debug data (see also PR #12445). The PR makes changes to js/flightlog_fields_presenter.js to accommodate the additional 4 columns. The PR follows the current convention for unused columns, so it is backward compatible with current debug. Thus, adding new columns to existing debug variables is easy and would involve only minor changes to this file for future work. Additionally, this PR updates 'DSHOT_RPM_TELEMETRY' debug to plot RPM data for up to 8 motors (see screenshot below), in accordance with changes made in PR #12445.
Screenshot 2023-02-28 at 7 03 36 PM

@sonarqubecloud
Copy link

sonarqubecloud bot commented Mar 3, 2023

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
85.4% 85.4% Duplication

@bw1129 bw1129 changed the title Update flightlog_fields_presenter.js Plot 8 columns of debug data Mar 3, 2023
@nerdCopter
Copy link
Member

Ignore the SonarCloud fail. it reported 85% duplication due to all the new debug inits.

@github-actions
Copy link

github-actions bot commented Mar 3, 2023

Do you want to test this code? Here you have an automated build:
Betaflight-Blackbox-Explorer-Linux
Betaflight-Blackbox-Explorer-macOS
Betaflight-Blackbox-Explorer-Windows
WARNING: It may be unstable and result in corrupted configurations or data loss. Use only for testing!

@blckmn
Copy link
Member

blckmn commented Mar 3, 2023

AUTOMERGE: (FAIL)

  • github identifies PR as mergeable -> FAIL
  • assigned to a milestone -> FAIL
  • cooling off period lapsed -> PASS
  • commit count less or equal to three -> PASS
  • Don't merge label NOT found -> PASS
  • at least one RN: label found -> PASS
  • Tested label found -> FAIL
  • assigned to an approver -> FAIL
  • approver count at least three -> FAIL

@bw1129
Copy link
Contributor Author

bw1129 commented Mar 11, 2023

@KarateBrot @haslinghuis @blckmn since PR betaflight/betaflight#12445 has been merged, and is dependent upon this one, be great if you're able to review this one

@KarateBrot
Copy link
Member

KarateBrot commented Mar 11, 2023

We could think about utilizing the 8 debug channels more efficiently by also allowing 2 sets of debugs with 4 channels each in the firmware (instead of hardcoding 8 channel names into the blackbox-log-viewer for all debugs).

@bw1129
Copy link
Contributor Author

bw1129 commented Mar 11, 2023

@KarateBrot I think 2 independent debugs sounds fantastic, but it is undoubtedly way more complicated and is beyond the scope of what the current PR is about, but I may not be appreciating what you are thinking in how this could be implemented elegantly. For example, if you wanted two independent 4 column debugs, an advantage would be that you could log GYRO_SCALED at the same time as say DSHOT_RPM_TELEMETRY, which would be fantastic indeed. Thus, I imagine there would be 2 dropdowns in the configurator, and the list of possible debug variables would be duplicated for debug dropdown 1 and debug dropdown 2 (and you could select any combination). But then it is not clear how you would elegantly deal with cases where you want RPM for all 8 motors in an x8 lifter for example (i.e., you'd want both dropdowns to have the same list of debug choices, like DSHOT_RPM_TELEMETRY, but in this case you'd need something like "DSHOT_RPM_TELEMETRY_MOTORS1-4" for dropdown 1, and "DSHOT_RPM_TELEMETRY_MOTORS5-8" for dropdown 2. I am probably not appreciating how you are imagining this, but as I said, the coding will definitely be more complex and really is beyond the scope of the current PR. The simplcity if the current PR opens a ton of possibilities without having to go down that rabbit hole (but please correct me if I am wrong).

@haslinghuis haslinghuis merged commit 3dfa39b into betaflight:master Mar 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: COMPLETED
Development

Successfully merging this pull request may close these issues.

5 participants