Irregularly-timed digital output despite code instructing otherwise

Hello,

To contextualize my issue, a bit of background on my project might be helpful. My total set up includes a button (which could be pressed, in the form of a lever inside a Skinner box), an Arduino Uno board, and an Intan (a stimulator used for the delivery of electrical pulses).

My intention was to write a program that instructs the Intan (via a digital-in B/C cable) to “fire up” a pre-defined waveform for a certain number of times and with a certain duration (in milliseconds) of inter-pulse delay.

For example, on the first lever press, I want the Intan to deliver 10 pulses, each 3 milliseconds apart. On the second subsequent lever press, I want the Intan to deliver 20 pulses, each 1.5 milliseconds apart, etc.

The issue I’m facing: on any given lever press, the inter-pules period between the “grouped” pulses is not regular—i.e., for a command that specifies 10 pulses 3 ms apart, some end up being less than 3 ms, some more. I’ve repeatedly checked my code, but I’m at my wits end at this point. I’d appreciate it if I could get some feedback on what’s going wrong with my code.

int ledPin = 6;
int buttonPin = 4;
int buttonCounter = 1;
int i;
int buttonRead;
unsigned long buttonTimeNow;
unsigned long buttonTimePrior=0;
unsigned long debounceDelay=50;
//{number of pulses, inter-pulse period}:
int motherMatrix [10][2] = {
  {167, 3},
  {125, 4},
  {100, 5},
  {83, 6},
  {71, 7},
  {63, 8},
  {56, 9},
  {50, 10},
  {45, 11},
  {42, 12}

};
//change this for a new list
int numCommands=10;


void setup() {
  // put your setup code here, to run once:

  pinMode(ledPin, OUTPUT);
  pinMode(buttonPin, INPUT);
};

void loop() {
  // put your main code here, to run repeatedly:

  buttonRead = digitalRead(buttonPin);



//debouncing:
  if (buttonRead==HIGH){
    buttonTimeNow=millis();
  };
  if (buttonTimeNow-buttonTimePrior>debounceDelay){
    buttonRead=digitalRead(buttonPin);
    buttonTimePrior=buttonTimeNow;
  }
  else {
    buttonRead=LOW;
  };
//

//setting up repeats when the number of lever presses exceeds the number of stored commands:
  if (buttonCounter<=numCommands){
    buttonCounter=buttonCounter;
  }
  else if (buttonCounter>numCommands){
    buttonCounter=1;
  };
//




  if (buttonRead == HIGH) {
    for (i = 1; i <= motherMatrix[buttonCounter - 1][0]; i++) {
      digitalWrite(ledPin, HIGH);
      delayMicroseconds(1);
      digitalWrite(ledPin, LOW);
      delay(motherMatrix[buttonCounter - 1][1]);
    };
    buttonCounter++;
  }
  else {
    digitalWrite(ledPin, LOW);
  };

//making sure the button is released before proceeding:
while (buttonRead==HIGH){
    buttonRead=digitalRead(buttonPin);
};
};

How is the input wired ? Do you have a pulldown resistor in place to keep the input LOW when the button is not pressed or is it floating at an unknown, possible HIGH voltage ?

      digitalWrite(ledPin, HIGH);
      delayMicroseconds(1);
      digitalWrite(ledPin, LOW);

Does turning the LED on for 1 millionth of a second achieve anything ?

Why have you terminated so many lines in the code with unnecessary semicolons ?

Firstly, thank you for responding.

Why so many semicolons: because the programming app instructed me to.

The ledPin is actually the output to the Intan; I just call it ledPin.

The input is wired as follows:
The lever in the SB is the button (pressing it presses the button), the output from the button then goes to pin 4.

Please let me know if I there's anything else I could provide.

From the micros() reference:

On 16 MHz Arduino boards (e.g. Duemilanove and Nano), this function has a resolution of four microseconds (i.e. the value returned is always a multiple of four).

So,

delayMicroseconds(1);

Will not do what you want.

Doing timing with delay() and delayMicroseconds() will not be very accurate as the other stuff that the program is doing will effect the timing. Better to use millis() and micros() for timing as described in the begiiner’s guide to millis tutorial and the several things at a time post.

};
};

The semicolons after the closing curly brackets are not required (except in certain circumstances).

Got it, thank you. I'll change it up and return back.

Just as a quick question, when you mentioned that the resolution of the Arduino 16MHz boards (mine's Uno) is 4 microseconds:

if I ask the program to delay 1 microseconds, it would delay anywhere from 1-4 microseconds, right?

groundFungus:
Doing timing with delay() and delayMicroseconds() will not be very accurate as the other stuff that the program is doing will effect the timing. Better to use millis() and micros() for timing as described in the begiiner’s guide to millis tutorial and the several things at a time post.

I did (assuming correctly):

  if (buttonRead == HIGH) {
    for (i = 1; i <= motherMatrix[buttonCounter - 1][0]; i++) {
      digitalWrite(ledPin, HIGH);
      startMillis=millis();
      digitalWrite(ledPin, LOW);
      currentMillis=millis();
      while (currentMillis-startMillis<motherMatrix[buttonCounter-1][1]){
        currentMillis=millis();
      };
    };
    buttonCounter++;
  }
  else {
    digitalWrite(ledPin, LOW);
  };

The output is still irregularly-timed. Why do you suspect that’s still happening?

      currentMillis=millis();
      while (currentMillis-startMillis<motherMatrix[buttonCounter-1][1]){
        currentMillis=millis();
      };

Congratulations, you have just invented the delay() function.

The issue with the method described in the BWoD for my project is that, if the time limit is not met, the program continues running the rest of the code.

I don't want that. I only want the continuation of the code to happen if and only if the time limit/period is met.

Does that make sense?

You had some kind of unusual code in your state change detection and debounce. What always works for me is:

boolean LastButtonState = LOW;
unsigned long ButtonTimePrior = 0;
const unsigned long DebounceDelay = 50;


void loop()
{
  unsigned long currentTime = millis();
  boolean buttonState = digitalRead(ButtonPin);


  //  State Change Detection and Debouncing:
  if (buttonState != LastButtonState && currentTime - ButtonTimePrior >= DebounceDelay)
  {
    ButtonTimePrior = currentTime;
    LastButtonState = buttonState;

Here is what that look like in your sketch:

const int LEDPin = 6;
const int ButtonPin = 4;


boolean LastButtonState = LOW;
int ButtonCounter = 0;


unsigned long ButtonTimePrior = 0;
const unsigned long DebounceDelay = 50;


const int NumCommands = 10;
// {number of pulses, inter-pulse period in milliseconds}:
int CommandList[NumCommands][2] =
{
  {167, 3},
  {125, 4},
  {100, 5},
  {83, 6},
  {71, 7},
  {63, 8},
  {56, 9},
  {50, 10},
  {45, 11},
  {42, 12}


};


void setup()
{
  pinMode(LEDPin, OUTPUT);
  pinMode(ButtonPin, INPUT);
};


void loop()
{
  unsigned long currentTime = millis();
  boolean buttonState = digitalRead(ButtonPin);


  //  State Change Detection and Debouncing:
  if (buttonState != LastButtonState && currentTime - ButtonTimePrior >= DebounceDelay)
  {
    ButtonTimePrior = currentTime;
    LastButtonState = buttonState;


    if (buttonState)
    {
      // Button Has Just Been Pressed
      ButtonCounter = (ButtonCounter + 1) % NumCommands;


      for (int i = 0; i < CommandList[ButtonCounter][0]; i++)
      {
        digitalWrite(LEDPin, HIGH);
        delayMicroseconds(1);
        digitalWrite(LEDPin, LOW);
        delay(CommandList[ButtonCounter][1]);
      }
    }
  }
}

Got it, thank you.

Do you think there's anything else causing the issue?

ArianKS:
Do you think there's anything else causing the issue?

I don't see anything in the actual pulse generation that would cause the timing to be off significantly and no way for the pulses to occur FASTER than the specified interval. How are you measuring the inter-pulse intervals?

You seem to be generating roughly enough pulses to fill 500 milliseconds. Both the interval (ButtonCount + 3) and the number of pulses (500 / (ButtonCount + 3)) can be calculated easily from the ButtonCount so there is no need for the array.

Thank you for your response.

The issue is that, while this array follows a logical order (like you mentioned), the ultimate goal of the set up is to run a randomized order of the commands (which would necessitate the use of arrays I believe).

I added your debounce code, and I still get the irregular time-intervals (recording with the Intan). I tried to avoid using delay interrupts, since I've been told it throws the timing of the code off.

Beyond that, I'm currently trying to write a code similar to the BWoD guideline, but I'm stuck; here's the issue:

let's assume that I do want to use a for() to generate each "bout" of impulse commands. Then the "timer" function I want should be right after the digitalWrite(x,LOW) command. The goal is to have the for() continue only if a specified amount of time has passed after the digitalWrite(x,LOW) command. I could use a 'while' function or the good old delay(), which both allegedly lead to time discrepancies in the code. Following the BWoD guideline—coding in such a way that would keep the main loop going while checking to see if the specified amount of time has passed—would not allow me to use a for() in a way that I specified.

I hope I'm making sense, it's a bit difficult to precisely describe my situation.

Hello,

Here’s a gist of my issue:

To contextualize my issue, a bit of background on my project might be helpful. My total set up includes a button (which could be pressed, in the form of a lever inside a Skinner box), an Arduino Uno board, and an Intan (a stimulator used for the delivery of electrical pulses).

My intention was to write a program that instructs the Intan (via a digital-in B/C cable) to “fire up” a pre-defined waveform for a certain number of times and with a certain duration (in milliseconds) of inter-pulse delay.

For example, on the first lever press, I want the Intan to deliver 10 pulses, each 3 milliseconds apart. On the second subsequent lever press, I want the Intan to deliver 20 pulses, each 1.5 milliseconds apart, etc.

The issue I’m facing: on any given lever press, the inter-pules period between the “grouped” pulses is not regular–i.e., for a command that specifies 10 pulses 3 ms apart, some end up being less than 3 ms, some more. I’ve repeatedly checked my code, but I’m at my wits end at this point. I’d appreciate it if I could get some feedback on what’s going wrong with my code.

I’ve also tried messing with Timer1, still to no avail.

int ledPin = 6;
int buttonPin = 4;
int buttonCounter = 1;
int i;
byte buttonRead;
unsigned long buttonTimeNow;
unsigned long buttonTimePrior = 0;
unsigned long debounceDelay = 50;
unsigned long startMillis;
unsigned long currentMillis;
//{number of pulses, inter-pulse period}:
int motherMatrix [10][2] = {
  {167, 3},
  {125, 4},
  {100, 5},
  {83, 6},
  {71, 7},
  {63, 8},
  {56, 9},
  {50, 10},
  {45, 11},
  {42, 12}

};
//change this for a new list
int numCommands = 10;


void setup() {
  // put your setup code here, to run once:
  cli();
  TCCR1A = 0;// set entire TCCR1A register to 0
  TCCR1B = 0;// same for TCCR1B
  TCNT1  = 0;//initialize counter value to 0
  // set compare match register for 1hz increments
  OCR1A = 15624;// = (16*10^6) / (1*1024) - 1 (must be <65536)
  // turn on CTC mode
  TCCR1B |= (1 << WGM12);
  // Set CS10 and CS12 bits for 1024 prescaler
  TCCR1B |= (1 << CS12) | (1 << CS10);
  // enable timer compare interrupt
  TIMSK1 |= (1 << OCIE1A);

  sei();
  pinMode(ledPin, OUTPUT);
  pinMode(buttonPin, INPUT);

};

void loop() {
  // put your main code here, to run repeatedly:

  buttonRead = digitalRead(buttonPin);



  //debouncing:
  if (buttonRead == HIGH) {
    buttonTimeNow = millis();
  };
  if (buttonTimeNow - buttonTimePrior > debounceDelay) {
    buttonRead = digitalRead(buttonPin);
    buttonTimePrior = buttonTimeNow;
  }
  else {
    buttonRead = LOW;
  };
  //

  //setting up repeats when the number of lever presses exceeds the number of stored commands:
  if (buttonCounter <= numCommands) {
    buttonCounter = buttonCounter;
  }
  else if (buttonCounter > numCommands) {
    buttonCounter = 1;
  };
  //




  if (buttonRead == HIGH) {

    for (i = 1; i <= motherMatrix[buttonCounter - 1][0]; i++) {
      digitalWrite(ledPin, HIGH);
      startMillis = millis();
      digitalWrite(ledPin, LOW);
      currentMillis = millis();
      while (currentMillis - startMillis < motherMatrix[buttonCounter - 1][1]) {
        currentMillis = millis();
      };
    };
    buttonCounter++;
  }
  else {
    digitalWrite(ledPin, LOW);
  };


  //making sure the button is released before proceeding:
  while (buttonRead == HIGH) {
    buttonRead = digitalRead(buttonPin);
  };
};

This looks very familiar.

Did you not like the replies on your other thread ?

I tried implementing them (and even completely got rid of the debounce in one of my versions, in case that was the culprit). No success.

The BWoD also does not apply to my situation here specifically.

I tried changing the interrupt frequency of Timer1 (another feedback I received, but don’t know how it would be relevant due to my being a novice), still no success.

Threads merged.

ArianKS:
The issue with the method described in the BWoD for my project is that, if the time limit is not met, the program continues running the rest of the code.

I don't want that. I only want the continuation of the code to happen if and only if the time limit/period is met.

Does that make sense?

Ok, so tell your code not to run those other parts if you don't want them to run outside of that time.

unsigned long past = 0;

int mode = 0;

void loop(){

  if(mode == 0){
    "I'll print this over and over while waiting for the time to be right"  
  }
  if(mode == 1){
    "I'll only print this when the time is right"
  }

  if(millis() - past >= 1000){
    mode = 1-mode;
    past = millis();
  }
  
}

That uses BWoD but only runs the appropriate part of the program based on the time.