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

Add all 8 debug values in sensors tab and only show active debugs #3522

Merged
merged 5 commits into from
Jul 24, 2023

Conversation

haslinghuis
Copy link
Member

@haslinghuis haslinghuis commented Jul 20, 2023

SpoilerOnce a debug != 0 it will show up persistent.

@github-actions

This comment has been minimized.

@haslinghuis
Copy link
Member Author

@blckmn
Copy link
Member

blckmn commented Jul 21, 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 -> PASS
  • approver count at least three -> FAIL

Copy link
Member

@limonspb limonspb left a comment

Choose a reason for hiding this comment

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

Does it work ok, if old firmware is connected with 4 values only?

@haslinghuis
Copy link
Member Author

I guess only some DEBUG sets contain all 8 values. Try GPS_UNIT_CONNECTION (testing with #12799

@nerdCopter

This comment was marked as outdated.

@nerdCopter
Copy link
Member

retested.
#3522 shows 8 debugs on sensors tab no matter what firmware. (0-7).

@haslinghuis
Copy link
Member Author

haslinghuis commented Jul 21, 2023

Change was made in #12445 so it should only apply to API 1.46 and above. But that would require a lot of work :(

The rows are at the bottom so they are not too disturbing imho.

@haslinghuis
Copy link
Member Author

Ok did it :)

@nerdCopter
Copy link
Member

nerdCopter commented Jul 21, 2023

i will test 40bc6e3 when i can, i was working on something as well, but upon failure, i quit.
it does not work, but may show where i was aiming: diff.txt

@haslinghuis haslinghuis changed the title Add all 8 debug values in sensors tab Add all 8 debug values in sensors tab and only show active debugs Jul 21, 2023
@github-actions

This comment has been minimized.

@nerdCopter

This comment was marked as outdated.

@haslinghuis
Copy link
Member Author

Fixed :)

@nerdCopter

This comment was marked as outdated.

@haslinghuis
Copy link
Member Author

Forgot the .gte after refactoring the line from an if statement.

sensors.debugColumns = semver.gte(FC.CONFIG.apiVersion, API_VERSION_1_46) ? 8 : 4;

@nerdCopter
Copy link
Member

nerdCopter commented Jul 21, 2023

  • i believe this is working as expected. (as coded)

  • i do see a strange behavior, maybe it is desired. where if there is no data, it will not display.

    • example: i have no baro, set debug = baro; no graph
    • example: set debug = antigravity; no graph 0,1, but shows graph 2,3

    edit: this test before new commit 68efcbd

@haslinghuis
Copy link
Member Author

Yes that's the dynamic part. Only when there is data the column is enabled for the whole session to hide inactive debug data (only 0)

Copy link
Member

@nerdCopter nerdCopter left a comment

Choose a reason for hiding this comment

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

  • tested 4.5 master
  • tested 4.3.2
  • seems as expected
  • maybe request other testers as well (have them enable console)

@github-actions

This comment has been minimized.

@sonarcloud
Copy link

sonarcloud bot commented Jul 23, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

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
0.0% 0.0% Duplication

@haslinghuis
Copy link
Member Author

Rebased on master

@github-actions
Copy link
Contributor

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

@ctzsnooze
Copy link
Member

Not sure that the logic within only show active debugs is totally reliable, but having all 8 is awesome!

@haslinghuis haslinghuis merged commit 62a9cde into betaflight:master Jul 24, 2023
9 checks passed
@haslinghuis haslinghuis deleted the add-debug branch July 24, 2023 09:50
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