256dpi / arduino-mqtt

MQTT library for Arduino
MIT License
998 stars 229 forks source link

Custom Clock Source #141

Closed jmpmscorp closed 5 years ago

jmpmscorp commented 5 years ago

Hi.

Firstly, thank you very much. Your library is wonderful.

I want to ask you about timer_set and timer_get in arduino. I have an arduino project where I need to save maximum battery as possible because it's powered by a battery with solar panel. To reach it, I use sleep_power_down (deeper power down mode) but it makes that millis() doesn't work well because timer 0 is power down is this mode. WDT wakes up my board every 8 secondos. Every wake up, I test mqtt.loop() and if keep_alive is needed, send it. But here is my problem. Library uses millis() inside get and set timers functions but millis() doesn't return correct time due to sleep periods. My board has a RTC I could use with epoch time, for example, to keep time correctly.

Do you see it possible? Could you follow me how would you do? I have been thinking on put another callback inside MqttClient class where user could set lwmqtt_arduino_timer_get and lwmqtt_arduino_timer_set.

Thanks and regards.

256dpi commented 5 years ago

Thanks for raising this question. I was always wondering what it would need to make this library work in deep sleep scenarios.

My counter idea would be to expose only the clock source as a single callback and leave the timer management. I assume that calling the RTC is slow and would degrade the performance if called repeatedly. Therefore I recommend to just the synchronize the internal clock with the RTC using some kind of offset:

void setup() {
  // ...
  client.setClockSource(customMillis);
  // ...
}

uint32 customMillis() {
  static uint32 offset = 0;

  // synchronize a custom counter with an external RTC clock on wake up
  if(didWakeup) {
   offset = rtcMillis() - millis();
  }

  return millis() + offset;
}

uint32 rtcMillis() {
  // get current time (epoch) and return it in milliseconds (*1000)
}

Do you think this could work?

jmpmscorp commented 5 years ago

Hi Jöel and thank you for reply, :).

I was thinking about it too and your "clock source" idea seems the perfect one. It would be easy to set only clock source callback instead both "set" and "get" timers.

As you mentioned, we could set volatile flag in every wake up to know when we should update the offset.

Would you put clock source callback inside MqttClient class?

jmpmscorp commented 5 years ago

Hi again @256dpi.

I have been reading your code and thinking about how to achieve sleep feature following your idea. As I'm not C++ guru I want to write here my code idea and, if you agree, I'll try it and I'll send you a PR.

// Modify lwmqtt_arduino_timer_t to allow customMillis() callback
typedef uint32_t ( * lwmqtt_arduino_custom_millis_cb) ();

typedef struct {
  uint32_t end;
  lwmqtt_arduino_custom_millis_cb customMillisCb;
} lwmqtt_arduino_timer_t;

inline void lwmqtt_arduino_timer_set(void *ref, uint32_t timeout) {
  // cast timer reference
  auto t = (lwmqtt_arduino_timer_t *)ref;

  // set future end time
  t->end = t->customMillisCb != nullptr ? 
         (uint32_t)(t->customMillisCb() + timeout) : (uint32_t)(millis() + timeout);
}

inline int32_t lwmqtt_arduino_timer_get(void *ref) {
  // cast timer reference
  auto t = (lwmqtt_arduino_timer_t *)ref;

  // get difference to end time
  return t->customMillisCb != nullptr ? 
        (int32_t)t->end - (int32_t)t->customMillisCb() : (int32_t)t->end - (int32_t)millis();
.......

class MQTTClient {
  // ... Other variables
  lwmqtt_arduino_timer_t timer1 = {0, nullptr};
  lwmqtt_arduino_timer_t timer2 = {0, nullptr};

  //... Rest of code
  void setClockSource(lwmqtt_arduino_custom_millis_cb customMillisCallback) {
    timer1->customMillisCb = customMillisCallback;
    timer2->customMillisCb = customMillisCallback;
  }
}

Finally, in arduino sketch file, we could use the code you posted before and refresh customMillis with RTC. We could use this feature with an external RTC in any avr board or any samd board with its internal RTC.

Would you modify or improve anything on my code? I want to know your opinion, :D

PAk-CatchFire commented 5 years ago

Hi, I am interested in get the topics subscribed after a deep sleep....would you mind to complete the code? In specific: uint32 rtcMillis() { // get current time (epoch) and return it in milliseconds (*1000) }

regards

jmpmscorp commented 5 years ago

Hi.

I have been working on this feature as @256dpi adviced me. I have been testing it with an avr compatible board (1280p) + Xbee Wifi + DS3231 succesfully and just now, I'm working on an example with Arduino UNO + eth shield (W5100) + DS1307 RTC. If this last test works succesfully, I'll send a PR.

PAk-CatchFire commented 5 years ago

Very cool!! Do you have an ETA? Will it work on an ESP32? Thank you

jmpmscorp commented 5 years ago

@PAk-CatchFire I have just sent a PR with this feature but it has problems with Travis on adafruitBoard examples. I have been waiting that @256dpi could check it because I haven't modified anything on them.

Excuse me, I have tested it with AVR and SAMD arduinos because I haven't any ESP32 board. I expect it works on your board. I only added two callbacks on timers.

If you want, while @256dpi check my PR, you could test code in my branch: https://github.com/jmpmscorp/arduino-mqtt

Regards.

PAk-CatchFire commented 5 years ago

Thank you, will try ASAP. Can you provide an example for the sketch as well? Regards

jmpmscorp commented 5 years ago

Of course @PAk-CatchFire.

In my fork branch, there is a new example called ArduinoEthernetShieldSleep. As you wanted to test on ESP32, I expect you could adapt it easylly. At start of example, there is a brief explanation about code works.

Also, in the readme, there is information about new feature. You could find it searching for: Register a custom clock source ("millis source") callback:

256dpi commented 5 years ago

@jmpmscorp Thanks for putting this together I will review it as soon I have time!

PAk-CatchFire commented 5 years ago

Ok, waiting for the ESP32 example as @256dpi suggested!! Great work both of you!!

PAk-CatchFire commented 5 years ago

Hi @jmpmscorp and @256dpi ...do you have any more news about this issue? Best regards

jmpmscorp commented 5 years ago

Hi @PAk-CatchFire.

While you wrote your post, I was working on it. I have had hard week at work and I couldn't put my attention on issue.

I have provided new example based on MKRGSM1400. As I wrote here and on PR thread, I haven't any ESP board. Maybe, when @256dpi check and test my PR, he could provide an ESP example.

Regards.

256dpi commented 5 years ago

I just cherry picked the custom clock source patch from @jmpmscorp's fork. I will cut a release later today, then everyone can test it and work on some examples. @jmpmscorp Can you rebase your PR to just contain the example code?

jmpmscorp commented 5 years ago

Done it, @256dpi !

PAk-CatchFire commented 5 years ago

Hello again. When could I expect to find an example for an ESP32, since the uC only executes setup after a deep sleep (No loop). Is the pull already integrated? Regards

PAk-CatchFire commented 5 years ago

Hi again @jmpmscorp, I see that you are from Spain (me too). I have some spare ESP32 boards, I can ginve/send one to you, so you can work on an ESP32 Deep Sleep example. Would you be interested? Regards

jmpmscorp commented 5 years ago

Hi @PAk-CatchFire again.

I have been looking for your email on github but I can't find it. Feel free to write me to ...

Regards.

PAk-CatchFire commented 5 years ago

Done!!