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

GPS location managed by Windows #2620

Merged
merged 36 commits into from
Sep 14, 2022
Merged

Conversation

uwes-ufo
Copy link
Contributor

@uwes-ufo uwes-ufo commented Sep 1, 2022

Description

GPS location managed by Windows
On Windows systems, built-in GPS sensors are managed by the operating system. There is no serial port or NMEA protocol. But Qt can handle it.

Type of change

  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • This change requires a documentation update

How Has This Been Tested?

Test Configuration:

  • Operating system: Windows 11
  • Graphics Card: Intel UHD 620

Checklist:

  • My code follows the code style of this project.
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation (header file)
  • I have updated the respective chapter in the Stellarium User Guide
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

@github-actions
Copy link

github-actions bot commented Sep 1, 2022

Great PR! Please pay attention to the following items before merging:

Files matching src/**/*.cpp:

  • Are possibly unused includes removed?

This is an automatically generated QA checklist based on modified files

@alex-w
Copy link
Member

alex-w commented Sep 4, 2022

I can try use GPS-dongle to test this PR tomorrow

@gzotti
Copy link
Member

gzotti commented Sep 4, 2022

I am busy this week. Maybe I can find a slot to test the smartphone/BT bridge which worked with the existing solution.

@alex-w
Copy link
Member

alex-w commented Sep 5, 2022

The PR works with GPS-dongle u-blox 7, but this is through NMEA

@alex-w alex-w added the enhancement Improve existing functionality label Sep 8, 2022
@uwes-ufo
Copy link
Contributor Author

do you have Windows UWP by the fact?

No.

loc.name = QString("GPS %1%2 %3%4")
.arg(loc.latitude<0?"S":"N").arg(floor(.5+abs(loc.latitude)))
.arg(loc.longitude<0?"W":"E").arg(floor(.5+abs(loc.longitude)));
core->moveObserverTo(loc, 0.0, 0.0);
Copy link
Member

Choose a reason for hiding this comment

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

These two lines could also be replaced by
emit queryFinished(loc);

to avoid some code duplication.

loc.isUserLocation = true;
loc.planetName = "Earth";
loc.name = QString("GPS %1%2 %3%4")
.arg(loc.latitude<0?"S":"N").arg(floor(.5+abs(loc.latitude)))
Copy link
Member

Choose a reason for hiding this comment

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

I understand the 0.5 is for rounding purposes. In this case we should also change the GPS-based name of the NMEALookupHelper::nmeaUpdated() on which this method is based.

@gzotti
Copy link
Member

gzotti commented Sep 14, 2022

Good to remove the code duplicates, thanks. Can you please add some lines of user documentation into guide/ch_advanced_use.tex, at line 448? Built-in GPS sensors in WIndows devices are new to me.

@uwes-ufo
Copy link
Contributor Author

uwes-ufo commented Sep 14, 2022

Built-in GPS sensors in WIndows devices are new to me.

I found this: https://support.microsoft.com/en-us/windows/windows-location-service-and-privacy-3a8eee0a-5b0b-dc07-eede-2a5ca1c49088#WindowsVersion=Windows_11

"... When this location setting is enabled, the Microsoft location service will use a combination of global positioning service (GPS), nearby wireless access points, cell towers, and your IP address (or default location) to determine your device’s location ..."

So you may get an approximate position without a GPS sensor?

I deactivated my gps sensor in the device manager as a test - it still works, but I don't get any altitude information.

@gzotti
Copy link
Member

gzotti commented Sep 14, 2022

@gzotti according to QGeoPositionInfoSource class documentation we should switch to use of it in the future for Windows/macOS.

The NmeaPositionInfoSource is a subclass. Uwe apparently has a Win11 device with built-in GPS receiver, which communicates directly.

@gzotti
Copy link
Member

gzotti commented Sep 14, 2022

Built-in GPS sensors in WIndows devices are new to me.

I found this: https://support.microsoft.com/en-us/windows/windows-location-service-and-privacy-3a8eee0a-5b0b-dc07-eede-2a5ca1c49088#WindowsVersion=Windows_11

"... When this location setting is enabled, the Microsoft location service will use a combination of global positioning service (GPS), nearby wireless access points, cell towers, and your IP address (or default location) to determine your device’s location ..."

So you may get an approximate position without a GPS sensor?

Ah good! If that is what is used here, fine. Maybe the button should then be renamed on Windows:

Get location from GPS or system service

Copy link
Member

@gzotti gzotti left a comment

Choose a reason for hiding this comment

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

OK, seems to work. I can use my Bluetooth NMEA or the system's fallback service.
Thank you!

@gzotti gzotti merged commit b7ddb8d into Stellarium:master Sep 14, 2022
@gzotti
Copy link
Member

gzotti commented Sep 14, 2022

So you may get an approximate position without a GPS sensor?

I deactivated my gps sensor in the device manager as a test - it still works, but I don't get any altitude information.

Yes, it seems this result is similar to the "get location from Network" option.

@uwes-ufo uwes-ufo deleted the uwes-ufo-gps branch September 14, 2022 19:13
@gzotti gzotti added this to the 1.0 milestone Sep 14, 2022
@alex-w alex-w added the state: published The fix has been published for testing in weekly binary package label Sep 17, 2022
@github-actions
Copy link

Hello @uwes-ufo!

Please check the fresh version (development snapshot) of Stellarium:
https://github.com/Stellarium/stellarium-data/releases/tag/weekly-snapshot

@uwes-ufo
Copy link
Contributor Author

What I stumbled across while browsing...

diff --git a/src/core/StelLocationMgr.cpp b/src/core/StelLocationMgr.cpp
index 5c3327c..a49d98d 100644
--- a/src/core/StelLocationMgr.cpp
+++ b/src/core/StelLocationMgr.cpp
@@ -1053,7 +1053,7 @@
 		.arg(loc.latitude<0?"S":"N").arg(qRound(abs(loc.latitude)))
 		.arg(loc.longitude<0?"W":"E").arg(qRound(abs(loc.longitude)));
 
-	StelApp::getInstance().getCore()->moveObserverTo(loc, 0.0, 0.0);
+	core->moveObserverTo(loc, 0.0, 0.0);
 	if (nmeaHelper)
 	{
 		if (verbose)

@gzotti
Copy link
Member

gzotti commented Sep 22, 2022

f6db5c6

@alex-w alex-w removed the state: published The fix has been published for testing in weekly binary package label Oct 1, 2022
@github-actions
Copy link

github-actions bot commented Oct 1, 2022

Hello @uwes-ufo!

Please check the latest stable version of Stellarium:
https://github.com/Stellarium/stellarium/releases/latest

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improve existing functionality
Development

Successfully merging this pull request may close these issues.

3 participants