Better to have multiple timers or multiple tasks?

[petkusj]

Dear Michael,

Is it better to have multiple timers with one task apiece, or one timer with multiple tasks? I ask because I can't reliably set multiple tasks and cancel them individually. The code below fires phasers and photon torpedoes on a model starship. I want to "randomly" alternate firing phasers and torpedoes with each press of a button, for a total of four weapons fire: one phaser, three torpedoes (port and starboard); one torpedo, one phaser, two torpedoes; etc.

I'm using the LedFlasher library from Nick Gammon to fire the phasers, but I use your timer to tell it when to stop. (I've also tried disabling LedFlasher.) I also use your timer to repetitively increase the intensity of the torpedo effect, and again to ramp down the effect. I've tried it with one timer and assigning a task without obtaining an identifier, and just doing a blanket cancel when I want to end the task. And I've also created multiple tasks and using systemTimer.cancel(phasTask) and systemTimer.cancel(torpTask).

Whatever method I try, it only seems to work for two calls to the randomFire function. After that, the fireTorpedo function or firePhaser functions act unreliably. The randomFire function, for instance, just keeps firing torpedoes endlessly.

I've also tried instantiating the timer with Timer<2> systemTimer, but that causes a different problem — the phaser fires endlessly.

I'd appreciate any suggestions. Tomorrow I'll try creating multiple timers with one task apiece. Ultimately I have four tasks for this sketch, to control spotlights, thrusters, phasers and torpedoes. But for now, I'd just be happy to handle two tasks.

Thanks,

Jennifer Petkus

#include <arduino-timer.h>
#include <LedFlasher.h>

const int CS_PIN = 10;
// Audio out = pin 9, declared in setup

const int WEAP_BTN = 8;
const int TORP1_PIN = 6;
const int TORP2_PIN = 5;
const int PHAS_PIN = 4;


auto systemTimer = timer_create_default();

/* Next value is a very loose approximation of the time
for a torpedo launch burst. Torpedo brightness increases
until it reaches threshold value, goes to 255, and then fades */
const int TORP_DUR = 1500/255;
const int TORP_THRESH = 64; // when torp launcher switches to full bright
int torpInt = 0; // current brightness of torp launcher
int torpCnt = 0; // port or starboard launcher
int fireCnt = 0; // how many times weapons fired
int fireLmt = 4; // max number weapons fire
bool weapFlag = 0; // prevents overlapping fire
bool warpFlag = 0; // prevents overlapping impulse/warp triggers
bool phasFlag = 0;
const int PHAS_LOW = 250;
const int PHAS_HIGH = 1500;

LedFlasher phasers(PHAS_PIN, 50, 100); // phaser flicker effect

void setup() {
  pinMode(TORP1_PIN, OUTPUT);
  pinMode(TORP2_PIN, OUTPUT);
  pinMode(WEAP_BTN, INPUT);

  phasers.begin();
}

void loop() {
  systemTimer.tick();
  if (digitalRead(WEAP_BTN) == HIGH && !weapFlag) { // prevents overlapping fire and firing in spacedock
  	weapFlag = 1;
  	randomFire();
  }
  if (phasFlag) {
  	digitalWrite(PHAS_PIN, HIGH);
    phasers.update();
  }
}

void randomFire() {
  if (fireCnt < fireLmt) {
    if (random(0,2) == 1) {
	    phasFlag = 1;
      firePhaser("fire");
    } else {
      systemTimer.every(TORP_DUR, fireTorpedo, "ramp");
    }
    fireCnt++;
  } else {
    fireCnt = 0;
  	weapFlag = 0;
  }
}

void fireTorpedo(const char *which) {
  if (which == "ramp") { // slowly lights LED until …
		if (torpInt > TORP_THRESH) { // … flash after threshold
      systemTimer.cancel();
			torpInt = 255;
			systemTimer.every(TORP_DUR/2, fireTorpedo, "fade");
		} else {
	    torpInt++;
	  }
  } else if (which == "fade") {
    if (torpInt <= 0) {
      if (torpCnt == 0) {
        systemTimer.cancel();
        systemTimer.every(TORP_DUR, fireTorpedo, "ramp");
        torpCnt++;
        torpInt = 0;
      } else if (torpCnt == 1) {
        torpCnt = 0;
        randomFire();
      }
    } else {
      torpInt--;
    }
  }
  int cur_pin = 0;
  if (torpCnt == 0) { // switches between port / stbd launcher
    cur_pin = TORP1_PIN;
  } else if (torpCnt == 1) {
    cur_pin = TORP2_PIN;
  }
  analogWrite(cur_pin, torpInt);
}

void firePhaser(const char *which) {
  if (which == "fire") {
    systemTimer.in(500, firePhaser, "stop");    
  } else if (which == "stop") {
    systemTimer.cancel();
  	digitalWrite(PHAS_PIN, LOW);
    phasFlag = 0;
    randomFire();
  }
}

[petkusj]

H'm, I did try creating multiple timers this morning without any improvement, so I think my problem has nothing to do with the timers. It's quite baffling. I will just go back to banging my head against the table. That usually works.

Jennifer

[contrem]

Hi Jennifer,

After a quick look, I see a few issues with your code:

1. Your timer handler functions need to return a boolean. I.E. void fireTorpedo() should be bool fireTorpedo() and it should return true or return false depending on if it should continue to trigger. Weird things can happen if you don't return a value when one is expected.
2. You're using == to compare char *. This doesn't work how you expect. It may be working for you now because the compiler is consolidating all equal strings to the same memory location, but normally you'll have to use the strcmp function, or use std::strings instead of char *.
3. You're adding a lot of tasks to the timer. Make sure in and every are not returning NULL to ensure the timer isn't already full

I suggest adding some serial prints to help you debug.

[petkusj]

Thanks so much, Michael! I'm new to C++. I'm more familiar with php, Javascript and ActionScript.

I did add serial prints to debug but I removed them for clarity in the code I posted. What seems to happen is that canceling the tasks works exactly twice and no more. I'll add in the return booleans and use strcmp — that's an example of my experience with less strictly typed languages. Not sure of how to ensure the timer returned null. Am I testing whether the task identifier is null, or call empty()?

Whatever, at least I can stop banging my head for a while.

Jennifer

[philj404]

@petkusj ,

Welcome to the quirks of C and C++! Nice project by the way.

Michael's suggestions are all good.

Another thing you may want to do is confirm that your phaser/torpedo task is FINISHED before starting another one (or cancel it). My first attempt to adjust a "blink" sketch kept blinking faster and faster as more tasks were added...

Something else that might be confusing you is that systemTimer.cancel() cancels ALL tasks, and sytemTimer.cancel(myTask) cancels your requested Task. But cancel() does not do anything about any old Task values you may have saved somewhere. They become obsolete, and when other Tasks get scheduled your obsolete reference may start pointing to them. It is best to set your saved reference to NULL on cancellation so that your systemTimer will recognize it should be ignored.

[philj404]

Oh... and back to your original question -- One timer is usually enough, as it can manage many different Tasks. You may need to give it more capacity, but that's adjustable.

If you need a different timeframe -- for example to support both microseconds and milliseconds delays -- you may want to add a second Timer.

[petkusj]

Thanks, Michael and Phil. I've made an even more stripped down version of the sketch (got rid of torpedoes) and it confirms I can only use a task two times before it stops working.

#include <LedFlasher.h>
#include <arduino-timer.h>

const int WEAP_BTN = 8;
const int PHAS_PIN = 4;

auto sysTmr = timer_create_default();
Timer<>::Task phasTsk;

int fireCnt = 0; // how many times weapons fired
int fireLmt = 4; // max number weapons fire
bool weapFlag = 0; // prevents overlapping fire
bool phasFlag = 0;

LedFlasher phasers(PHAS_PIN, 50, 100); // phaser flicker effect

void setup() {
 	Serial.begin(9600);
  pinMode(WEAP_BTN, INPUT);
  phasers.begin();
}

void loop() {
  sysTmr.tick();
  if (digitalRead(WEAP_BTN) == HIGH && !weapFlag) { // prevents overlapping fire and firing in spacedock
  	weapFlag = 1;
  	randomFire();
  }
  if (phasFlag) {
    phasers.update();
  }
}

void randomFire() {
  Serial.println("random");
  if (fireCnt < fireLmt) {
		phasFlag = 1;
		firePhaser("fire");
    fireCnt++;
  } else {
    fireCnt = 0;
  	weapFlag = 0;
  }
}

bool firePhaser(const char *which) {
  if (strcmp(which, "fire") == 0) {
   	Serial.println("fire");
   	unsigned long duration = 500;
   	Serial.print("phasTsk: ");
   	Serial.println(phasTsk);
   	Serial.print("number of tasks: ");
	  Serial.println(sysTmr.empty());
   	Serial.print("timer size: ");
	  Serial.println(sysTmr.size());
    phasTsk = sysTmr.in(duration, firePhaser, "stop"); 
  	return true;
  } else {
   	Serial.println("stop");
   	Serial.print("timer size: ");
	  Serial.println(sysTmr.size());
   	Serial.print("phasTsk: ");
   	Serial.println(phasTsk);
    sysTmr.cancel(phasTsk);
   	Serial.print("phasTsk: ");
   	Serial.println(phasTsk);
    phasTsk = NULL;
   	Serial.print("phasTsk: ");
   	Serial.println(phasTsk);
  	digitalWrite(PHAS_PIN, LOW);
    phasFlag = 0;
    randomFire();
	  return false;
  }
}

And here's the output of the Serial.println's:

random
fire
phasTsk: 0
number of tasks: 1
timer size: 0
stop
timer size: 1
phasTsk: 529
phasTsk: 0
phasTsk: 0
random
fire
phasTsk: 0
number of tasks: 1
timer size: 0

So "random" shows every time the randomFire function is called: twice. I never have more than one task and it looks like the task is removed after the first time I stop the phaser fire.

Thanks, Phil, for saying I'm good with one timer and multiple tasks, instead of multiple timers with a single task.

Question: is a timer created with an automatic task?

Well, it's back to banging my head on the table. I could just go back to reading millis(), but that way lies madness.

Jennifer

[contrem]

The problem occurs because you are canceling a task from within itself. If you remove the following line

 sysTmr.cancel(phasTmr);

it should work as expected.

When using timer.in, once the handler has been executed, it is automatically removed/canceled. This is similar to a double-free error. Canceling a task from within itself when it will be canceled automatically upon termination can cause problems. This problem could also arise if you cancel an every task from within itself.

Your current flow is

loop 
-> randomFiire
----> firePhaser(fire)
--------> queue up firePhaser(stop)
--------> return to randomFire
----> return to loop

loop
-> tick (within phasTsk)
----> firePhase(stop)
--------> cancel(phasTsk) *** ERROR canceling phasTsk within phasTsk
--------> randomFire
------------> queue up firePhaser(fire)
------------> return to firePhase(stop)
--------> return to tick
----> internal cancel(phasTsk) *** ERROR double cancel
----> return to loop

At the second ERROR point, it's actually canceling the new task that you added, and thus it's not triggering.

Question: is a timer created with an automatic task?

I'm not sure what you're asking here, but hopefully the above explanation solves your issue.

[petkusj]

Ah, brilliant! Thanks, Michael. That works for the timer.in. Although for the torpedo effect, I was using a timer.every because I need it to progressively increase the brightness of an LED. So I shouldn't cancel that task, but simply redefine it?

And I was asking about the automatic task because when I called sysTmr.empty() it returned 1, even though I hadn't yet declared a task yet. No big deal, just curious.

Again, thanks very much. I will try this out.

Jennifer

[petkusj]

Yes! That did it!

Now I just need to go back to the full sketch and slowly start turning things back on. I'll report back once I get it all working again.

Thanks, and more thanks!

Jennifer

[contrem]

when I called sysTmr.empty() it returned 1

bool empty() returns true if the timer has no tasks, in other words the timer is empty, and false if it's not empty. When you print true as an integer it converts into a 1, and false into a 0.

size_t size() will return the number of active tasks in the timer.

[petkusj]

UPDATE: Although I had success with my simplified phaser sketch, I was again having trouble once I added the photon torpedoes function back in. The phasers function could just use in(), but I was using every() for the torpedoes and I was getting inconsistent results. I also didn't know whether to return true or false after I canceled a task. I'd either try not to cancel and retask, or change the return value, with each combination giving me a different result.

So I gave up and switched the torpedo call to sysTmr.in(), which I had to repetitively call until the torpedo effect was over. The advantage being that I never needed to cancel the task because by definition once the function was called the task was done.

I think every() is best used when it is an endless repetition and to cancel it sparingly.

Anyway, I've finally got it working, although there were still some residual problems. I found out that it's not a good idea to separately call a function that a task will call. Once I figured that out, however, it seems to be pretty reliable. Thanks again for all the help.

Jennifer