Posting my heartbeat code - it mostly works, but it's buggy. Any ideas?

Troubleshooting
1

So I guess there’s two reasons for posting this. One is in case anyone in the future might benefit from this code, which is ‘my’ implementation of a heartbeat check between Particle devices. Big thanks to @BulldogLowell, @ScruffR, @nrobinson2000, @Grenello for their massive helps, various.

Secondly, it’s in case anyone can help me work out why my boolean flag “RemoteFail” is seemingly never ‘true’.

I should point that that the code does work, to a point, it’s just the second check I wanted to get working if possible.

INO

#include "VineAlert.h"

using namespace RetailAlert;

SYSTEM_THREAD(ENABLED);

// PUB-SUB constants
constexpr char * DEVICE_NAME = "ALERT";
constexpr char * Remote_FAIL_MSSG = "COMMS FAIL FROM Remote";
constexpr char * Remote_FAIL2_MSSG = "2nd COMMS FAIL FROM Remote";

int slidingstate = 3;
int Remotekeepstate = 3;
boolean RemoteFail = false;

MillisTimer HeartbeatRemote(960000, HBRemoteCallback, false);
MillisTimer ResetHBRemote(3600000, ResetRemoteCallback, false);



void eventHandler(const char * event,
        const char * data);

void printDetail(uint8_t type, int value);


void setup() {
        Particle.subscribe(DEVICE_NAME, eventHandler, MY_DEVICES);
        
		HeartbeatRemote.begin();
		ResetHBRemote.begin();
		HeartbeatRemote.start();
}

void loop() {

        MillisTimer::processTimers();
        static unsigned long timer = millis();

}

void HBRemoteCallback(void) {
        
        if (RemoteFail == false) {
                HeartbeatRemote.stop();
				RemoteFail = true;
                Particle.publish(DEVICE_NAME, Remote_FAIL_MSSG, 60, PRIVATE);
				HeartbeatRemote.start();
				ResetHBRemote.start();
        }
        else if (RemoteFail == true) {
                HeartbeatRemote.stop();
                Particle.publish(DEVICE_NAME, Remote_FAIL2_MSSG, 60, PRIVATE);
				RemoteFail = false;
        }
        
}

void ResetRemoteCallback(void) {
RemoteFail = false;
        
}




void eventHandler(const char * event,
        const char * data) {

     	if (sscanf(data, "K%d, S%d", & Remotekeepstate, & slidingstate)) {
			     HeartbeatRemote.stop();
				 HeartbeatRemote.start();

		}
}

.H

#ifndef RETAILALERT_H
#define RETAILALERT_H

#include "Particle.h"
#include <vector>

constexpr uint16_t DEBOUNCE_TIME = 2500;  //milliseconds

template <class T> inline Print& operator <<(Print& obj, T arg)
{
  obj.print(arg);
  return obj;
}

namespace RetailAlert{

  using functionPtr = void(*)(void);


  class MillisTimer{
    public:
      MillisTimer(uint32_t time, functionPtr action, bool repeat);
      static void processTimers(void);
      void start(void);
      void start (bool repeat);
      void stop(void);
      void begin(void);
    private:
      enum TimerState{
        STOPPED,
        RUNNING,
      }state = STOPPED;
      functionPtr callback;
      uint32_t delay;
      uint32_t lastMillis = 0;
      bool repeats;
      static std::vector<MillisTimer*>timerArray;
  };

};

#endif

.CPP

#include "Particle.h"
#include <vector>
#include "VineAlert.h"

// ButtonPressAction

using namespace RetailAlert;



// MillisTimer

std::vector<MillisTimer*> MillisTimer::timerArray;

MillisTimer::MillisTimer(uint32_t milliseconds, functionPtr action, bool repeat) {
  //timerArray.push_back(this);
  delay = milliseconds;
  callback = action;
  repeats = repeat;
}

void MillisTimer::begin(void) {
  timerArray.push_back(this);
  Serial << "Timer Added" << "\n";
}

void MillisTimer::processTimers(void) {
  uint32_t currentMillis = millis();
  int count = 0;
  for (auto& i : timerArray)
  {
    if(currentMillis - i->lastMillis >= i->delay)
    {
      if (i->state == RUNNING){

        if(i->callback){
          i->callback();
        }
        if (!i->repeats){
          i->state = STOPPED;
        }
        else {
          i->lastMillis = currentMillis;
        }
      }
    }
  }
}

void MillisTimer::start (void){
  lastMillis = millis();
  state = RUNNING;
  Serial << "Timer Start\n";
}

void MillisTimer::start (bool repeat){
  repeats = repeat;
  lastMillis = millis();
  state = RUNNING;
  Serial << "Timer Start\n";
}

void MillisTimer::stop(void){
  lastMillis = millis();
  state = STOPPED;
}

Many thanks!

2

@daneboomer, I can’t speak to the RemoteFail logic issue but your approach seems overly complex with the MillisTimer class. Personally, I would have used two Software Timers which you can start/stop/reset and, when they expire, would set a flag to be sampled in loop(). When the flags are set, loop() can call the appropriate callback function.

3

@peekay123 I’m confused. Aren’t I essentially doing exactly that? Isn’t the “millistimer” a software timer? And am I not doing essentially what you suggest in my code logic?
Hmm. I will do some people investigating! But if nothing else your suggestion to try a different timer is a good one as it might resolve my issue! Thank you.

4

@daneboomer, the Software Timers are part of the DeviceOS and use a FreeRTOS thread to run. So all the work is done for you. The MillisTimer class is an unnecessary extra IMO.

1 Like
5

Seemingly is the key word here. Putting in some print statements shows that RemoteFail does in fact become true the first time HBRemoteCallback is called. Something is wrong with the MillisTimer code, because even though you start the HeartbeatRemote timer again, the callback is never called for a second time. I'm not quite sure why (it's too late here for me to figure it out), but switching to Particle's software timers should solve the problem.

4 Likes
6

Thanks, @Ric - I will modify my code to use the Particle software timer. Thank you!

7

I've converted my script to use the built in timer (NOW I remember why I was using the MililsTimer class, this code has been ported from a Core...) and I'm wondering if this is actually true - doesn't seem to reset.

If start() is called for a running timer, it will be reset.

8

What Device OS version have you tested this with?

uint32_t restart = 2000;
uint32_t lastRestart = 0;

Timer toggle(1000, toggleLED);

void setup() {
    Particle.function("setInterval", setInterval);

    pinMode(D7, OUTPUT);
    toggle.start();
}

void loop() {
  if (millis() - lastRestart < restart) return;
  lastRestart = millis();
  toggle.start();            // with interval < 1000 LED should NOT toggle
}

void toggleLED() {
    digitalWrite(D7, !digitalRead(D7));
}

int setInterval(const char* arg) {
    restart = constrain(atoi(arg), 100, 10000);
    return restart;
}

With this test code (on 0.7.0 and 0.8.0-rc.10 - and most likely most other versions) you can check that timer.start() actually does reset the timer.
When you set the restart interval less than the timer interval you will see the D7 LED stopping to toggle, which would not be the case if timer.start() didn’t reset the timer.