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.

Sign up for GitHub

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

Jump to bottom

Reorder DEBUG_GPS_RESCUE_THROTTLE_PID fields #13261

Closed
wants to merge 1 commit into from
Closed

Reorder DEBUG_GPS_RESCUE_THROTTLE_PID fields #13261

wants to merge 1 commit into from

Conversation

haslinghuis
Copy link
Member

Discussed with @ctzsnooze

From:

DEBUG_SET(DEBUG_GPS_RESCUE_THROTTLE_PID, 0, lrintf(throttleP));
DEBUG_SET(DEBUG_GPS_RESCUE_THROTTLE_PID, 1, lrintf(throttleD));
DEBUG_SET(DEBUG_GPS_RESCUE_THROTTLE_PID, 2, lrintf(rescueState.sensor.currentAltitudeCm));
DEBUG_SET(DEBUG_GPS_RESCUE_THROTTLE_PID, 3, lrintf(rescueState.intent.targetAltitudeCm));
DEBUG_SET(DEBUG_GPS_RESCUE_THROTTLE_PID, 4, lrintf(throttleI));
DEBUG_SET(DEBUG_GPS_RESCUE_THROTTLE_PID, 5, lrintf(tiltAdjustment));
DEBUG_SET(DEBUG_GPS_RESCUE_THROTTLE_PID, 6, lrintf(throttleD)); // throttle D before lowpass smoothing
DEBUG_SET(DEBUG_GPS_RESCUE_THROTTLE_PID, 7, lrintf(throttleAdjustment));

To:

DEBUG_SET(DEBUG_GPS_RESCUE_THROTTLE_PID, 0, lrintf(throttleP));
DEBUG_SET(DEBUG_GPS_RESCUE_THROTTLE_PID, 1, lrintf(throttleI));
DEBUG_SET(DEBUG_GPS_RESCUE_THROTTLE_PID, 2, lrintf(throttleD));
DEBUG_SET(DEBUG_GPS_RESCUE_THROTTLE_PID, 3, lrintf(throttleD)); // throttle D before lowpass smoothing
DEBUG_SET(DEBUG_GPS_RESCUE_THROTTLE_PID, 4, lrintf(tiltAdjustment));
DEBUG_SET(DEBUG_GPS_RESCUE_THROTTLE_PID, 5, lrintf(throttleAdjustment));
DEBUG_SET(DEBUG_GPS_RESCUE_THROTTLE_PID, 6, lrintf(rescueState.sensor.currentAltitudeCm));
DEBUG_SET(DEBUG_GPS_RESCUE_THROTTLE_PID, 7, lrintf(rescueState.intent.targetAltitudeCm));

Fixes betaflight/blackbox-log-viewer#680

@haslinghuis haslinghuis added this to the 4.5 milestone Jan 2, 2024
@haslinghuis haslinghuis self-assigned this Jan 2, 2024
@github-actions GitHub Actions
Copy link

github-actions bot commented Jan 2, 2024

Do you want to test this code? You can flash it directly from Betaflight Configurator:

  • Simply put #13261 (this pull request number) in the Select commit field of the Configurator firmware flasher tab (you need to Enable expert mode, Show release candidates and Development).

WARNING: It may be unstable. Use only for testing!

@blckmn
Copy link
Member

blckmn commented Jan 2, 2024 •
edited
Loading

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 -> PASS
  • Don't merge label NOT found -> PASS
  • at least one RN: label found -> FAIL
  • Tested label found -> FAIL
  • assigned to an approver -> PASS
  • approver count at least three -> FAIL

@ctzsnooze
Copy link
Member

It would be better to not rearrange these fields.
In the log viewer, we do version control on the field names, but the graph scaling can't easily be changed.
If we re-arrange these parameters, and an older file is loaded, the wrong graph scaling will apply.

I think the cause of the problem was because in blackbox there was no version 4.5 field definitions forGPS_RESCUE_THROTTLE_PID, and in 4.5 we used all 8 values, whereas in 4.4 we only used the first 4. Then I think a PR would 'hide' undefined names, leading to invisible names for a 4.5 log.

This should be fixed by fixing the log viewer, not changing the fields in the firmware.

@haslinghuis haslinghuis closed this Jan 3, 2024
@haslinghuis haslinghuis deleted the reorder-debug-gps-rescue-throttle-pid branch January 3, 2024 22:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@haslinghuis haslinghuis

Labels
Cleanup Needs coordination with betaflight-configurator Needs coordination with blackbox-log-viewer
Projects
Finalizing Firmware 4.5 Release
Status: COMPLETED
Milestone
4.5
Development

Successfully merging this pull request may close these issues.

4.5 RC1: Blackbox - GPS_RESCUE_THROTTLE_PID - Missing signal names
3 participants
@haslinghuis @blckmn @ctzsnooze