I'm experiencing some weird issue with my metronome program.
It's suddenly all messed up as millis() seems to get back to 0 all the time.
Here's a snippet of my code where I put some print statements :
void playMetronome() {
if (inPlayMetronome) return; // To avoid restarting the function while it's already busy!
inPlayMetronome = true;
long interval = 60000/tempo;
if (millis() - previousMillis > interval) {
previousMillis = millis();
if (currentBeat == 1) {
// Reinitialize notePositions2[]
for (int i = 0; i < sizeof(notePosition2); i++) {
notePosition2[i] = notePosition[i];
}
beatMillis = millis(); // This is for the second voice to know when the 1st beat comes
Serial.print("Upbeat : ");
Serial.println(millis());
noTone(piezzoPin);
tone(piezzoPin, downBeatTone, beepDuration); // High tone if 1st beat
}
else {
Serial.print("Downbeat : ");
Serial.println(millis());
noTone(piezzoPin);
tone(piezzoPin, upBeatTone, beepDuration); // Lower tone if other beat
}
if (currentBeat == timeSig) {
currentBeat = 1; // Starting back at beat 1 if end of time signature
}
else currentBeat++; // Or incrementing beat nr
}
for (int i = 0; i < notesAdded ; i++) {
if (notePosition2[i] > 0 && millis() - beatMillis > notePosition2[i] - toleranceMargin && millis() - beatMillis < notePosition2[i] + toleranceMargin) {
Serial.print("Accent : ");
Serial.println(millis());
noTone(piezzoPin);
tone(piezzoPin, accentTone, beepDuration); // Other tone if at position defined by user
notePosition2[i] = 0;
}
}
inPlayMetronome = false;
}
"And I get in my serial monitor : Upbeat : 0 Downbeat : 1 Downbeat : 1002 Downbeat : 2003 Upbeat : 0 Downbeat : 1 Downbeat : 1001 Downbeat : 2002 etc."
I assume the etc. to mean the pattern keeps going (eg. the pattern of 2003, 2002, 2001, 2000, 1999).
Do I assume right? If not, please give us the readings of about twice as many as you did.
I would not rule out the arduino resetting either.
One software way to check that would be to add a long variable such as loopCounter, and increment it inside the loop(), then display it to the serial monitor. Above loop(), initialize it to a value, such as 1000.
I vote that the program restarts. You can Serial.print() in setup() any message that the rest of the program doesn't use to verify whether it restarts. Something like, "OK," "Initializing," or "YIKES!" The easier it is to see among a bunch of other output, the better.
As for what might be making it restart, we can't tell. But, this looks suspicious:
void playMetronome() {
if (inPlayMetronome) return; // To avoid restarting the function while it's already busy!
That suggests that some of your code might try to call playMetronome() while it's already running. Function playMetronome() doesn't call itself, so any code that calls it can only execute when playMetronome() isn't running, unless playMetronome() is called from an ISR. This function calls Serial.print(), which shouldn't be done from a ISR. It could also run pretty long for an ISR, depending on the values of the loop limits.
Variable inPlayMetronome looks to be global. You might be better served by having the calling program check the value of inPlayMetronome, and refrain from calling it if it's true. You might be even better served by moving all this work outside of an ISR, if an ISR is calling it.
@jack wp, it goes 0, 1, about 1000, about 2000 then again 0, 1, 1000, 2000. What it should do is consistently go up by about 1000 (0, 1000, 2000, 3000) but millis() is not supposed to be on 0 like this.
@Vaclav, I tried your suggestion, I get this back :
@tmd3, I tried and it ran setup() only once at the beginning while my millis() still obviously resets all the time.
To answer the rest of your message, playMetronome() gets called from loop() so as to get the regular ticks I need. And inPlayMetronome is an instance variable. Pardon my noobness but I don't know what an ISR is.
Btw I tried commenting out "if (inPlayMetronome) return;" but that didn't change anything.
Also I tried assigning millis() to a variable currentMillis but as expected it didn't change the problem either. I chose to use the function again everytime so as to avoid delays in reading.
Ok so here's my whole code.
I know it's a bit ugly as is (too many instance variables where I could have used parameter passing for instance) but this is my first personal project on Arduino...
I have to post it in several posts as it's too long for one.
/* CONSTANTS & INSTANCE VARIABLES */
// LCD
#include <LiquidCrystal.h>
LiquidCrystal lcd(12, 11, 7, 6, 5, 4);
// Pins
const int onOffSwitchPin = 9;
const int addNotePin = 2;
const int modeSwitchPin = 3;
const int potPin = A0;
const int resetPin = 13;
const int piezzoPin = 8;
// Buttons
boolean modeState = LOW;
boolean modeLastState = LOW;
int currentMode = 0;
int maxModes = 3;
boolean onOffState = LOW;
boolean onOffLastState = LOW;
boolean on = false;
boolean addNoteState = LOW;
boolean addNoteLastState = LOW;
boolean resetState = LOW;
boolean resetLastState = LOW;
int potVal = 0;
int lastPotVal = 0;
// Time
long previousMillis = 0;
long beatMillis = 0;
float pos = 0;
float lastPos = 0;
boolean inPlayMetronome = false;
int toleranceMargin = 10; // in ms
// Music
const int beepDuration = 20;
const int downBeatTone = 880;
const int upBeatTone = 440;
const int accentTone = 659;
long tempo = 100;
int minTempo = 30;
int maxTempo = 350;
int lastTempo = 0;
int timeSig = 4;
int minTimeSig = 1;
int maxTimeSig = 20;
int lastTimeSig = 0;
const int noteStep = 1; // in amount of 16th notes, steps by which to increment the position in the bar when setting up a second voice
int currentBeat = 1;
int notesAdded = 0;
float notePosition[10];
float notePosition2[10];
float notePositionTempo[10];
/* MAIN FUNCTIONS */
void setup() {
Serial.begin(9600);
pinMode(onOffSwitchPin, INPUT);
pinMode(addNotePin, INPUT);
pinMode(modeSwitchPin, INPUT);
pinMode(potPin, INPUT);
lcd.begin(16, 2); // set up the LCD's number of columns and rows
updateLCD();
// Initialize notePosition[]
for (int i = 0 ; i < sizeof(notePosition) ; i ++) {
notePosition[i] = 0;
}
// Initialize notePosition2[]
for (int i = 0; i < sizeof(notePosition2); i++) {
notePosition2[i] = notePosition[i];
}
// Initialize notePositionTempo[]
for (int i = 0; i < sizeof(notePositionTempo); i++) {
notePositionTempo[i] = tempo;
}
}
void loop() {
checkOnOffButton();
onOffAction();
checkModeButton();
modeAction();
checkAddNoteButton();
checkResetButton();
}
/* FUNCTIONS */
void updateLCD() {
lcd.clear();
lcd.print(tempo);
lcd.print(" bpm");
lcd.print(" ");
lcd.print(timeSig);
lcd.print("/4");
lcd.setCursor(0,1);
lcd.print(currentModeToString(currentMode));
}
void changePos() {
potVal = analogRead(potPin);
float potPercent = (float) potVal/1024; // 1024 instead of 1023 so we never get to 100% (100% of a bar is the beginning of the next bar)
float barDuration = calculateBarDuration();
float stepsPerBeat = (4/(float)noteStep);
float stepsPerBar = stepsPerBeat * (float)timeSig;
float stepDuration = calculateNoteDuration(noteStep);
float potStep = 1 / stepsPerBar;
float tempFloat = (float)(int)floor(potPercent/potStep);
pos = tempFloat*stepDuration;
if (lastPos != pos) { // WATCH OUT : GOES HERE BASICALLY ALL THE TIME. THE VALUE ALWAYS VARIES FOR SOME REASON
String posString = millisPosToMusicalPos();
lcd.setCursor(0,1);
lcd.print(posString);
}
lastPos = pos;
}
String stdth(int number) {
if (number % 10 == 1 && number != 11) return "st";
else if (number % 10 == 2 && number != 12) return "nd";
else if (number % 10 == 3 && number != 13) return "rd";
else return "th";
}
int n16thsToFraction(int n16ths) {
if (n16ths == 1) return 16;
else if (n16ths == 2) return 8;
else if (n16ths == 8) return 2;
else if (n16ths == 16) return 1;
else return n16ths;
}
String millisPosToMusicalPos() {
float barDuration = calculateBarDuration();
float stepsPerBeat = (4/(float)noteStep);
float stepsPerBar = stepsPerBeat * (float)timeSig;
float stepDuration = calculateNoteDuration(noteStep);
int stepPos = pos/stepDuration; // CHECKED OK
int placeInBeat = ((int)stepPos % (int)stepsPerBeat) + 1; // 1 % 4 = 1
int beatNumber = (int)floor(stepPos/(int)stepsPerBeat) + 1; // floor() rounds down
int noteStepFraction = n16thsToFraction(noteStep);
String result = "BEAT ";
return result + beatNumber + ", " + placeInBeat + stdth(placeInBeat) + " " + noteStepFraction + stdth(noteStepFraction); + " "; // I add the " " to make sure whatever's left of the screen is cleared.
}
void checkAddNoteButton() {
addNoteState = digitalRead(addNotePin);
if (addNoteState != addNoteLastState) { // if button was just pressed
if (addNoteState == HIGH) { // to avoid doing it again after releasing the button
addNoteAction();
}
}
addNoteLastState = addNoteState;
}
// In milliseconds
float calculateNoteDuration(float note) {
float tempoFloat = (float) tempo;
return (float) ((60000/tempoFloat) * (note/4));
}
// In milliseconds
float calculateBarDuration() {
float tempoFloat = (float) tempo;
float timeSigFloat = (float) timeSig;
return (float) ((60000/tempoFloat) * timeSigFloat);
}
void changeTimeSig() {
potVal = analogRead(potPin);
float potPercent = (float) potVal/1023;
timeSig = (int) ((potPercent * (maxTimeSig - minTimeSig)) + minTimeSig);
if (lastTimeSig != timeSig) {
currentBeat = 1;
// Serial.print("Time Signature : ");
// Serial.print(timeSig);
// Serial.println("/4");
updateLCD();
}
lastTimeSig = timeSig;
}
void changeTempo() {
potVal = analogRead(potPin);
float potPercent = (float) potVal/1023;
tempo = (int) ((potPercent * (maxTempo - minTempo)) + minTempo);
if (lastTempo != tempo) {
// Serial.print("Tempo : ");
// Serial.println(tempo);
updateLCD();
}
updateNotePositionTempi();
lastTempo = tempo;
}
void modeAction() {
switch(currentMode) {
case 0:
changeTempo();
break;
case 1:
changeTimeSig();
break;
case 2:
changePos();
break;
default:
changeTempo();
break;
}
}
void checkModeButton() {
modeState = digitalRead(modeSwitchPin);
if (modeState != modeLastState) { // if button was just pressed
if (modeState == HIGH) { // to avoid doing it again after releasing the button
// Serial.println(currentMode);
// Serial.print("Mode : ");
if (currentMode == maxModes - 1) {
currentMode = 0;
// Serial.println(currentModeToString(currentMode));
} else {
currentMode++;
// Serial.println(currentModeToString(currentMode));
}
updateLCD();
}
}
modeLastState = modeState;
}
String currentModeToString(int currentModeInt) {
switch(currentModeInt) {
case 0:
return "Edit tempo";
case 1:
return "Edit meter";
case 2:
return "Add a note";
default:
return "Invalid mode";
}
}
void resetAction() {
for (int i = 0 ; i < notesAdded ; i ++) {
notePosition[i] = 0;
}
notesAdded = 0;
// Serial.println("Notes removed");
lcd.setCursor(0,1);
lcd.print("Notes removed ");
}
void checkResetButton() {
resetState = digitalRead(resetPin);
if (onOffState != onOffLastState) { // if button was just pressed
if (onOffState == HIGH) { // to avoid doing it again after releasing the button
resetAction();
}
}
resetLastState = resetState;
}
void onOffAction() {
if (on) {
playMetronome();
} else currentBeat = 1;
}
void checkOnOffButton() {
onOffState = digitalRead(onOffSwitchPin);
if (onOffState == onOffLastState) return; // if button was not just pressed
if (onOffState == HIGH) { // to avoid doing it again after releasing the button
if (!on) {
on = true;
beatMillis = millis();
// Serial.println("On");
} else if (on) {
on = false;
// Serial.println("Off");
}
}
onOffLastState = onOffState;
}
void addNoteAction() {
notesAdded++;
notePosition[notesAdded - 1] = pos;
notePositionTempo[notesAdded - 1] = tempo;
lcd.setCursor(0,1);
lcd.print("Note added! ");
Serial.print("Note added at ");
Serial.print(pos);
Serial.println(" or ");
Serial.println(millisPosToMusicalPos());
}
void updateNotePositionTempi() {
for (int i = 0; i < notesAdded; i++) {
if (notePositionTempo[i] != tempo) {
/*
float oldPosition = notePosition[i];
float oldTempo = notePositionTempo[i];
500 & tempo 60 & noteStep 1 -> noteValue 2,
get back the relative position with the old tempo
convert to new tempo
notePositionTempo[i] = tempo;
*/
}
}
}
void playMetronome() {
if (inPlayMetronome) return; // To avoid restarting the function while it's already busy!
inPlayMetronome = true;
long interval = 60000/tempo;
if (millis() - previousMillis > interval) {
previousMillis = millis();
if (currentBeat == 1) {
// Reinitialize notePositions2[]
for (int i = 0; i < sizeof(notePosition2); i++) {
notePosition2[i] = notePosition[i];
}
beatMillis = millis(); // This is for the second voice to know when the 1st beat comes
Serial.print("Upbeat : ");
Serial.println(millis());
noTone(piezzoPin);
tone(piezzoPin, downBeatTone, beepDuration); // High tone if 1st beat
}
else {
Serial.print("Downbeat : ");
Serial.println(millis());
noTone(piezzoPin);
tone(piezzoPin, upBeatTone, beepDuration); // Lower tone if other beat
}
if (currentBeat == timeSig) {
currentBeat = 1; // Starting back at beat 1 if end of time signature
}
else currentBeat++; // Or incrementing beat nr
}
for (int i = 0; i < notesAdded ; i++) {
/*Serial.print("notePosition[");
Serial.print(i);
Serial.print("] = ");
Serial.println(notePosition[i]);*/
if (notePosition2[i] > 0 && millis() - beatMillis > notePosition2[i] - toleranceMargin && millis() - beatMillis < notePosition2[i] + toleranceMargin) {
Serial.print("Accent : ");
Serial.println(millis());
noTone(piezzoPin);
tone(piezzoPin, accentTone, beepDuration); // Other tone if at position defined by user
notePosition2[i] = 0;
// Serial.print("ms offset (inaccuracy) : ");
// Serial.println(millis() - beatMillis - notePosition[i]);
}
}
inPlayMetronome = false;
//delay(20);
}
// Initialize notePosition2[]
for (int i = 0; i < sizeof(notePosition2); i++) {
notePosition2[i] = notePosition[i];
}
You do that in lots of places. The sizeof an array is its size in bytes, and a float is 4 bytes. So immediately you have corrupted memory. Do something like this:
Even if you start the serial monitor immediately and print millis(), it will never be zero until it rolls over (43 days?). Something else is wrong with the code. It must be overwriting the data area where the millis variable is stored.
