Skip to content
This repository has been archived by the owner on Sep 10, 2023. It is now read-only.

[suggestion] Add compile for AVR and ESP8266 and organize conditional compilation #25

Closed
Martin-Laclaustra opened this issue Apr 25, 2019 · 4 comments

Comments

@Martin-Laclaustra
Copy link

Dear Alex,
Thank you for programming this library.
I just created a pull request to allow compilation for AVR and ESP8266 (#24).
The code for the time conversion functions has become a bit messy.
Do you think it would be sensible to refactor it to a series of preprocessor directives in order explicitly stating each platform and a final default one?

#if !defined(_WIN32) && !defined(__AVR__) && !defined(ESP8266) && !defined(ANDROID)
/* forward declaration for platforms that may need it */
/* can be hidden in time.h */
time_t timegm(struct tm* __tp);
#endif

time_t cron_mktime(struct tm* tm) {
#if defined(_WIN32)
    return _mkgmtime(tm);
#elif defined(__AVR__)
    return mk_gmtime(tm);
#elif defined(ESP8266)
    /*code...*/
#elif defined(ANDROID)
    /*code...*/
#else
    return timegm(tm);
#endif
}

... and equivalent structured code for cron_time() and for the same two functions in the case of local time.

@staticlibs
Copy link
Owner

Hi, thanks for the suggestion!

Yes, I will happily accept the PR with the cleanup for platform-specific time functions. This part became too hairy after the addition of Android, and more platforms (mingw etc) just making it more confusing.

Please let me know, if I need to merge #24 now, or is it better to wait for the restructuring described in this issue.

@Martin-Laclaustra
Copy link
Author

I created an additional commit with the refactored code.
It compiles in linux gcc, ESP8266, and Arduino Uno (with and without the local flag). Please test it in win, mingw, and android.
I performed a minimal functional test of this new code in ESP8266 and it seems to perform the expected function.
I defined some intermediate functions:

cron_mktime_gm(tm)
cron_time_gm(date, out)
cron_mktime_local(tm)
cron_time_local(date, out)

... aiming to propose, later on (future work), the possibility of creating expressions referred to UTC and to local time, which should be stored in the expression, and taken into account when calculating cron_next (time_t provided to cron_next should be always UTC, and the system TZ should be correctly set in the environment).

@staticlibs
Copy link
Owner

Thanks for the update! Windows (no mingw though), Mac and Android passes on CI, I merged the changes in #24 . Closing this assuming PR fully covers this issue, please feel free to reopen if needed.

@Martin-Laclaustra
Copy link
Author

I created a wrapper to allow use of your library in Arduino IDE.
https://github.com/Martin-Laclaustra/CronAlarms.git

Thank you for your work!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants