Help Using millis()

Hey all,

Pretty new to Arduino, and I'm having some trouble with using the millis() function. I found several tutorials explaining how it works, but they've all only been explanations of the BlinkWithoutDelay example, without showing any further application. I understand the philosophy behind using it, but when I try to incorporate millis() into my code I can't get it to work. I haven't been able to find anything online that explains applying the mills() function outside of the BlinkWithoutDelay demo.

I am using SparkFun's Musical Instrument Shield (SparkFun Music Instrument Shield - DEV-10587 - SparkFun Electronics) to create a keyboard that uses infrared emitters/detectors. But before I add in my infrared inputs I am just trying to get the program running with a push button.

What I want to do is

if digitalRead==Down
    then "set timer" Timer = millis, noteOn
if "timer expired" mills-Timer >=200
    then noteOff.

Where am I going wrong?

void loop()
{
  
  val = digitalRead(pushButton);
  if (val == HIGH) {
    noteOn(0, note, 100);             //Channel, Note Value, Velocity
      unsigned long currentMillis = millis();
        if(currentMillis - previousMillis > interval) {
          // save the last time  
            previousMillis = currentMillis;   
              noteOff(0, note, 60);
    } 
  } 
}

This is where you are going wrong

        if(currentMillis - previousMillis > interval) {
          // save the last time  
            previousMillis = currentMillis;   
              noteOff(0, note, 60);

Think about it.

Then look at blink without delay. Next time post the whole of your code never post a snippet!

Mark

#include <SoftwareSerial.h>

SoftwareSerial mySerial(2, 3); // RX, TX

int val = 0;
int pushButton = 7;   // pushbutton connected to digital pin 7


byte note = 60; //The MIDI note value to be played
byte resetMIDI = 4; //Tied to VS1053 Reset line
byte ledPin = 13; //MIDI traffic inidicator
int  instrument = 108;



// Variables will change:
long previousMillis = 0;        // will store last time LED was updated

// the follow variables is a long because the time, measured in miliseconds,
// will quickly become a bigger number than can be stored in an int.
unsigned long interval = 200;           // interval at which to blink (milliseconds)

void setup() {
  
  
  pinMode(pushButton, INPUT);      // sets the digital pin 7 as input
  
  Serial.begin(57600);

  //Setup soft serial for MIDI control
  mySerial.begin(31250);

  //Reset the VS1053
  pinMode(resetMIDI, OUTPUT);
  digitalWrite(resetMIDI, LOW);
  delay(100);
  digitalWrite(resetMIDI, HIGH);
  delay(100);
  talkMIDI(0xB0, 0x07, 120); //0xB0 is channel message, set channel volume to near max (127)
  talkMIDI(0xB0, 0, 0x00); //Default bank GM1
  talkMIDI(0xC0, instrument, 0);
}

void loop()
{
  
  val = digitalRead(pushButton);
  if (val == HIGH) {
    noteOn(0, note, 100);             //Channel, Note Value, Velocity
      unsigned long currentMillis = millis();
        if(currentMillis> interval) {
            currentMillis =0;   
              noteOff(0, note, 60);
    } 
  } 
}

//Send a MIDI note-on message.  Like pressing a piano key
//channel ranges from 0-15
void noteOn(byte channel, byte note, byte attack_velocity) {
  talkMIDI( (0x90 | channel), note, attack_velocity);
}

//Send a MIDI note-off message.  Like releasing a piano key
void noteOff(byte channel, byte note, byte release_velocity) {
  talkMIDI( (0x80 | channel), note, release_velocity);
}

//Plays a MIDI note. Doesn't check to see that cmd is greater than 127, or that data values are less than 127
void talkMIDI(byte cmd, byte data1, byte data2) {
  digitalWrite(ledPin, HIGH);
  mySerial.write(cmd);
  mySerial.write(data1);

  //Some commands only have one data byte. All cmds less than 0xBn have 2 data bytes 
  //(sort of: http://253.ccarh.org/handout/midiprotocol/)
  if( (cmd & 0xF0) <= 0xB0)
    mySerial.write(data2);

}

Thanks for responding Mark, there’s the whole code. ^

I just made a change in the loop section?

That also makes sense to me…

if currentMillis > interval
    then set currentMillis back to 0, and turn noteOff

But I just tested it and it doesn’t work.

This is killing me, What am I missing!!!

what you are missing is that millis() returns the number of milliseconds that have passed SINCE YOUR PROGRAM STARTED! It is ever increasing and doesn't go back to zero until you power up or reload your program. It's like a clock.

Ah okay I see so then what I want to do is wait for the noteOn, then count 200milliseconds after that, then turn the noteOff.

But that's what I thought I was doing in the original code snippet

cooper_hu:
Ah okay I see so then what I want to do is wait for the noteOn, then count 200milliseconds after that, then turn the noteOff.
But that's what I thought I was doing in the original code snippet

Nope.

First, you only enter the if block if the pushbutton is pressed, which means that you keep assigning ever greater values of millis() to currentMillis.
You then immediately check to see if the interval has passed. It hasn't because you just set currentMillis again. You keep doing that as long as the pushbutton is pressed.

When you finally let go of the pushbutton, the tone is still playing, and will keep on playing, because you never check the millis() again, or at least not until you press the button again, and enter the first if block again, at which time you start assigning ever increasing values of millis() to currentMillis, and immediately check to see if interval has passed.

Use the computer you were born with... follow the statements one at a time, and figure out where you go in the program flow.

cooper_hu:
This is killing me, What am I missing!!!!

What you are missing is the notion of 'state persistence', which is a pretty common thing for beginners to miss.

Pressing the button triggers the noteOn state change, which you want to persist for some_period, before automatically triggering the noteOff state change. So, you will need a variable to hold the current state, a variable to hold the period of persistence and another variable to hold the time the period started.

bool notePlaying = false;
uint16_t noteStart = 0;
uint16_t notePeriod = 100;
// thanks to the magic of overflow, an unsigned 16 bit integer 
// can be used to measure periods up to ~65 seconds.

void loop(void) {
	uint16_t now = millis();  
	if(!notePlaying) {
		if(digitalRead(notePin)) {
			noteStart  = now;
			noteOn();
			notePlaying = true;
		}
	}
	else if(now - noteStart > notePeriod) {
		noteOff();
		notePlaying = false;
	}
}

Ahhh I see!! That's the aspect from the blink without delay example that I was omitting in my sketch!

So noteStart is the variable that holds the time period started,

notePeriod is the variable that holds the period of persistence,

and notePlaying is the boolean that holds the current state, right?

So what I was doing wrong was

  1. not using a variable that holds the current state
  2. wrongly formatting the persistence of some_period before noteOff.

Is that more-or-less on point?
Thanks for explaining Matt! This has been bugging me since Saturday

Blink without delay is an example of a simple finite state machine (FSM). Look them up in the playground.

Mark