Strange behavior with delay()

Using Arduino Programming Questions
#1

Every now and again, it looks as though delay isn't working correctly.
I've seen this several times,
I have a sequence:

digitalWrite( BlueLed, HIGH);
delay(2);
digitalWrite( BlueLed, LOW);

I should only see a momentary flash of the LED.
Every now and then, I see it stay statically on
for about 1 second.
After some delay, it starts working as normal again.
I do have another interrupt connected to a 1 second square wave
from a clock chip. It reads micros() and set two variables. Not a
long interrupt.
I wondering if delay() might have a problem with being interrupted or it
it has a problem in implementation or I just have a bad processor.
Dwight

#2

Without more information (context, source, ...)
only imagination limits the number of valid guess.

Bad processor ranges very low in my list.

If delay really had a problem, it would have been fixed,
nearly each beginner tests delay extensively,
so it is probably one of the best tested functions.

#3

I doubt that you can see a 2ms flash. I would guess that loop is executing this many times and when you think it is "on" it is really just half-bright.

#4

I can. Try. Depends on the brightness of the led.

#5

It could very well be that under normal operating conditions,
(if there is very little in void loop{})
then:

digitalWrite( BlueLed, HIGH);
delay(2);
digitalWrite( BlueLed, LOW);

gets executed, the pin is turned LOW then HIGH immediately which would invalidate ever
having a digitalWrite( BlueLed, LOW); line.

Then when the interrupt is called, it takes about 17ms or more to execute which causes the momentary blink and has nothing to do with the loop code.

Normally for blinks you have an extra delay.

digitalWrite( BlueLed, HIGH);
delay(2);
digitalWrite( BlueLed, LOW);
delay(2);

But, too tough to say without extra info.

Otherwise the brain has a hard time seeing flicker past 60Hz, so anything less than 17ms is incredibly difficult to pick up.

#6

@dwightthinker, post your complete program if you want useful advice rather than guesses.

…R

#7

I can publish the rest of my code but I don't think it would make much difference.
Visually 17ms is hard to deal with but it is not outside what the brain can handle.
I have an oscilloscope and can tell you that I can clearly see a 2ms flash.
I might not distinguish a 2 from a 3ms but I can clearly see the flash.
Years ago, I worked for a company called "Speed Call" They made commercial
touch tone equipment. We sent a 15ms tone burst.
On a speaker, I got so I could clearly tell a 14 ms from a 15 ms or a 16 ms.
That was audio but your right, the visual is not as exact.
I use the LED flash as a strobe light and for what I'm doing it works quite well.
I only rarely see the problem. It is on the order of an hour or so between seeing it.
The light will sometimes just stop not lit and then after a second or so continue
as if nothing happened. Once it happened right in the middle of the sequence
you I posted. The light stayed on about a second, then went off and about a second
or so later went back to normal operation. ( flash the light every .66666.. seconds )
I know the processor is not resetting because I don't see the init sequence happening.
For what I'm doing, the occasional shift of a few milliseconds is not noticed but the
long term effects of such would be.
I've written my code to self compensate for most such errors.
I've check the overall timing with both a scope and a calibrated frequency counter.
At times the light didn't blink and then restarted, I'd thought could be something
in my code. Holding the light on is not something I could do in my code without
delay() not working for some reason. I know it wasn't just running through my loop
real fast because I have another light that would alternately be blinking as well ( red ) for
a 2ms as well.
I also blink the boards LED at the rate of .3333 seconds and it was not blinking.
I'll post my code later when I can.
Please excuse some of the sloppiness. It is still in a state of being updated
and changed.
Dwight

#8

I know you guys love to see code.
Here is what I have right now. It has a fixed BPH but that will
be some more init code I’ll be adding. The fix rate works fine
for the current code.

//clock calibrator
/************************************
 * This is used to calibrate a pendulum clock.
 * It does this with two strobes, on red and one blue.
 * The are intended to shine on the pendulum rod from about
 * 1 foot away from thee front of the clock.
 * 
 * A white piece of paper is to be placed behind the pendulum
 * such that the red and blue happen at the center of the swing
 * and cast a shadow on the paper. A mark on the paper forms
 * the reference.
 * To synchronize to the clock, hold the phase button down
 * and adjust the phase pot until the red shines with
 * its apex right at the edge of the pendulum, at the center
 * of the swing.
 * 
 */

#include <Wire.h>
#include <SPI.h>
// DS3231
const uint8_t Clock = 0x68;
const uint8_t CtrlReg = 0x0E;
const uint8_t SQW = 19;  // Square wave output of the DS3231, will be 1.00Hz
const uint8_t LED = 13;
//const uint8_t LED = 5;
const uint8_t RedLed = 7;
const uint8_t BlueLed = 6;
// SSD1103 display
const int ResetPin = 49; // for display
const int DC = 47; // data/command for display
// SPI MISO 50
// SPI CLK 52
// SPI SS  53
// SPI MOSI 51  Master out
int xDisp ;  // 0-131 incase SH1106
int yDisp ;  // 0-63
int pixelOnOff ; // -1 on, 0 off
uint8_t dispArray[ 1056 ];  // Array is global for easy access
int x = 0; // used a lot for screen
int y = 0;
uint8_t curLine = 0;
uint8_t curCol = 0;

const uint8_t LeftPot = A0;
const uint8_t RightPot = A1;


//  setup code
//  ***************
void setup() {
  initSpi ();  // Used to talk to display includes display init
  initSw();  // 8 bit dip switch for options, use readSW() to get value
  initClockChip(); // does I2C to clock chip and includes interrupt init
  initCal();
}

void initClockChip() {
  delay(3000);
  Wire.begin();
  // Turn on 1Hz square wave out
  Wire.beginTransmission(Clock);
  Wire.write(CtrlReg);
  Wire.write(byte(0x00)); // Start 1Hz output square wave.
  Wire.endTransmission();

  pinMode(SQW, INPUT_PULLUP); // 1 sec square wave
  pinMode(LED, OUTPUT);
  pinMode(RedLed, OUTPUT);
  pinMode(BlueLed, OUTPUT);

  attachInterrupt(digitalPinToInterrupt(SQW), OneSecISR, RISING);
  delay(2000); // give it a chance to have a couple interrupts
  initCal();
}

/**********************
 * Clock calibrator starts here
 *
 */

uint8_t LeftSw = 22 ;
uint8_t RightSw = 24 ;
volatile float fBPH = 10800.0;
volatile float fSecPerBeat = 0.0;
volatile unsigned long uSecPerBeat;
volatile long Beats;
volatile unsigned long uSecCpu = 0;
volatile unsigned long SecMark = 0;
volatile unsigned long LastSecMark = 0;
volatile unsigned long uSecErr = 0;
volatile unsigned long LastuSecCpu = 0;
volatile float fErrPerSec = 0.0;
volatile unsigned long CurMicros = 0;
unsigned long NxtStrb = 0 ;
int8_t TicToc = 0;

void initCal() {
  unsigned long tmp ;
  pinMode( LeftSw, INPUT_PULLUP );
  pinMode( RightSw, INPUT_PULLUP );

  fSecPerBeat = 3600.0 / fBPH;
  uSecPerBeat = long( 1000000.0 * fSecPerBeat );
  Beats = 0;
  LastSecMark = 0;
  while ( LastSecMark == 0 ) {
    CheckSecPast();
  };
  tmp = LastSecMark ;
  while ( LastSecMark == tmp ) {
    CheckSecPast();
  }; // wait two seconds to get variables initialized correctly
  tmp = LastSecMark ;
  while ( LastSecMark == tmp ) {
    CheckSecPast();
  }; // wait two seconds to get variables initialized correctly
  CurMicros = micros();
  NxtStrb = CurMicros + 1000000UL; // wait a second
}

unsigned long center;
unsigned long Phase;
unsigned long PhaseSum ;

void PhaseCal() {
  if ( LeftSw == LOW ) {
    // ???
  }
}

// One second interrupt from DS3231
void OneSecISR() {
  uSecCpu = micros();
  SecMark = SecMark + 1UL;
}

void loop() {
  WaitNxtStrob();
  CheckSecPast();
  SetPhase();
}
unsigned long SaveTime;
unsigned long BeginTime = 0;
unsigned long EndTime = 1;

void LeftButCalc() { // used to display how much an dajustment makes
  if ( digitalRead(RightSw == LOW) ) {
  unsigned long RunTime;
  float fRunTime ;
  uint8_t SignFlag = (( PhaseSum & 0x80000000 ) != 0 );
    if ( SignFlag ) {
      PhaseSum = ~PhaseSum + 1; // Make it a positive number
    }
    float fPhaseSum = float( PhaseSum ) ;

    BeginTime = EndTime;
    EndTime = SaveTime;
    RunTime = EndTime + ~BeginTime + 1;
    int SecPerHrErr = int( 36000.0 * fPhaseSum / fRunTime ); // in tenths seconds
    // print err;
    PhaseSum = 1UL;
  } else {
    if ( PhaseSum == 1UL ) {
      PhaseSum = 0UL ;
    }
  }
}

void SetPhase() { // used to move the strobe around to match pendulum
  // doing signed math with unsigned values
  unsigned long tmp;
  if ( digitalRead(RightSw) == LOW ) {
    if (center == 0x500) { // it can never be 500H because 400H is full scale
      center = analogRead(RightPot); // starting position of potentiometer
      center = ~center + 1; // make negative for add but keep unsigned as value
    }
    NxtStrb = NxtStrb + ~Phase + 1; // remove previous phase
    tmp = analogRead(RightPot);
    Phase = (tmp + center) << 7 ;
    NxtStrb = NxtStrb + Phase ; // add new phase
    SaveTime = micros();
  } else {
    PhaseSum = PhaseSum + Phase ;
    center = 0x500;
    Phase = 0;
  }
}
void CheckSecPast() {
  // On the second  need to update error
  if ( SecMark != LastSecMark ) { // a second most have happened
    LastSecMark = SecMark ;
    uSecErr = uSecCpu - LastuSecCpu - 1000000 ; // The difference between the micros count and a real second
    LastuSecCpu = uSecCpu ;
    fErrPerSec = float( uSecErr ); // will want to do some math in floating
  }
}

void WaitNxtStrob() {
  CurMicros = micros();
  if ( NxtStrb < CurMicros ) { // time is up
    digitalWrite(LED, HIGH);
    if ( TicToc & 1 ) {
      digitalWrite(RedLed, HIGH);
      delay(2);
      digitalWrite(RedLed, LOW);
    } else {
      digitalWrite(BlueLed, HIGH);
      delay(2);
      digitalWrite(BlueLed, LOW);
    }
    digitalWrite(LED, LOW);
    SetNextStrobe();
    TicToc++;
  }
}

void SetNextStrobe() {
  long uErr = 0;
  Beats = Beats + 1;
  uErr = long( fErrPerSec * fSecPerBeat ); // use floats to avoid rounding
  //  DecPrint(long(1000 * fSecPerBeat), 4 );
  DecPrint(uErr, 3 );
  NxtStrb = NxtStrb + uSecPerBeat + uErr ;
  DecPrint(SecMark, 0);
}

Look for:

WaitNxtStrob()

and

OneSecISR()

For the things of possible interest.
I’ve removes some code related to the display because it complains about
over 9000 chars.
Dwight

#9

dwightthinker:
I

//clock calibrator

/************************************

  • This is used to calibrate a pendulum clock.

So, you want your pendulum clock to be as accurate as the resonator on your arduino. Slight problem with that…

NxtStrb = NxtStrb + ~Phase + 1; // remove previous phase

Isn't ~Phase+1 the same as -phase? Why not NxtStrb -= Phase; ?

#10

If you read carefully, the resonator is corrected.
Because I thought there might be a math error, I'd rewritten
the subtraction to ensure that it wasn't messing up on
corner cases.
The drift rate of the resonator is slow enough that a periodic
correction as I do in my code is good enough to keep the
timing within the limits of my calibrated counter.
Do note that I use the DS3231 as a constant reference.
The resonator in my board is about 300 us per second fast but varies
quite a bit with temperature.
Notice uSecErr in CheckSecPast().
It is used by SetNextStrobe() to make a correction based on the
rate of the particular pendulum.
Phase is just used to synch the strobe with the pendulum so that
it catches the swing at is fastest and most sensitive to change.
The DS3231 produces a 1 second pulse that is within about
2.5 us by my counter. That is within the range of error of my counter
so it might be better or slightly worse than that.
Dwight

#11

I should note that the Blue and Red LEDs were intended to be used to set the
beat of the clock ( term used for evenness of tic and toc ).
I found the flash and the sound of the clock to not be easy to correlate. I suspect
I'll keep either the Blue or Red for the timing and add a small
speaker for beat setting instead.
I use a cheap loupe for collimating the light so that I can place the strobe light
about a foot to a foot and a half without too much spread of the beam.
At a foot distance, I'm getting about a 2.5 inch spot with the sharpest
shadow of the pendulum.
I can get a smaller spot, by moving the LED relative to the loupe, but at the cost
of a poorer shadow edge.
Dwight

#12

@dwightthinker, could you provide a succinct english (not code) description of that your program is trying to do?

I know it has something to do with a clock and time correction, but I can't get my head around the project.

...R

#13 
  uint8_t SignFlag = (( PhaseSum & 0x80000000 ) != 0 );
    if ( SignFlag ) {
      PhaseSum = ~PhaseSum + 1; // Make it a positive number
    }

Huh?

How about:

PhaseSum = abs (PhaseSum); // Make it a positive number

if ( PhaseSum == 1UL ) {
      PhaseSum = 0UL ;

How about:

if (PhaseSum == 1)
  PhaseSum = 0;

    RunTime = EndTime + ~BeginTime + 1;

Are you entering some competition?

#14

Hi Nick
Most of the strange coding you’ve noted was my attempt
to figure out where the code was going bad.

As I said, it would sometimes just stop for a second or so
and then continue. I was thinking it had to be something
in the code that I was doing.
I was thinking that maybe A-B wasn’t the same as A + ~B +1 and
if I coded it in a way that there could be no error, maybe I’d find
the problem.

The while watching it, one time it stopped right in the middle of the
code that blinks the blue light. The blue LED stayed on for about a second
and then went out for about a second or so. After that things continued.
Since the only thing that could delay thing that could cause the blue led
to stay on was the delay() function, I made this post and the other
about subtraction of unsigned longs.

The code was in the middle of debugging to see what was causing the
strange behavior so things had been changed in places from what one
would think the simplest. Various print statements placed at places to
check values for debug.

It is not for a contest or such.
There are already several fine tools for timing clocks. The problem with
all of them is that they all require some method of connecting to the
clock.

Optical interrupters need to be closely spaced and are effected by ambient
lighting. They often are not compatible with the size and shape of the
pendulum.

Acoustic pickups are also problematic in that clocks make a lot
of different sound and require special filters and careful level adjustments.

It occurred to me that a strobe could be made to work if it had some
added features. The strobe has the advantage that it doesn’t need to
be connected to the clock.
One would set the phase of the strobe to
match the clock and let it run for some time, watching the drift.

One would then re-center with the phase and readout how much
time was gained or lost for that amount of phase change.
One needed an accurate strobe rate. This is the done with the math and the
DS3231 reference to the 1 second pulse to correct the resonator used to
time the processor.

It needed a way of adjusting the phase of the strobe to match the pendulum
so that one could set the strobe at a location where is was most sensitive to
a change in speed ( the center of the swing ).
I do this with a scaled value from a 10K pot.

One needs to be able to make a change of the clocks adjustment and know
how much it effected the rate ( coding in progress ). It has the math, I just need
to add the display code. I want the display to say FAST or SLOW. That is the
reason for separating the sign and your comment about the abs() function.

The part I’m planning to change is how one sets the evenness of the beat.
I’d thought to use the two colors of LED to set the beat. On experimenting,
I realized that the connection between blink and sound wasn’t going to work.
If the blink happened anywhere near the same time as the sound, the mind
seems to just connect the two together, as though they both happen at the
same time. For this I expect to add a speaker to make the tic and toc sound
and remove one light.

I have yet to add the code to set the BPH rate, for different clocks, as things
are still being worked on.
The main function is working fine except the strange delay behavior.
Dwight

#15

Dwight - I would love to help but reading your posts is painful. Can you please at least put blank lines between the paragraphs ?

#16

For Robin

OneSecISR() captures the one second clock from the DS3231 and the current micros() for correcting
the rate of the processor.

LeftButCalc() work in progress. The left button is used to both save the current phase settings
and the current time.
It also calculates the amount fast or slow that the total phase has been adjusted since the last
time the left button was pressed. This is to be displayed.

SetPhase() If the right button is pressed, the pot is used to lead or lag the strobe so that it catches
the pendulum right at the center of the swing. Since this is a sensitive adjustment, the range of the
pot is less than one cycle of the pendulum. Each time one presses the left button, the current position of the
pot is 0 phase. This way one can ratchet the adjustment to bring the strobe into range by holding the
button down, rotating the pot full, releasing the button and then counter rotation the pot.
I keep track of the amount of total time the phase is adjusted to be used by LeftButCalc.
I don't do a processor speed correction to this value since the error in this would be a fraction of a
fraction and not worth the hassle of maintaining. ( 300 us error per second error in processor rate ).

CheckSecPast() connects the information from the one second interrupt to an error value that
the code will later use to keep the strobe accurate. It needs to check that there wasn't an interrupt
in the middle of the read of uSecErr from the interrupt since I can't disable the interrupt while
doing this read and long values are not fetched as a single read ( multiple bytes ).

WaitNxtStrob() Waits for the next strobe. It flashes the strobe and then it calculates the time to the next one
based on the rate and the processor error.

SetNextStrobe() does the math for WaitNxtStrob

Dwight

#17

Now, that we've dissected the entire code, how about back to the
original problem.
How could the BlueLed stay on for about a second with the code:

digitalWrite(BlueLed, HIGH);
delay(2);
digitalWrite(BlueLed, LOW);

I'm thinking that my reading of micros() within the interrupt:

void OneSecISR() {
uSecCpu = micros();
SecMark = SecMark + 1UL;
}

may be the problem source. I'm not sure how the code in micros() deals with
making a read of the timer.
Might it be disrupting the delay() function, because I'm doing a micros() within
an interrupt?
What counters are used by micros() and delay()?
Dwight

#18

#19

dwightthinker:
I’m not sure how the code in micros() deals with making a read of the timer.

Have a look into wiring.c

unsigned long micros() {
	unsigned long m;
	uint8_t oldSREG = SREG, t;
	
	cli();
	m = timer0_overflow_count;
#if defined(TCNT0)
	t = TCNT0;
#elif defined(TCNT0L)
	t = TCNT0L;
#else
	#error TIMER 0 not defined
#endif

#ifdef TIFR0
	if ((TIFR0 & _BV(TOV0)) && (t < 255))
		m++;
#else
	if ((TIFR & _BV(TOV0)) && (t < 255))
		m++;
#endif

	SREG = oldSREG;
	
	return ((m << 8) + t) * (64 / clockCyclesPerMicrosecond());
}
#20

This has a potential problem with over flows:

*64 / clockCyclesPerMicrosecond()

This is why in Forth we have a special operator:

*/

that keeps a double sized intermediate value.
The problem is that I’m already working with long values.
In this case, it can be solved with a pre-calculated value
that can be set during init.
You’ll notice that in my code I avoid this issue by using floating
point for ratios, where I don’t always know how close to
overflowing I might get.

I’ll try using similar code, to yours, in my interrupt and see if the problem
goes away. Messy code for an interrupt though.
Thanks a bunch.
Dwight