Skip to content

Conversation

per1234
Copy link

@per1234 per1234 commented Dec 30, 2018

  • Make readRules and writeRules functions compatible with ESP8266/ESP32
  • Add esp8266 and esp32 to the library.properties architectures field

I updated the WriteRules example sketch to be compatible with ESP8266/ESP32. This meant adding some preprocessor directives which make it a bit less beginner friendly. I think it was worthwhile to demonstrate to the user that they need to call EEPROM.begin() in their own sketch if using the readRules or writeRules functions. If you prefer that sketch to remain in its previous AVR-only state, I'm happy to revert that part of this PR.

Fixes #43

CC: @remcoNL

- Switch from avr-libc's avr/eeprom.h to the Arduino EEPROM library for a universal API.
- Add a call to EEPROM.commit() for ESP8266 and ESP32, since this is only necessary for those boards.
- Add a call to EEPROM.begin() for ESP8266/ESP32 in the WriteRules example sketch.
Now that the readRules and writeRules functions have been updated, there is nothing AVR-specific about this library.
@remcoNL
Copy link

remcoNL commented Dec 30, 2018

It works for me. I even understand the code! (-;

@remcoNL
Copy link

remcoNL commented Dec 30, 2018

It compiles well, but now I did another test. I tried to save and read the TimeChangeRules. This works on the arduino nano without a problem. The ESP32 seems not to read or write the new rules.

The sketch saves the timerules, they are commented out and the sketch is uploaded again. The arduino still shows the correct offset, but the ESP32 does not and falls back to the original offset.

#include <Timezone.h> 
#include <Time.h>
#include <EEPROM.h>

TimeChangeRule *tcr; 
Timezone  usEastern(300);
void setup()
{
EEPROM.begin(400);  // comment out on Arduino
 
// ** *** *** Comment out/in to see if read/write works (different offset)
//
//TimeChangeRule usEdt = {"EDTZ", Second, Sun, Mar, 2, 360};  
//TimeChangeRule usEst = {"EST", 1, Sun, 7, 2, 360};    
//Timezone usEastern(usEdt, usEst);
//usEastern.writeRules(300);   

Serial.begin(9600);
setTime(usEastern.toUTC(compileTime()));
}

void loop()
{
time_t utc;
utc = now(); 
time_t local = usEastern.toLocal(utc, &tcr);

Serial.print("HOUR: ");
Serial.println(hour());
Serial.println(second());

delay(2000);
    
}


time_t compileTime()
{
    const time_t FUDGE(10);     // fudge factor to allow for compile time (seconds, YMMV)
    const char *compDate = __DATE__, *compTime = __TIME__, *months = "JanFebMarAprMayJunJulAugSepOctNovDec";
    char chMon[3], *m;
    tmElements_t tm;

    strncpy(chMon, compDate, 3);
    chMon[3] = '\0';
    m = strstr(months, chMon);
    tm.Month = ((m - months) / 3 + 1);

    tm.Day = atoi(compDate + 4);
    tm.Year = atoi(compDate + 7) - 1970;
    tm.Hour = atoi(compTime);
    tm.Minute = atoi(compTime + 3);
    tm.Second = atoi(compTime + 6);
    time_t t = makeTime(tm);
    return t + FUDGE;           // add fudge factor to allow for compile time
}

@JChristensen
Copy link
Owner

Guys, I hate to just close this but I feel that's the best choice for me at this time. I don't have ESP hardware, so I can't test and I can't much afford the bandwidth anyway. @per1234 I very much appreciate your efforts and the spirit in which they are offered.

@remcoNL
Copy link

remcoNL commented Dec 30, 2018

I also found that EEPROM does not work outside of the setup or loop function on the ESP32.

exit status 1
'EEPROM' does not name a type

@remcoNL
Copy link

remcoNL commented Dec 30, 2018

O sorry you closed it, it didn't refresh yet.

@JChristensen
Copy link
Owner

No worries. I am just not able to address the ESP hardware. Quite frankly the whole EEPROM thing seems like a bit of a hack if they're emulating EEPROM using the flash memory. OK I guess but it seems like a square peg in a round hole. Sorry.

@remcoNL
Copy link

remcoNL commented Dec 30, 2018

Ok thanks. For other users this might help.

@per1234
Copy link
Author

per1234 commented Dec 31, 2018

@remcoNL Ah, that constructor won't work on the ESP8266 and ESP32 because it runs before the call to EEPROM.begin(). I did do testing of the updated writeRules and readRules on AVR, ESP8266 and ESP32, but I didn't test the constructor. I only noticed that feature at the last minute and changed the preprocessor conditional thinking "well, I already tested readRules so that's all good", without considering the implications.

@JChristensen I understand your reasoning. I was mislead into thinking you would be receptive to this PR by your statement:

Assuming it's compatible with the Arduino EEPROM library from an API standpoint, you could try taking the preprocessor directives out of the Timezone library. If you try that and it works, let me know and I can change the Timezone library.

If this was pursued further, I would suggest reverting the preprocessor conditional on Timezone::Timezone(int address) to #ifdef __AVR__, since I don't see a good way to make that work on ESP8266/ESP32. I still think it would be useful to make writeRules and readRules non-AVR specific.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants