kosma / minmea

a lightweight GPS NMEA 0183 parser library in pure C
Do What The F*ck You Want To Public License
756 stars 246 forks source link

Handle four-digit year in minmea_gettime #25

Closed asund closed 6 years ago

asund commented 6 years ago
kosma commented 6 years ago

This really needs unit testing to make sure we're handling all cases correctly. Could you please add tests for edge conditions in test_minmea_gettime? Specifically, the following cases:

asund commented 6 years ago

Thank you, will do. Do you mind the assumption of the GPS epoch for interpreting two digit years?

kosma commented 6 years ago

The assumption sounds fair to me - at least, this the most logical way to handle two-digit years. Also, minmea_gettime() is just a convenience helper, so if someone happens to own a receiver with really b0rked date handling they can just roll out their own version.

asund commented 6 years ago

Another question: I've written tests that run well on my Mac, but I'm a little less confident in systems with a 32-bit time_t and a standard library that is affected by the year 2038 problem. I can ifdef out tests out of this range based on sizeof(time_t) and an assumption that a typical platform will use a signed int. Does this sound good to you?

EDIT: Can't mix ifdef and sizeof, so I used an if instead.

kosma commented 6 years ago

I like if (sizeof(time_t) == sizeof(int64_t)) as platforms with 32-bit time_t are screwed anyway. One more thing though: can you please normalize whitespace in your patches? All tabs should be four spaces. That, and we're good to merge.

asund commented 6 years ago

Thanks for your patient feedback.

kosma commented 6 years ago

Merged. Thanks a lot!