Skip to content
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

Debug 8 columns #12445

Merged
merged 5 commits into from Mar 10, 2023
Merged

Debug 8 columns #12445

merged 5 commits into from Mar 10, 2023

Conversation

bw1129
Copy link
Contributor

@bw1129 bw1129 commented Mar 3, 2023

This PR adds an additional 4 columns to the blackbox debug variable for a total of 8 useable columns of data (see associated Blackbox Explorer PR betaflight/blackbox-log-viewer#631). This is extremely useful for logging such things as RPM across all 8 motors of an x8 cinelifter for example, which is very useful for troubleshooting a potentially unhealthy motor or ESC before bad things happen (see attached screenshot). There are many other instances where > 4 columns of debug data would be very useful (Eg, to track the movement of all 5 DNs, etc). The PR involves minor changes to blackbox.c and debug.h. The PR also includes a small change to dshot.c to allow logging of erpm data for up to 8 motors.
Screenshot 2023-02-28 at 7 03 36 PM

-added 4 additional columns to debug
-modified dshot.c to allow rpm telemetry to log erpm for up to 8 motors
@github-actions

This comment has been minimized.

@blckmn
Copy link
Member

blckmn commented Mar 3, 2023

AUTOMERGE: (FAIL)

  • github identifies PR as mergeable -> FAIL
  • assigned to a milestone -> PASS
  • cooling off period lapsed -> PASS
  • commit count less or equal to three -> FAIL
  • 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

@SteveCEvans
Copy link
Member

If would be great if we could use this either for a single debug mode with eight channels or two of the existing modes. Perhaps have a debug_mode2 to control it? Being able to look at GYRO_SCALED and D_MIN at the same time would be great, for example.

@bw1129
Copy link
Contributor Author

bw1129 commented Mar 3, 2023

@SteveCEvans yea, I like that. I think a case could be made for both, but it would require a lot more extensive changes to have 2 separate debugs, which would also require changes to the configurator BB logging tab

Copy link
Member

@KarateBrot KarateBrot left a comment

Choose a reason for hiding this comment

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

This makes sure we don't write to nonexistent debug channels in case DEBUG16_VALUE_COUNT is smaller than the number of max supported motors. Right now it's both set to 8 and in theory this works perfectly. But if someone changes the number of debug channels to 7 for example and flies with 8 motors, the FC would freeze and we wouldn't know where the bug comes from (not without debugging).

src/main/drivers/dshot.c Outdated Show resolved Hide resolved
src/main/drivers/dshot.c Outdated Show resolved Hide resolved
Co-authored-by: Jan Post <Rm2k-Freak@web.de>
@bw1129
Copy link
Contributor Author

bw1129 commented Mar 3, 2023

@KarateBrot good catch, thanks

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@haslinghuis
Copy link
Member

@bw1129 you forgot to fix line 236 :)

Co-authored-by: Jan Post <Rm2k-Freak@web.de>
@bw1129
Copy link
Contributor Author

bw1129 commented Mar 6, 2023

@haslinghuis oops, thanks, done!

@github-actions
Copy link

github-actions bot commented Mar 6, 2023

Do you want to test this code? Here you have an automated build:
Assets
WARNING: It may be unstable. Use only for testing! See: https://www.youtube.com/watch?v=I1uN9CN30gw for instructions for unified targets!

@blckmn blckmn merged commit 2efc895 into betaflight:master Mar 10, 2023
18 checks passed
davidbitton pushed a commit to davidbitton/betaflight that referenced this pull request Feb 5, 2024
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.

None yet

5 participants