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

Set correct light pollution level when location=auto #4216

Merged
merged 5 commits into from
Mar 20, 2025
Merged

Conversation

alex-w
Copy link
Member

@alex-w alex-w commented Mar 17, 2025

Description

This patch should obtaining a realistic light pollution level for auto discovered locations (at least for big cities)

Fixes #2762 (issue)

Type of change

  • Bug fix (non-breaking change which fixes an issue)

Copy link

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.

@10110111
Copy link
Contributor

Where will it ultimately come from?

@alex-w
Copy link
Member Author

alex-w commented Mar 17, 2025

Where will it ultimately come from?

From our database if city name is known

@alex-w
Copy link
Member Author

alex-w commented Mar 17, 2025

@10110111 could you check how works getting the location (and light pollution level) from IP for St. Petersbourg?

@10110111
Copy link
Contributor

could you check how works getting the location (and light pollution level) from IP for St. Petersbourg?

Not sure about SPb, since I don't have access to my machine there, but for Kuta, Indonesia I get a very wrong location:

Got location ", , Indonesia (-6.175, 106.829; Asia/Jakarta)" for IP "106.0.50.27"

So there's no city, but the coordinates indicate Jakarta.

And when I choose light pollution setting "From location database", the value gets set to Bortle class 2, which is very wrong for Kuta that's the brightest part of Bali, and just as well wrong for Jakarta that's the brightest part of Java.

I think it would be better to use some kind of a light pollution map instead of a city-based table. E.g. The World Atlas of the Artificial Night Sky Brightness.

@gzotti
Copy link
Member

gzotti commented Mar 17, 2025

The IP-based location lookup has set me to places 18km (regularly) or even 160km away from my place. It seems OK to set the location close enough to get a roughly correct sky, but for anything else we'd need more. Looking up the IP-based coordinates in a LP database will therefore also not really help.

The various OSes have introduced their own location APIs which may work better, but many users dislike being "tracked". Should we try to make use of that?

@10110111
Copy link
Contributor

The various OSes have introduced their own location APIs which may work better, but many users dislike being "tracked". Should we try to make use of that?

Isn't it just an API to the GPS sensors, whose input we already support?

@gzotti
Copy link
Member

gzotti commented Mar 17, 2025

They must do something around network traffic. My desktop PC and laptop don't have sensors, but in the system control panel there are settings that work considerably better than our IP-based lookup.

@10110111
Copy link
Contributor

Well, if it's indeed something more than the GPS, then I suppose we could add support for this, maybe as a button near the "Get from GPS" one.

@gzotti
Copy link
Member

gzotti commented Mar 17, 2025

Ah yes, it's in Qt... https://doc.qt.io/qt-6/qtpositioning-index.html
Maybe try that first, then our current IP lookup. OS panel may popup to confirm the program may find out location.

EDIT: I am trying my luck on this right now... #4221

@alex-w alex-w marked this pull request as draft March 17, 2025 17:35
@alex-w alex-w marked this pull request as ready for review March 18, 2025 12:23
@alex-w
Copy link
Member Author

alex-w commented Mar 18, 2025

Added code for processing locations, which presents by coordinates only

alex-w and others added 2 commits March 18, 2025 20:10
Probably 1/2 degree (55km) is still too large anyhow,
but in dense areas you may get several entries even in 0.2 deg radius.

I have added some debug messages.
I cannot test this because my network retrieval finds a known city.
@alex-w alex-w added this to the 25.1 milestone Mar 20, 2025
@alex-w
Copy link
Member Author

alex-w commented Mar 20, 2025

At least it works now (not 100% good but better than nothing)

@alex-w alex-w merged commit f887ec1 into master Mar 20, 2025
28 of 29 checks passed
@alex-w alex-w deleted the issue2762 branch March 20, 2025 19:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Location=auto fails to set correct light pollution level
3 participants