Band-aid for delay() abusers

We often encounter code that abuses delay, and indeed long applications that have been written to do their primary task successfully with lots of delay() - and then they realize they need some trivial operation running in the background, say to blink a LED or something...

How heinous is the following coding pattern? (this would be accompanied by find/replace delay() with delay_bandAid())

void delay_bandAid(unsigned long ms) //copied from wiring.c
{
  uint16_t start = (uint16_t)micros();
  while (ms > 0) {
    doDelayTasks(); //this line added
    if (((uint16_t)micros() - start) >= 1000) {
      ms--;
      start += 1000;
    }
  }
}

void doDelayTasks() {
unsigned long t=millis();
if (t > ToggleLEDAt) {
     ToggleLEDAt+=500;
     PINA=0x80; //LED is connected to PA7
}
//and so on, making an effort to keep everything here pretty fast so it doesn't throw the delays off.
}

I don't happen to be in this position, but I had this thought while looking at some awful code today... Provided the code spends most of it's time in delay() and the delay tasks are trivial and not time-critical, this should be an acceptable band-aid, right?

this should be an acceptable band-aid, right?

No. Ctrl-A + Ctrl-X is the acceptable workaround for piss-poor coding practices.

My objection to your code is that you add time values. Addition is NOT guaranteed to work. Subtraction is.

Providing band-aids for poor programming is not doing anyone any favors.

Which IDE version are you using? This has already been added in version 1.5.1.

https://github.com/arduino/Arduino/blob/master/hardware/arduino/avr/cores/arduino/wiring.c#L111

void delay(unsigned long ms)
{
	uint16_t start = (uint16_t)micros();

	while (ms > 0) {
		yield();
		if (((uint16_t)micros() - start) >= 1000) {
			ms--;
			start += 1000;
		}
	}
}

Aaah! I did not realize that had been added...

DrAzzy:
How heinous is the following coding pattern? (this would be accompanied by find/replace delay() with delay_bandAid())

Well what will happen if there are 8 delay()s in the program each replaced with something that makes a call to a function called doDelayTasks().

Apart from the fact that the person struggling with delay() will probably also struggle with the idea of a function, it seems to me that the logic would be inside out.

It's not (IMHO) that you want to call tasks during a delay but, rather, that you don't want the "delay period" to be a task at all.

...R

oqibidipo:
Which IDE version are you using? This has already been added in version 1.5.1.

https://github.com/arduino/Arduino/blob/master/hardware/arduino/avr/cores/arduino/wiring.c#L111

void delay(unsigned long ms)

{
uint16_t start = (uint16_t)micros();

while (ms > 0) {
	yield();
	if (((uint16_t)micros() - start) >= 1000) {
		ms--;
		start += 1000;
	}
}

}

Not a quantum improvement, since yield() must be non-blocking anyway (just as another tested section of a polled circular multitask alternative would have to be). But it was thoughtful.

PaulS:
No. Ctrl-A + Ctrl-X is the acceptable workaround for piss-poor coding practices.

My objection to your code is that you add time values. Addition is NOT guaranteed to work. Subtraction is.

Providing band-aids for poor programming is not doing anyone any favors.

Where he added it IS guaranteed to work since it will always stay behind of the current time. Work it out and you'll see. You can add to a time in the past to catch it up to the now. You CANNOT add to the time now and expect it to accurately represent the future.

It's the same as saying previousTime = currentTime except that it handles the case where you've missed checking the time and you've gone one or two milliseconds longer than your interval. Doing it this way will make sure your next interval is a little shorter to make up for it and the timing hits where it would have had you checked it right at the interval. Doing it with previousTime = currentTime handles the case where you want to make sure the next interval is at least as long as it should have been.

It makes a difference if for instance you are running a clock and want to update on the second. If you miss and your timing code gets called at 1002ms once because a serial transmission came in and you had to handle it or something then all the rest of your timing points get moved forward in time by that amount. This way you shorten up the next interval and the timing points stay in the same place.

Work it out and you'll see.

I looked at it again.

I'm not convinced that adding time is a good idea. I AM sure that subtracting then from now, to determine if it is time to do something WILL always work.