cpu/atmega_common: Use updated time.h

[ZetaR60]

On the ATmega, functions like mktime and gmtime output garbage. There is a mismatch between some of the definitions in cpu/atmega_common/avr-libc-extra/time.h and avr-libc. I have removed this file so that the system header file can be used. There is a minor struct, not provided by time.h, that I moved. And the time_t typedef now conforms to avr-libc. See also: https://www.nongnu.org/avr-libc/user-manual/group__avr__time.html

Required by: #8842

[jnohlgard]

I think the time.h header was added when avr-libc was missing support for it, I don't remember what version or how long ago that was, but it is a good idea to check that we are not breaking the build for users with a somewhat older avr-libc.

[ZetaR60]

I can confirm that many Linux distros do not have a new enough version of avr-libc to include time.h. I could simply update the file so that it matches the current avr-libc provided time.h.

[jnohlgard]

I think that might be a better solution, until the distros catch up.

[ZetaR60]

Added time.h from avr-libc-2.0.0. I checked trunk for avr-libc as well, and its only differences from the 2.0.0 release are some comments.

[ZetaR60]

Seems to be working well with the #8842 RTC code. The only other uses of time.h I have seen are relatively minor things in cpu/atmega_common/include/sys/stat.h and cpu/atmega_common/include/sys/time.h. Should be ready for CI build testing.

[ZetaR60]

Squashed. Please let me know if there is anything else that needs to be done on this PR.

[ZetaR60]

Tested and working with move from cpu/atmega_common/avr-libc-extras to cpu/atmega_common/include/vendor. This prevents CI from doing format checking of the header files (which are copies of avr-libc headers).

[ZetaR60]

@gebart Is there anything else that is needed for this PR?

[jnohlgard]

Why did you move struct timespec to sys/time.h? The posix specification states that it should be defined in time.h

[ZetaR60]

vendor/time.h is an exact copy of the avr-libc-2.0.0 time.h, which didn't include timespec for whatever reason. Should I put it in the avr-libc time.h? It wouldn't be an exact copy anymore.

[jnohlgard]

In RIOT we try to conform to standards as far as reasonable. Let's do it like this: create one commit where you import the avr-libc header with no changes, then create a separate commit where you add the struct definition. This way we can easily reapply the change if we update the header in the future.

[ZetaR60]

Done. Should be clear for future updaters now.

[jnohlgard]

Looks fine. I don't have any atmega boards available for testing. Which test application did you use when you discovered the issue you described in the original post about mktime etc?

[jnohlgard]

tests/pkg_minmea is failing for avr now:

minmea.c: In function 'minmea_gettime':
minmea.c:630:19: error: comparison between signed and unsigned integer expressions [-Werror=sign-compare]
     if (timestamp != -1) {
                   ^

[ZetaR60]

I caught this problem when testing tests/periph_rtc for #8842. It outputted garbage because time.h's tm struct defines did not match the assumptions of the code in avr-libc.

Unfortunately there are some compatibility issues with pkg_minmea and sys/timex due to avr-libc implementing non-standard types for time.h. I don't think this is fixable without rewriting those parts of avr-libc. I have blacklisted pkg_minmea and I am working on triage for sys/timex.

[ZetaR60]

Rebased on trunk to resolve merge conflicts.

[ZetaR60]

I think I have put out all the fires (there were only small ones). Let me know when to squash.

[ZetaR60]

@gebart Squash?

[smlng]

yes, please squash

[ZetaR60]

Squashed and double checked with tests/periph_rtc using #8842. Everything is working fine and ready to go.

[kYc0o]

Looks OK and it's already ACKed, so let's merge!