current_time() is not valid in gpsbabel_testmode()

[tsteven4]

Today, in gpsbabel_testmode(), i.e., if GPSBABEL_FREEZE_TIME is set, current_time is set to the unix epoch. current_time is of type gpsbabel::DateTime. gpsbabel::DateTime.isValid() considers the unix epoch to be invalid. This is problematic in that current_time cannot be checked for validity before use.

Some possible solutions are:

1. change gpsbabel::DateTime.isValid() to match QDateTime::isValid(), i.e., no special handling of the unix epoch. This is problematic as historically creation_time was of type time_t, and we considered time_t=0 invalid. I think removing the assumption that the unix epoch is invalid, and relying on QDateTime::isValid is an ideal solution, although difficult to implement.
2. change current_time in test mode to a time both gpsbabel::DateTime and QDateTime consider valid, e.g. 1980-01-06T00:00:00Z. This involves massive changes to references, but doesn't force us to remove the historical assumption about the unix epoch.

[robertlipe]

#2 moves the problem around. I don't have a source tree handy, but I think #1 is the right answer. Less of our code should be using time_t as we go along. IMO while I know there is valid timestamped positioned data from the final hours of 1969, we just don't have a lot of GPS tracks from Jan around those hours. Every month we move it forward, we increase the chances of collisions with valid data. Earth, for example, has lots of real data going back to the 40s and it's useful to model things in a continental scale, so it has to keep strong concepts of isBalid vs a real date that's just unlikely. Still, we have some places where we can't really represent the concept of "invalid". What's invalid in CSV? Year zero? Year one? It's probably ok to have some inconsistent behavior here between formats. Maybe 1.1.1970 is invalid in CSV but ok in kml - or vice versa; I'd have to think more about it. I'll wait to be more awake, but from the hip, I vote #1.

…

On Fri, Jan 20, 2023, 10:16 AM tsteven4 ***@***.***> wrote: Today, in gpsbabel_testmode(), i.e., if GPSBABEL_FREEZE_TIME is set, current_time is set to the unix epoch. current_time is of type gpsbabel::DateTime. gpsbabel::DateTime.isValid() considers the unix epoch to be invalid. This is problematic in that current_time cannot be checked for validity before use. Some possible solutions are: 1. change gpsbabel::DateTime.isValid() to match QDateTime::isValid(), i.e., no special handling of the unix epoch. This is problematic as historically creation_time was of type time_t, and we considered time_t=0 invalid. I think removing the assumption that the unix epoch is invalid, and relying on QDateTime::isValid is an ideal solution, although difficult to implement. 2. change current_time in test mode to a time both gpsbabel::DateTime and QDateTime consider valid, e.g. 1980-01-06T00:00:00Z. This involves massive changes to references, but doesn't force us to remove the historical assumption about the unix epoch. — Reply to this email directly, view it on GitHub <#993>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACCSD3Z56VOBYKI2G33CBSDWTK27HANCNFSM6AAAAAAUBWLT4M> . You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>

[tsteven4]

I would say an invalid date time in csv is an empty field. We are part way there, QDateTIme, QDate, QTime toString methods return an empty string if the object is invalid. I am not at all sure we use empty fields for this purpose in csv writers consistently.

I implemented 1). Our test cases show issues with

• gpx reader
• holux reader
• humminbird reader and writer
• igc reader
• random realtime writer in test mode

All but the last can be fixed to work either way. The last is tied up with testmode and current_time, and if we are to consider the epoch valid then a reference file needs to change.

the gpx reader is easy to fix and a clear omission as we moved to QDateTime.

holux doesn't really seem to know if time is local or gmt, and our single sample seems to have no time information at all. The holux writer doesn't even set the timespec before converting the creation_time, although this is undetected in test. It's easy to keep the test going but I have to ask if this format is even used anymore. I can't find any reference files on the web.
holux as a company is gone, the mtk_logger seemed to generate a lot of traffic in the last few years, but maybe the holux gm-100 format is ready to die.

humminbird has comments rejecting time_t = 0 values in routes/tracks, if we do the same for waypoints we get the reader fixed. We do have a sample with a time_t value of 0 (and 1???) seemingly mixed in with other valid data. Fixing that reveals a writer issue. gpsbabel::DateTime.toTime_t returns -1 cast to a uint for and invalid object, as Qt did before, but if we are using time_t = 0 to indicate invalid in the binary file we need to handle it.

igc is actively in use, and has recent specs. Lots of samples are on the web. We can "fix" the code so the test works, although we may be chasing our tail here with reference files we created.

[robertlipe]

. I am not at all sure we use empty fields for this purpose in csv writers consistently.

I am sure we don't. :⁠-⁠) I implemented 1). Our test cases show issues with

- gpx reader - holux reader - humminbird reader and writer - igc reader - random realtime writer in test mode - All but the last can be fixed to work either way. The last is tied up with testmode and current_time, and if we are to consider the epoch valid then a reference file needs to change.

For that one, time should always be valid if w have a gps fix at all. holux doesn't really seem to know if time is local or gmt, and our single

sample seems to have no time information at all. The holux writer doesn't even set the timespec before converting the creation_time, although this is undetected in test. It's easy to keep the test going but I have to ask if this format is even used anymore. I can't find any reference files on the web.

Holux, the company, is dead. Their little film can colored logger used to be popular with geotaggers, but that a broken mtk mutant. I'm in bed and can't check data, but I wouldn't fight with Holux much. Let's try to keep m241, but I don't think we care about gm-100. Permission to kill granted. holux as a company is gone, the mtk_logger seemed to generate a lot of

traffic in the last few years, but maybe the holux gm-100 format is ready to die. humminbird has comments rejecting time_t = 0 values in routes/tracks, if we do the same for waypoints we get the reader fixed. We do have a sample with a time_t value of 0 (and 1???) seemingly mixed in with other valid data. Fixing that reveals a writer issue. gpsbabel::DateTime.toTime_t returns -1 cast to a uint for and invalid object, as Qt did before, but if we are using time_t = 0 to indicate invalid in the binary file we need to handle it.

That was probably an intentional crutch as QDT went in, but I'm hip removing that now. igc is actively in use, and has recent specs. Lots of samples are on the

web. We can "fix" the code so the test works, although we may be chasing our tail here with reference files we created.

Almost certainly. The author came in with a bang, but we had ni reference files and I think we largely made our own. I can't even remember if our writer is "real" or exists solely for test. Given the use case, I suspect our reader is more valuable/trustworthy than our writer. I don't feel a fondness for igc (the code is gross) but we DID get some push back on nuking that one. —

…

Reply to this email directly, view it on GitHub <#993 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACCSD3Z2SD6QZB5XGSIM3WLWTL5F3ANCNFSM6AAAAAAUBWLT4M> . You are receiving this because you commented.Message ID: ***@***.***>

[robertlipe]

@tsteven4 , you said you implemented approach. Did you ever land this? Please close this if so.

[tsteven4]

This is not resolved.

qDebug() << current_time() << current_time().isValid();
in testmode this results in

QDateTime(1970-01-01 00:00:00.000 UTC Qt::UTC) false