-
-
Notifications
You must be signed in to change notification settings - Fork 147
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
Fix heading offset #600
Fix heading offset #600
Conversation
This comment has been minimized.
This comment has been minimized.
AUTOMERGE: (FAIL)
|
0681baf
to
d0a215c
Compare
This comment has been minimized.
This comment has been minimized.
js/flightlog_fields_presenter.js
Outdated
case 'ALTITUDE': | ||
switch (fieldName) { | ||
case 'debug[0]': // GPS Trust * 100 | ||
value.toFixed(0); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
value.toFixed(0); | |
return value.toFixed(0); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, that was a silly mistake.
js/graph_config.js
Outdated
var sysConfig = flightLog.getSysConfig(); | ||
var debugModeName = DEBUG_MODE[sysConfig.debug_mode]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
var sysConfig = flightLog.getSysConfig(); | |
var debugModeName = DEBUG_MODE[sysConfig.debug_mode]; | |
const sysConfig = flightLog.getSysConfig(); | |
const debugModeName = DEBUG_MODE[sysConfig.debug_mode]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change was from a commit that was accidentally added in this PR by mistake.
It was intended to add the debug name into the text that appears to the right of the graph, but it didn't work, so I've removed that commit.
That removes the need to make this change, but I'll put it into the code I was testing.
Thank you again.
js/imu.js
Outdated
if (heading > 2* Math.PI) | ||
heading -= 2* Math.PI; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if (heading > 2* Math.PI) | |
heading -= 2* Math.PI; | |
if (heading > 2 * Math.PI) { | |
heading -= 2 * Math.PI; | |
} | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks, wasn't sure of the need for the braces
Thanks very much @haslinghuis for your advice and comments. |
d0a215c
to
6944b89
Compare
Kudos, SonarCloud Quality Gate passed! 0 Bugs No Coverage information |
Do you want to test this code? Here you have an automated build: |
The heading value shown in Explorer's IMU calculation did not match that shown in the GPS Heading or IMU Debug values.
I noted that the blackbox IMU heading calculation is derived from other parameters, and differs from the IMU heading value in debugs. However it may be useful in some cases, and should at least have the same values and display the same when graphed. This PR fixes that problem so that all heading values match up numerically and in graphs.
Other minor changes: