Refactoring TZ support

[alex-w]

Description

The fixator for missing and wrong time zones is refactored in this pull request - the old code are splitted into code and data now.

Type of change

• Bug fix (non-breaking change which fixes an issue)
• 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: <Name, version number>
• Graphics Card: <Manufacturer (likely Intel, NVidia, AMD?), Model (HD, Geforce, Radeon..., with model number), driver version?>

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]

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]

Ok, it’s ready

[alex-w]

The time zone support in Qt is not very good (esp. in Windows) and if we want to get the better support then we need use 3rd-party library and IANA data.

[gzotti]

Actually Qt uses OS resources, but TZ support on Windows is limited. I have seen interesting results in Linux (like TZ rule details applied in previous years) which were different in Windows. However, when we have a TZ entry for a city, and city changes the actual timezone at some point, this may also not help. It may be more important to find some TZ lookup per geographic coordinates, i.e. clicking on the map. Or replacing (or just extending when online) the map with Qt6.5 by the new map QML interface (with zoom etc.). However, as I am struggling with #3180, this may also need some more extensive redesign.

[gzotti]

From a user perspective: what should the user see when pressing "Update time zones"? IMHO this should happen behind the screen. I have not looked into the relevant code, but what about, at startup:

If (unknownTimezonesDetected())
    updateTimezones()    // check file age, download new file if possible. 
If (unknownTimezonesDetected()) // still unknown?
    complainAsBefore()   // qDebug() messages.

The local TZ file might include an optional timestamp in line 1 to avoid online lookup at every restart. (Don't lookup again on the same date.) It should also contain a warning in the header that discourages users from editing:

# This file is automatically retrieved on demand. Do not edit!

[alex-w]

Actually Qt uses OS resources, but TZ support on Windows is limited. I have seen interesting results in Linux (like TZ rule details applied in previous years) which were different in Windows. However, when we have a TZ entry for a city, and city changes the actual timezone at some point, this may also not help.

IANA TZDB has all changes in timezones. I don't like the latest changes in TZ data (removing TZ data from main file, renaming locations, using fictional TZ for locations by political reasons instead of factical data), but this is a comprehensive timezones database.

It may be more important to find some TZ lookup per geographic coordinates, i.e. clicking on the map. Or replacing (or just extending when online) the map with Qt6.5 by the new map QML interface (with zoom etc.). However, as I am struggling with #3180, this may also need some more extensive redesign.

Mixing Widgets and QML is a bad idea - we've already had this mix and it was removed due problems.

[gzotti]

Where and when did we have QML? It's just new, and mixing should work. Likely not in an embedded dialog but separate window, though. If there was a better Map widget with nice built-in features (e.g. geodata/tz lookup), of course it would be the clearer way to go.

[alex-w]

From a user perspective: what should the user see when pressing "Update time zones"?

Progress bar for downloading the data.

IMHO this should happen behind the screen. I have not looked into the relevant code, but what about, at startup:

If (unknownTimezonesDetected())
    updateTimezones()    // check file age, download new file if possible. 
If (unknownTimezonesDetected()) // still unknown?
    complainAsBefore()   // qDebug() messages.

I can implement it, but this is bad idea:

1. Moving the code from LocationDialog class to StelLocationMgr class will adding extra dependencies for all plugins, where locations are using;
2. The time of loading the Stellarium will be increasing due downloading TZF file every startup (esp. for low speed connections or offline systems);

Of course we can add the code for updating the data by some frequency (every month or something else) like in pluigns, but this is will not resolve point 1 from the list above.

The local TZ file might include an optional timestamp in line 1 to avoid online lookup at every restart. (Don't lookup again on the same date.)

How? With standard Qt's classes for network management this is impossible, because Qt doesn't support partially loading the data. We may use HEAD request and GET request later if filesize is changed, but this is not give us guarantee for following a real changing the data.

It should also contain a warning in the header that discourages users from editing:

# This file is automatically retrieved on demand. Do not edit!

OK

[gzotti]

From a user perspective: what should the user see when pressing "Update time zones"?

Progress bar for downloading the data.

I mean, when and how should John Simple (who never reads the logfile) ever want to press this button? Just because it's "There is a button, let's see what happens!"? Pressing this button appears useful if there is a complaint about missing timezone in the logfile. But this condition can trigger the update automatically.

IMHO this should happen behind the screen. I have not looked into the relevant code, but what about, at startup:

AMENDED:

If (unknownTimezonesDetected() && lastUpdateLongEnoughAgo())
     updateTimezones()    // download new file if possible, save date of last retrieval. 
 If (unknownTimezonesDetected() && !lastUpdateLongEnoughAgo()) // still unknown?
     complainAsBefore()   // qDebug() messages.

I can implement it, but this is bad idea:

1. Moving the code from `LocationDialog` class to `StelLocationMgr` class will adding extra dependencies for all plugins, where locations are using;

Why? The LocationMgr is the central authority for location-specific stuff.

2. The time of loading the Stellarium will be increasing due downloading TZF file every startup (esp. for low speed connections or offline systems);

Only if missing timezones are detected shall there be any attempt at downloading. And of course, not at every start!

Of course we can add the code for updating the data by some frequency (every month or something else) like in pluigns, but this is will not resolve point 1 from the list above.

Not even regular frequency. Update attempt shall be done on demand. (Usually after update of tzdata OS package...)

The local TZ file might include an optional timestamp in line 1 to avoid online lookup at every restart. (Don't lookup again on the same date.)

How? With standard Qt's classes for network management this is impossible, because Qt doesn't support partially loading the data. We may use HEAD request and GET request later if filesize is changed, but this is not give us guarantee for following a real changing the data.

Then use some other mechanism, like a datestamp in config.ini. "Don't attempt to download on the same day" (or +1 week? This gives us a few days to fix such missing timezone). The pseudo-function lastUpdateLongEnoughAgo() would test this.

[alex-w]

Where and when did we have QML? It's just new, and mixing should work. Likely not in an embedded dialog but separate window, though. If there was a better Map widget with nice built-in features (e.g. geodata/tz lookup), of course it would be the clearer way to go.

See db64b13

[github-actions]

This pull request has conflicts, please resolve those before we can evaluate the pull request.

[gzotti]

Ah, interesting, 2014... This was QML1. Whatever this was used for, it looked like some testing environment.

[alex-w]

I mean, when and how should John Simple (who never reads the logfile) ever want to press this button? Just because it's "There is a button, let's see what happens!"? Pressing this button appears useful if there is a complaint about missing timezone in the logfile. But this condition can trigger the update automatically.

Obviously that John Simple also never updating OS and, of course, time zones. I don't see a big problem if users will be press the button.

I can implement it, but this is bad idea:

1. Moving the code from `LocationDialog` class to `StelLocationMgr` class will adding extra dependencies for all plugins, where locations are using;

Why? The LocationMgr is the central authority for location-specific stuff.

Because this is C++. Adding #include <QNetworkReply> into StelLocationMgr.hpp file will add requirement Qt{5|6}::Network as dependencies for all plugins where StelLocationMgr.hpp is included.

2. The time of loading the Stellarium will be increasing due downloading TZF file every startup (esp. for low speed connections or offline systems);

Only if missing timezones are detected shall there be any attempt at downloading. And of course, not at every start!

Seriously? Do you think John Simple above will report about missing time zone? :) Of course, no. So, we don't have data about new missing time zone and we don't add the data into file and result is obvious - attempt to downloading the fixes at every start.

Of course we can add the code for updating the data by some frequency (every month or something else) like in pluigns, but this is will not resolve point 1 from the list above.

Not even regular frequency. Update attempt shall be done on demand. (Usually after update of tzdata OS package...)

So, you expected that user will see the log and press the button. But John Simple...

[github-actions]

Conflicts have been resolved. A maintainer will review the pull request shortly.

[gzotti]

I mean, when and how should John Simple (who never reads the logfile) ever want to press this button? Just because it's "There is a button, let's see what happens!"? Pressing this button appears useful if there is a complaint about missing timezone in the logfile. But this condition can trigger the update automatically.

Obviously that John Simple also never updating OS and, of course, time zones. I don't see a big problem if users will be press the button.

TZ updates are done by the operating system. Typically this is automated today. If there are no OS updates, timezones don't change, and there will be no need for updates.

I can implement it, but this is bad idea:

1. Moving the code from `LocationDialog` class to `StelLocationMgr` class will adding extra dependencies for all plugins, where locations are using;

Why? The LocationMgr is the central authority for location-specific stuff.

Because this is C++. Adding #include <QNetworkReply> into StelLocationMgr.hpp file will add requirement Qt{5|6}::Network as dependencies for all plugins where StelLocationMgr.hpp is included.

Ah, header inclusion. You can probably declare "class QNetworkReply" in the header file (if at all needed), and only include the header in the cpp file.

2. The time of loading the Stellarium will be increasing due downloading TZF file every startup (esp. for low speed connections or offline systems);

Only if missing timezones are detected shall there be any attempt at downloading. And of course, not at every start!

Seriously? Do you think John Simple above will report about missing time zone? :) Of course, no. So, we don't have data about new missing time zone and we don't add the data into file and result is obvious - attempt to downloading the fixes at every start.

Not John Simple, but those few who have reported will hopefully continue to do so. And no, with a timestamp "last-attempted-to-update" stored, there can be a few days of updating silence.

I have somehow forgotten what actually happens if tz is not found. Invalid time for those locations? Other strange behaviour?

Of course we can add the code for updating the data by some frequency (every month or something else) like in pluigns, but this is will not resolve point 1 from the list above.

Not even regular frequency. Update attempt shall be done on demand. (Usually after update of tzdata OS package...)

So, you expected that user will see the log and press the button. But John Simple...

The current situation is IMHO just not good. The test for unknown timezones can simply trigger an update action (... or just the visibility of this button! But one must open the location panel to see that in the first place.), but only if the latest update attempt was already some time ago, preventing update attempts at every start. Usually there will then be no unknown timezones, therefore no network activity. If needed, another config entry could even inhibit this lookup attempt, e.g. on offline installations or when network activity must be inhibited. Those offline sites will also not get tzdata updates from their OS services, so, no problems.

[gzotti]

I did not intend you to kill this at 80%. We had planned such auto-update "in the future" of ~2017/18 when we see how the missing timezones issue develops. Just that I think this update should run without user interaction.

[github-actions]

This pull request has conflicts, please resolve those before we can evaluate the pull request.