Program or hardware lag?

Projects Programming
1

I'm working on an LCD sequencer for the Bleep Labs drum machine, and I've got it working about 95%. The remaining 5% is that, every so often, the program seems to lag out for just a second. There is a momentary pause that would probably normally be irrelevant but, considering it's a drum machine, is very noticeable. Oddly enough, this lag-out only happens when the tempo is set to 114 or above. The way I've done debounce and things like that is a little bit janky I'm sure, and I assume that might have something to do with it.

Here's the code. If you see any glaring issue that would cause this to get ahead of itself or something like that, please let me know as I really want this functioning at 100%. FYI, the pads will trigger on a low pulse, and the pulse clock ensures that each pulse is long enough for the drum machine to see.

#include <SPI.h>
// include the library code:
#include <LiquidCrystal.h>

// initialize the library with the numbers of the interface pins
LiquidCrystal lcd(8, 13, 9, 4, 5, 6, 7);

int butClk= 0; //debounce buttons
int trigClk = 0; //debounce pad triggers
int pulClk = 0; //send pulses
boolean butStr = false;
boolean triStr = false;
boolean pulStr = false;

long t = 110; //t
long tLast = 110; //last recorded ts
long tMil; //space in millis between beat triggers
const int y = 12; //kick
const int b = 3; //tom
const int g = 2; //clap
const int r = 11; //snareq

int seqLoc = 0; //playb location in seq
int seqSel = 0; //selection location
int padSel = 0; //selected pad
int tempSel = 0; //selected t digit
boolean tEdit = false; //editing t

int seqR[32] = {
  0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0};
int seqB[32] = {
  0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0};
int seqG[32] = {
  0,0,0,0,1,0,0,0,0,0,0,0,1,0,0,0,0,0,0,0,1,0,0,0,0,0,0,0,1,0,0,0};
int seqY[32] = {
  1,0,0,0,1,0,0,0,1,0,0,0,1,0,0,0,1,0,0,0,1,0,0,0,1,0,0,0,1,0,0,0};

String fr, fb, fg, fy, br, bb, bg, by; //display seqs

void setup() {
  Serial.begin(9600);
  
  tMil = newtMil(t);

  pinMode(y, OUTPUT);
  pinMode(b, OUTPUT);
  pinMode(r, OUTPUT);
  pinMode(g, OUTPUT);

  digitalWrite(y, HIGH);
  digitalWrite(b, HIGH);
  digitalWrite(r, HIGH);
  digitalWrite(g, HIGH);

  lcd.begin(16, 2);
  lcd.print("BPM:110");
  lcd.setCursor(0,1);
  genSeqStrings();
  lcd.print(fr);
  lcd.setCursor(9,0);
  lcd.print("S:");
  lcd.setCursor(12, 0);
  lcd.print("1-16");
  lcd.setCursor(0,1);
  lcd.cursor();
}

void loop() {
  //t changes
  if (t != tLast) {
    tLast = t;
    tMil = newtMil(t);
  }

  if (analogRead(0) < 800 && !butStr) {
    butStr = true;    
    buttonRead();
  }

  if (butStr && butClk < 1000) {
    butClk += 1;
  } 
  else {
    butClk = 0;
    butStr = false;
  }

  //trigger control
  if (millis() % tMil == 0 && !triStr) {
    playNext();
    triStr = true;
  } 
  else if (triStr && trigClk < 10) {
    trigClk += 1;
  }
  else {
    triStr = false;
    trigClk = 0;
  }

  //pulse control
  if (pulStr) {
    if (pulClk >= 10) {
      digitalWrite(r, HIGH);
      digitalWrite(b, HIGH);
      digitalWrite(g, HIGH);
      digitalWrite(y, HIGH);
      pulClk = 0;
      pulStr = false;
    } 
    else {
      pulClk += 1;
    }
  }
}

void buttonRead() {
  int x = analogRead(0);

  if (x < 100) { //RIGHT
    if (!tEdit) {
      if (seqSel >= 31) {
        tEdit = true;
        tempSel = 2;
      } 
      else {
        seqSel += 1;
      }
    } 
    else {
      if (tempSel >= 2) {
        tEdit = false;
        seqSel = 31;
      } 
      else {
        tempSel += 1;
      }
    }
  } 
  else if (x < 200){ //UP
    if (!tEdit) {
      if (padSel >= 3) {
        padSel = 0;
      } 
      else {
        padSel += 1;
      }
    }
    else {
      switch(tempSel) {
      case 0:
        t += 100;
        break;
      case 1:
        t += 10;
        break;
      case 2:
        t += 1;
        break;
      }

      if (t > 480) {
        t = 480;
      }
    }
  } 
  else if (x < 400){ //DOWN
    if (!tEdit) {
      if (padSel <= 0) {
        padSel = 3;
      } 
      else {
        padSel -= 1;
      }
    }
    else {
      switch(tempSel) {
      case 0:
        t -= 100;
        break;
      case 1:
        t -= 10;
        break;
      case 2:
        t -= 1;
        break;
      }

      if (t < 30) {
        t = 30;
      }
    }
  } 
  else if (x < 600){ //LEFT
    if (!tEdit) {
      if (seqSel <= 0) {
        tEdit = true;
        tempSel = 0;
      } 
      else {
        seqSel -= 1;
      }
    } 
    else {
      if (tempSel <= 0) {
        tEdit = false;
        seqSel = 0;
      } 
      else {
        tempSel -= 1;
      }
    }
  } 
  else if (x < 800){ //SELECT
    if (!tEdit) {
      if (padSel == 0) {
        if (seqR[seqSel] == 0) {
          seqR[seqSel] = 1;
        } 
        else {
          seqR[seqSel] = 0;
        }
      } 
      else if (padSel == 1) {
        if (seqB[seqSel] == 0) {
          seqB[seqSel] = 1;
        } 
        else {
          seqB[seqSel] = 0;
        }
      } 
      else if (padSel == 2) {
        if (seqG[seqSel] == 0) {
          seqG[seqSel] = 1;
        } 
        else {
          seqG[seqSel] = 0;
        }
      } 
      else if (padSel == 3) {
        if (seqY[seqSel] == 0) {
          seqY[seqSel] = 1;
        } 
        else {
          seqY[seqSel] = 0;
        }
      }
    }
  }
  updateDisplay();
}

void genSeqStrings() {
  fr = "";
  fb = "";
  fg = "";
  fy = "";
  br = "";
  bb = "";
  bg = "";
  by = "";
  for (int i = 0; i<16; i++) {
    fr += seqR[i];
    br += seqR[i+16];
    fb += seqB[i];
    bb += seqB[i+16];
    fg += seqG[i];
    bg += seqG[i+16];
    fy += seqY[i];
    by += seqY[i+16];
  }
}

void updateDisplay() {
  lcd.clear();
  lcd.print("BPM:");
  if (t >= 100) {
    lcd.print(t);
  }
  else {
    lcd.print(" ");
    lcd.print(t);
  }

  lcd.setCursor(9,0);
  switch(padSel) {
  case 0:
    lcd.print("S:");
    break;
  case 1:
    lcd.print("T:");
    break;
  case 2:
    lcd.print("C:");
    break;
  case 3:
    lcd.print("K:");
    break;
  }

  lcd.setCursor(0,1);
  genSeqStrings();

  switch(padSel) {
  case 0:
    if (seqSel <= 15) {
      lcd.print(fr);
    } 
    else {
      lcd.print(br);
    }
    break;
  case 1:
    if (seqSel <= 15) {
      lcd.print(fb);
    } 
    else {
      lcd.print(bb);
    }
    break;
  case 2:
    if (seqSel <= 15) {
      lcd.print(fg);
    } 
    else {
      lcd.print(bg);
    }
    break;
  case 3:
    if (seqSel <= 15) {
      lcd.print(fy);
    } 
    else {
      lcd.print(by);
    }
    break;
  }

  if (seqSel > 15) {
    lcd.setCursor(11, 0);
    lcd.print("17-32");
  } 
  else {
    lcd.setCursor(12, 0);
    lcd.print("1-16");
  }
  if (!tEdit) {
    if (seqSel > 15) {
      lcd.setCursor(seqSel - 16, 1);
    } 
    else {
      lcd.setCursor(seqSel, 1);
    }
  }
  else {
    lcd.setCursor(tempSel + 4, 0);
  }

  lcd.cursor();
}

void playNext() {
  if (seqR[seqLoc] == 1) {
    digitalWrite(r, LOW);
  }
  if (seqB[seqLoc] == 1) {
    digitalWrite(b, LOW);
  }
  if (seqG[seqLoc] == 1) {
    digitalWrite(g, LOW);
  }
  if (seqY[seqLoc] == 1) {
    digitalWrite(y, LOW);
  }

  if (seqLoc >= 31) {
    seqLoc = 0;
  }
  else {
    seqLoc += 1;
  }

  pulStr = true;
}

long newtMil(int t) {
  float s1 = 60.0/t;
  float s2 = s1/4.0;
  float s3 = s2 * 1000.0;
  long final = long(s3);
  return final;
}
2

The obvious bug is here:

  if (millis() % tMil == 0 && !triStr) {

millis() does NOT generate every consecutive value, occasionally it skips
a value. So such a test will fail every so often.

That code will also fail when millis() wraps round.

This is how its done:

unsigned long prev_t = 0L ;

...

  if (millis () - prev_t >= tMil)  // subtract before compare so always works
  {
    prev_t += tMil ;  // advance prev_t for next time.
    if (!triStr)
    {
      ...
3 
    prev_t += tMil ;  // advance prev_t for next time.

No. You shouldn't be adding times.

  unsigned long now = millis();
  if (now - prev_t >= tMil)  // subtract before compare so always works
  {
    prev_t += now;  // keep track of last time.
4

I may be missing something, but I'm not sure I quite understand this line.

prev_t += now;  // keep track of last time.

Did you perhaps mean prev_t = now? I tried the code with = now instead, and it works like a charm! The only thing I don't quite understand is how the if statement is working. Wouldn't "now - prev_t" always simply be 1?

5

Did you perhaps mean prev_t = now?

I did.

The only thing I don't quite understand is how the if statement is working. Wouldn't "now - prev_t" always simply be 1?

After one millisecond, yes. After less than that, like on the next pass through loop, the difference will be 0. After a while, the difference will go to 2, then 3, then 4, etc., until it exceeds the defined interval (tMil). Then, prev_t will be updated.

6

PaulS:

    prev_t += tMil ;  // advance prev_t for next time.

No. You shouldn't be adding times.

Try reading the OP's code - tMil is the delay in milliseconds per beat - its an
interval, not an absolute time:

long tMil; //space in millis between beat triggers

So I definitely should be adding the interval to prev_t to set up the next time.

7

Lets get this nice and clear:

common idiom:

unsigned long prev_t = 0L ;

void loop ()
{
  unsigned long now = millis () ;
  if (now - prev_t >= DELAY)
  {
    prev_t = now ;
    .. do stuff ..
  }
  .. do other stuff ..
}

Note the two flaws here:

  1. If milis () skips values (which it does on many Arduino boards) then
    every skip will guarantee a 1ms slip in time carries forwards into the future.

  2. If the "stuff" or "other stuff" happens to take longer than DELAY ms then
    the whole sketch will slip back in time never to recover. If you were meant
    to be providing time to a clock display this would be an issue.

This idiom cures both:

unsigned long prev_t = 0L ;

void loop ()
{
  unsigned long now = millis () ;
  if (now - prev_t >= DELAY)
  {
    prev_t += DELAY ;
    .. do stuff ..
  }
  .. do other stuff ..
}

The "do stuff" will keep running until the sketch catches up with the absolute
time, should something delay the sketch. Missed millis() values merely have
a 1ms temporary jitter effect (unavoidable if only calling millis()).

When wanting drift-free timing via micros(), this idiom is invaluable as microseconds
then matter.

8

Note the two flaws here:

  1. If milis () skips values (which it does on many Arduino boards) then
    every skip will guarantee a 1ms slip in time carries forwards into the future.

If a 1 millisecond discrepancy has ANY impact on the application, milliseconds is the wrong time frame to be using.

  1. If the "stuff" or "other stuff" happens to take longer than DELAY ms then
    the whole sketch will slip back in time never to recover. If you were meant
    to be providing time to a clock display this would be an issue.

Then, again, you are using the wrong approach.

In general, it is better to keep track of the last time something happened, and determine if now minus then indicates that it is time to do it again. I stand by not adding times.