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

Display heading on GPS Tab #3355

Merged
merged 3 commits into from Mar 2, 2023
Merged

Conversation

atomgomba
Copy link
Contributor

@atomgomba atomgomba commented Feb 24, 2023

Use data from magnetometer (when enabled) to display heading in degrees and rotate the map accordingly. Can be used to verify correct mag alignment or for testing in general. This PR also adds Virtual FC support for GPS Tab so when connected to VFC pre-defined sample data and thus the map as well will show up.

@github-actions

This comment has been minimized.

@blckmn
Copy link
Member

blckmn commented Feb 24, 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 -> 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

Copy link
Member

@haslinghuis haslinghuis left a comment

Choose a reason for hiding this comment

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

Good idea !!!

@@ -221,4 +235,22 @@ const VirtualFC = {
},
};

const SampleGpsData = {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
const SampleGpsData = {
const sampleGpsData = {

"speed": 0,
"ground_course": 1337,
"distanceToHome": 0,
"ditectionToHome": 0,
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
"ditectionToHome": 0,
"directionToHome": 0,

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is interesting, because I copied the object from memory to clipboard using copy(FC.GPS_DATA) in the console. I have to double check this, I think the typo is fundamentally somewhere else...

Comment on lines 187 to 189
const heading = hasMag
? Math.atan2(FC.SENSOR_DATA.magnetometer[1], FC.SENSOR_DATA.magnetometer[0])
: undefined;
Copy link
Member

Choose a reason for hiding this comment

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

I would prefer one line:

Suggested change
const heading = hasMag
? Math.atan2(FC.SENSOR_DATA.magnetometer[1], FC.SENSOR_DATA.magnetometer[0])
: undefined;
const heading = hasMag ? Math.atan2(FC.SENSOR_DATA.magnetometer[1],FC.SENSOR_DATA.magnetometer[0])
: undefined;

console.log(`Map error ${err}`);
}
} catch (err) {
console.error('Map error', err);
Copy link
Member

Choose a reason for hiding this comment

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

Why remove string interpolation?

Copy link
Contributor Author

@atomgomba atomgomba Feb 24, 2023

Choose a reason for hiding this comment

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

So that we let the browser format the error object as it wishes and not force it to be a string manually.

Copy link
Contributor Author

@atomgomba atomgomba Feb 24, 2023

Choose a reason for hiding this comment

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

Plus, it won't really show since the map is running in its own webview with its own console. You have to have the console of the inner webview open to see the message anyway. It's bad, but I didn't take the time to look deeper into the issue

ublox_use_galileo: 1,
};

virtualFC.GPS_DATA = SampleGpsData;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
virtualFC.GPS_DATA = SampleGpsData;
virtualFC.GPS_DATA = sampleGpsData;

Copy link
Member

@haslinghuis haslinghuis left a comment

Choose a reason for hiding this comment

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

Oops, duplicate found:

image

@github-actions

This comment has been minimized.

@atomgomba
Copy link
Contributor Author

atomgomba commented Feb 24, 2023

Oops, duplicate found:

image

Oh, you have found the typo! I believe the original typo was in fc.js but it was just a default value which didn't cause a problem, since on every update in MSPHelper.js the name was correct and this field isn't used in BFC at all! Nice catch!

@sonarcloud
Copy link

sonarcloud bot commented Feb 24, 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.3% 0.3% Duplication

@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!

@nerdCopter
Copy link
Member

  • is everything resolved here?
  • can @atomgomba add an photo/screenshot of what it does in the description?

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.

nice! approving. watched demo video

@blckmn blckmn merged commit a4bc6ef into betaflight:master Mar 2, 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.

None yet

4 participants