Millis in combination with tone

Projects Programming
1

What Im trying to make - instructables (yes, this Instructable is "stolen" from original author, but instructions are more clearer there)

However, I want to use momentary pushbutton and leds (preferably with some animation, like slow fading away etc..), and that requires the Arduino to multitask - use millis.

So what Ive done is converted everything to millis, but the original code for making square waves was so complicated for me, that I failed with converting it. Then I found out about tone, so now the whole code uses millis with tone.

I have a problem with the output. I somehow cant get Arduino to output at certain frequency for defined amount of time. Instead it just makes a quick pop sound every time it should start outputting, then silence until defined time passes and loop repeats. Also, it doesnt go through all of the 8 buttons, it just plays the frequency of the first button.

what Im trying to achieve is clearly visible from the Instructable

ino file as an attachement

(the tempo and duration variables are probably wrongly placed in the code for now)

seqTONEaMILLIS.ino (4.14 KB)

2

This instructable is wrong. You must not connect a speaker directly to a digital output of the Arduino. Depending on the position of the potentiometer (volume) you might have already destroyed your pin 11.

The maximum current an Arduino pin is able to provide is about 20mA. A typical speaker has an impedance of about 4-8Ω, so almost a short circuit for the Arduino.

3

I know, thats why for now Im using 8ohm speaker with resistor, but I will use audio amplifier in the final version
yeah and the speaker is fine, it plays normally with a different program

4 
void generator(int freq, int t, unsigned long currentMillis)
{
  //unsigned long currentMillis = millis(); --idk if it should be
  //rewritten here again, or taken from main loop (as it is now)
  if (currentMillis - previousMillis2 >= duration) {
    previousMillis2 = currentMillis;
    tone(DigitalOutSignal, freq);
    digitalWrite(13, HIGH);
    Serial.print("Frequency: ");
    Serial.print(freq);
    Serial.println();
    Serial.print("Duration: ");
    Serial.print(t);
    Serial.println();
  }
  else {
    noTone(DigitalOutSignal);
    digitalWrite(13, LOW);  
  }
}

currentMillis is not updated. If the "if" test is true once previousMillis2 is updated and the clause will never be true again for the whole loop. Also if it wasn't true at the begin of the loop, it will never get true as no condition is changed.

5

pylon:
currentMillis is not updated. If the "if" test is true once previousMillis2 is updated and the clause will never be true again for the whole loop. Also if it wasn't true at the begin of the loop, it will never get true as no condition is changed.

thx for help, but I dont quite get you. So, currentMillis should be updated when void generator is called, I take it?

void generator(int freq, int t)
{
  unsigned long currentMillis = millis();
  if (currentMillis - previousMillis2 >= duration) {
    previousMillis2 = currentMillis;
    **play**
  }
  else {
    **stop**
  }
}

But I cant make out the rest of what you said, sorry

6

That code might work although I don't actually understand what it the intended functionality is. Remember, tone() is a blocking call, it won't return as long as the tone is playing.

7

Actually, if you rewrite this program to always play regardless of button on/off state, you can try this program on any Arduino and see through serial monitor that it always plays just one tone.
One more time, this is supposed to:

play tone 1 for duration time
stop for tempo time
play tone 2 for duration time
stop for tempo time
...
play tone 8 for duration time
stop for tempo time
REPEAT

But right now it constantly plays just the first tone, and it doesnt play like written above, it just makes a pop sound every duration seconds.
EDIT: After more testing, it goes through all the 8 steps, but only if duration is set to 10 or less, and it doesnt stop between the steps, it just plays all the steps at once, and it plays the last tone until the tempo time passes

8

This is a corrected version as I understood your project (not tested):

//NOTE: the code needs much more optimization, like debounce
//or whatever for momentary pushbutton etc...
//for testing, pots resistance values
//are replaced with constant value and
//switches are not being read


#define AnalogInFrequency 1
#define AnalogInTempo 2
#define AnalogInDuration 0
#define DigitalOutSignal 11
#define DigitalInStartStop 10
#define DigitalOutLED 12
uint8_t DigitalInSwitch[] = {2, 3, 4, 5, 6, 7, 8, 9};
// Set up the array for each step
int steps[] = {100,120,140,160,180,200,220,240};
#define NumberOfSteps 8
// misc housekeeping
int tempo = 10;
int duration = 50;
int pitchval = 1; //leftover from original code, idk what to do with it for now
int lastPushedStep = -1;

//turn on/off with momentary push button
int reading;
int previous = LOW;

//for millis()
unsigned long previousMillis = 0;
uint8_t step = 0;
#define PS_OFF 0
#define PS_PAUSE 1
#define PS_TONE 2
uint8_t playState = PS_OFF;

void setup()
{
  pinMode (DigitalOutSignal, OUTPUT);
  pinMode (DigitalOutLED, OUTPUT);
  pinMode (13, OUTPUT);
  Serial.begin(9600);
  analogWrite(12, 4);
  delay(580);
  digitalWrite(12, LOW);
}


void loop()
{
  //ON/OFF button
  reading = digitalRead(DigitalInStartStop);
  if (reading == HIGH && previous == LOW) {
    if (playState == PS_OFF) {
      playState = PS_PAUSE;
      digitalWrite (DigitalOutLED, HIGH);
      }
    else {
      playState = PS_OFF;
      digitalWrite (DigitalOutLED, LOW);
    }
  }
  previous = reading;

  unsigned long currentMillis = millis();
  if (playState == PS_PAUSE && currentMillis - previousMillis >= tempo) {
    previousMillis = currentMillis; // start tone
    playState = PS_TONE;
    //readSwitches();
    readPots();
  }
  // Make the noise
  if (playState == PS_TONE) {
    if (currentMillis - previousMillis >= duration) {
      previousMillis = currentMillis; // start pause
      playState = PS_PAUSE;
      noTone(DigitalOutSignal);
      step++;
      if (step >= NumberOfSteps) {
        step = 0;
      }
      digitalWrite(13, LOW);
    }
    else {
      tone(DigitalOutSignal, steps[step]);
      digitalWrite(13, HIGH);  
    }
  }

}



// Read the current values of the pots, called from the loop.
void readPots()
{
    tempo = 150;//(analogRead (AnalogInTempo) * 1.9);
    duration = 2000;//(analogRead (AnalogInDuration));
    Serial.println("-------------------POTS-------------------");
}

// Read the current values of the switches and
// if pressed, replace the switch's slot frequency
// by reading the frequency pot.
void readSwitches()
{
  // reset last pushed button number
  lastPushedStep = -1;

  for (uint8_t i = 0; i < 8; i++) {
    if (digitalRead(DigitalInSwitch[i]) == HIGH) {
      steps[i] = analogRead(AnalogInFrequency);
      lastPushedStep = i + 1;
    }
  }
}
9

Wow! it works. It doesnt stop playing when button is pressed again, but that I fixed easily, everything else works just as its supposed to. Impressive that you wrote this without testing!
I might post once more since I dont want to just straight away use your code, but I´ll rather try to fix mine that after rewriting the whole play part of the code started working properly, however the Arduino became self-aware and started imagining steps -1 with frequency 420 and step 8 with frequency 11565 :smiley:
ino attached if anyone wants their arduino to wake up and slowly take over the world

seqTONEaMILLIS.ino (4.11 KB)

10

Final code with the self-aware part removed:

#define AnalogInFrequency 1
#define AnalogInTempo 2
#define AnalogInDuration 0
#define DigitalOutSignal 11
#define DigitalInSwitch0 2
#define DigitalInSwitch1 3
#define DigitalInSwitch2 4
#define DigitalInSwitch3 5
#define DigitalInSwitch4 6
#define DigitalInSwitch5 7
#define DigitalInSwitch6 8
#define DigitalInSwitch7 9
#define DigitalInStartStop 10
#define DigitalOutLED 12
// Set up the array for each step
int steps[] = {100,120,140,160,180,200,220,240};
// misc housekeeping
int tempo = 10;
int duration = 50;
int pitchval = 1; //leftover from original code, idk what to do with it for now
int lastPushedStep = -1;

byte fPlayMode = 0;

//turn on/off with momentary push button
int reading;
int previous = LOW;
unsigned long debouncePeriod = 200;  
unsigned long debounceMillis = 0;

//for millis()
unsigned long previousMillis = 0;

int i = 0;

void setup() {
  digitalWrite(12, HIGH);
  pinMode (DigitalOutSignal, OUTPUT);
  pinMode (DigitalOutLED, OUTPUT);
  pinMode (13, OUTPUT);
  Serial.begin(9600);
  delay(500);
  digitalWrite(12, LOW);
}

void loop() {
  //ON/OFF button
  unsigned long currentMillis = millis();
  reading = digitalRead(DigitalInStartStop);
  if (reading == HIGH && previous == LOW) {
    if ((currentMillis - debounceMillis) >= debouncePeriod) {
      debounceMillis = currentMillis;
      if (fPlayMode == 0) {
        fPlayMode = 1;
        digitalWrite (DigitalOutLED, HIGH);
       }
      else {
        fPlayMode = 0;
        digitalWrite (DigitalOutLED, LOW);
        noTone(DigitalOutSignal);
      }
    }
  }
  previous = reading;
  if (fPlayMode == 1 && currentMillis - previousMillis >= tempo) {
    previousMillis = currentMillis;
    fPlayMode = 2;
    //readSwitches();
    readPots();
  }
  if (fPlayMode == 2) { 
    if (currentMillis - previousMillis >= duration) {
      previousMillis = currentMillis;
      fPlayMode = 1;
      noTone(DigitalOutSignal);
      i++;
      if (i >= 8) {
        i = 0;
      }
      digitalWrite(13, LOW);
    }
    else {
      digitalWrite(13, HIGH);
      tone(DigitalOutSignal, steps[i]);
    }
  }
}

// Read the current values of the pots, called from the loop.
void readPots()
{
  tempo = (analogRead (AnalogInTempo) * 1.9);
  duration = (analogRead (AnalogInDuration));
  Serial.print(tempo);
  Serial.print("  ::  ");
  Serial.print(duration);
  Serial.println();
}

// Read the current values of the switches and
// if pressed, replace the switch's slot frequency
// by reading the frequency pot.
void readSwitches()
{
  // reset last pushed button number
  lastPushedStep = -1;
  // check switch 0, if pressed, get the current freq into step 0, etc. etc.
  if (digitalRead (DigitalInSwitch0) == HIGH)
  {
    steps[0] = analogRead(AnalogInFrequency);
    lastPushedStep = 1;
  }
  else if (digitalRead (DigitalInSwitch1) == HIGH)
  {
    steps[1] = analogRead(AnalogInFrequency);
    lastPushedStep = 2;
  }
  else if (digitalRead (DigitalInSwitch2) == HIGH)
  {
    steps[2] = analogRead(AnalogInFrequency);
    lastPushedStep = 3;
  }
  else if (digitalRead (DigitalInSwitch3) == HIGH)
  {
    steps[3] = analogRead(AnalogInFrequency);
    lastPushedStep = 4;
   }
  else if (digitalRead (DigitalInSwitch4) == HIGH)
  {
    steps[4] = analogRead(AnalogInFrequency);
    lastPushedStep = 5;
  }
  else if (digitalRead (DigitalInSwitch5) == HIGH)
  {
    steps[5] = analogRead(AnalogInFrequency);
    lastPushedStep = 6;
  }
  else if (digitalRead (DigitalInSwitch6) == HIGH)
  {
    steps[6] = analogRead(AnalogInFrequency);
    lastPushedStep = 7;
  }
  else if (digitalRead (DigitalInSwitch7) == HIGH)
  {
    steps[7] = analogRead(AnalogInFrequency);
    lastPushedStep = 8;
  }
}