help needed with return; function

Using Arduino Programming Questions
1

Hi.

(Please see the full code in the attachment, unable to paste full code directly, goes beyond posting limit).

In the following version of the code, all was well. Even though there was a minor delay, the return function worked as intended and pressing the alarm button while the melody was playing, did switch off the alarm.

void playTone(int tone, int duration)
{
  for (long i = 0; i < duration * 1000L; i += tone * 2) {
    digitalWrite(speakerPin, HIGH);
    delayMicroseconds(tone);
    digitalWrite(speakerPin, LOW);
    delayMicroseconds(tone);
  }
}

void playNote(char note, int duration)
{
  char names[] = { 'c', 'd', 'e', 'f', 'g', 'a', 'b', 'C' };
  int tones[] = { 1915, 1700, 1519, 1432, 1275, 1136, 1014, 956 };

  // play the tones according to the notes
  for (int i = 0; i < 8; i++) {
    if (names[i] == note) {
      playTone(tones[i], duration);

    }
  }
}

void playAlarm()
{
  if ((secondClock == secondAlarm && minuteClock == minuteAlarm && hourClock == hourAlarm && alarmOnOff == true))
  {
    for (int i = 0; i < length; i++)
    {
      playNote(notes[i], beats[i] * tempo);
      delay(tempo / 4);
      //read the alarm button value, if the "mode change" button is pressed while alarm is playing, it will take the system to alarm mode and switch alarm off.
      alarmValue = analogRead(buttonAlarm);
      if (alarmValue < 1000)
      {
        alarmOnOff = false;
        return;
      }
    }
  }
  return;
}

//main loop to recall all functions
void loop()
{
  timing();
  lightOnLed(modeClock);
  readButtons();
  playAlarm();
}

But that was not exactly the melody that I wanted. So, I found another one. But this other one is in a different format. Although, I tried to make it fit with the code that I have, I noticed the return; function is no longer working, because if I press the alarm button while the tone is playing, it does nothing. it is supposed to switch alarm off.

New version of code: (ofcourse, there is more notes to it, than I have pasted, but this is only for the sake of demonstration.

void playAlarm()
{
  if ((secondClock == secondAlarm && minuteClock == minuteAlarm && hourClock == hourAlarm && alarmOnOff == true))
  {
    // Set this to be the pin that your buzzer resides in. (Note that you can only have one buzzer actively using the PWM signal at a time).
    int tonePin = 9;

    tone(tonePin, 523, 562.5);
    delay(625.0);
    delay(312.5);
    tone(tonePin, 391, 281.25);
    delay(312.5);
    tone(tonePin, 523, 562.5);
    delay(625.0);
    tone(tonePin, 391, 421.875);
    delay(468.75);

//read the alarm button value, if the "mode change" button is pressed while alarm is playing, it will take the system to alarm mode and switch alarm off.
    alarmValue = analogRead(buttonAlarm);
    if (alarmValue < 1000)
    {
      alarmOnOff = false;
      return;
    }
  }
  return;
}

//main loop to recall all functions
void loop()
{
  timing();
  lightOnLed(modeClock);
  readButtons();
  playAlarm();
}
2

here. forgot to attach files:

final_test_with_alarm.ino (23.2 KB)

new_test.ino (9.77 KB)

3

you are using delay() which means if you press and release the button while delay is executing the press it will be missed
use millis() to generate the delays, e.g.

also you are using analogRead() to read the button state - why not digitalRead()?

4 
    delay(312.5);
    tone(tonePin, 391, 281.25);

What is all this nonsense with floats as parameters to these functions ?

5

UKHeliBob:

    delay(312.5);

tone(tonePin, 391, 281.25);


What is all this nonsense with floats as parameters to these functions ?

I don't know. I just used this piece of software to convert the desired midi file to arduino sketch. Is there a better way? To be honest, I'm not sure about the code, but the music, as far as the notes go, is spot on. I don't know how to write music for arduino in a better way. Is there a way to put all this into a better format?

6

horace:
you are using delay() which means if you press and release the button while delay is executing the press it will be missed
use millis() to generate the delays, e.g.
Using millis() for timing | Multi-tasking the Arduino - Part 1 | Adafruit Learning System

also you are using analogRead() to read the button state - why not digitalRead()?
digitalRead() - Arduino Reference

Thanks. Did you mean I'm using delay() in that "void playAlarm" section or in general throughout the code? Like I said, in my previous version the button worked OK.

7

Is there a way to put all this into a better format?

It#s not so much the format, rather it is the fact that the functions take integers as parameters so using floats is a wate of time as the fractional part of the value will be ignored anyway.

8

horace:
you are using delay() which means if you press and release the button while delay is executing the press it will be missed
use millis() to generate the delays, e.g.
Using millis() for timing | Multi-tasking the Arduino - Part 1 | Adafruit Learning System

also you are using analogRead() to read the button state - why not digitalRead()?
digitalRead() - Arduino Reference

I looked at adafruit example of using delays with millis. but if I write that much code for the amount of notes that I have for music, then my code will be too large to manage. is there a simpler way?

9

arduinoware:
Thanks. Did you mean I'm using delay() in that "void playAlarm" section or in general throughout the code? Like I said, in my previous version the button worked OK.

to check the switch you are using a technique called polling where you check the state of the input device at regular intervals for changes - the problem is the change can be missed if the program is doing other things for significant periods of time
in your case the delays in the second program are longer than in the first

for the delay try millis(), e.g. this program blinks a LED - every time the button is pressed the blink time doubles

// blink LED - on switch press double blink time

int switch1 = 10;         
int ledPin1 = 5;           

void setup() {
  Serial.begin(9600);
  pinMode(switch1, INPUT);
  pinMode(ledPin1, OUTPUT);
}

void loop() {
  unsigned long int timer=millis();
  static unsigned long int delayTime=100;
  digitalWrite(ledPin1, !digitalRead(ledPin1));
  while(millis()-timer < delayTime)
     {
     if(!digitalRead(switch1))         // switch pressed?
       {
        delayTime*=2;                  // double blink time
        Serial.println(delayTime);
        while(!digitalRead(switch1));  // wait for switch release
       }
     }
}

note there is no checking for switch bounce
when I run the code and press the button the serial monitor displays

200
400
800
1600
10

horace:
to check the switch you are using a technique called polling where you check the state of the input device at regular intervals for changes - the problem is the change can be missed if the program is doing other things for significant periods of time
in your case the delays in the second program are longer than in the first

for the delay try millis(), e.g. this program blinks a LED - every time the button is pressed the blink time doubles

// blink LED - on switch press double blink time

int switch1 = 10;       
int ledPin1 = 5;

void setup() {
  Serial.begin(9600);
  pinMode(switch1, INPUT);
  pinMode(ledPin1, OUTPUT);
}

void loop() {
  unsigned long int timer=millis();
  static unsigned long int delayTime=100;
  digitalWrite(ledPin1, !digitalRead(ledPin1));
  while(millis()-timer < delayTime)
    {
    if(!digitalRead(switch1))        // switch pressed?
      {
        delayTime*=2;                  // double blink time
        Serial.println(delayTime);
        while(!digitalRead(switch1));  // wait for switch release
      }
    }
}



note there is no checking for switch bounce
when I run the code and press the button the serial monitor displays

200
400
800
1600

Thank you. But this is getting to where it is becomming a little too much for me to handle. I think I will just try to find music with similar sketch and go with that.

11

arduinoware:
I looked at adafruit example of using delays with millis. but if I write that much code for the amount of notes that I have for music, then my code will be too large to manage. is there a simpler way?

put the delay in a function, e.g.

// blink LED - on switch press double blink time

int switch1 = 10;         
int ledPin1 = 5;           

void setup() {
  Serial.begin(9600);
  pinMode(switch1, INPUT);
  pinMode(ledPin1, OUTPUT);
}


void loop() {
  static unsigned long int delayTime=100;
  digitalWrite(ledPin1, !digitalRead(ledPin1));
  if(delayTimer(delayTime))          // if switch pressed
    {       
      delayTime*=2;                  // double blink time
      Serial.println(delayTime);
    }
}

// delay for a time 
// if switch pressed return 1 else 0
int delayTimer(unsigned long int delayTime)
{
  unsigned long int timer=millis();
  while(millis()-timer < delayTime)
     if(!digitalRead(switch1))         // switch pressed?
       {
        while(!digitalRead(switch1));  // wait for switch release
        return 1;                      // yes, return 1
       }
  return 0;                            // no return 0
}

you could use it

    tone(tonePin, 391, 281.25);
    if(delayTimer(312)return;

you will probably need to stop the tone playing before returning

12

The demo Several Things at a Time illustrates the use of millis() to manage timing without blocking. It may help with understanding the technique.

...R

13

Ok just a quick one, for my project, is it worth having the return function? I mean, I want to completely remove the bit that checks if the alarm button is pressed while alarm is playing. So in that case, can I remove both of the returns?

void readButtons()
{
  alarmValue = analogRead(buttonAlarm);
  //read the value of mode change button
  if (alarmValue < 1000)
  {
    if (modeClock == true)
    {
      modeClock = false;
      modeAlarm = true;
      tone(speakerPin, 1900);
      delay(200);
      noTone(speakerPin);

      //after mode change, zero all signals at digital outputs
      for (int i = 0; i <= 13; i++)  {
        digitalWrite(i, LOW);
      }
      delay(200);
    } else
    {
      modeClock = true;
      modeAlarm = false;
      //after mode change, zero all signals at digital outputs
      for (int i = 0; i <= 13; i++) {
        digitalWrite(i, LOW);
      }
      delay(200);
    }

  }
  //read the values of buttons for hour/min change.
  hourValue = analogRead(buttonHour);
  minuteValue = analogRead(buttonMinute);
  //in alarm mode, simultaneous pressing on hour and minute buttons, toggles alarmclock on/off()
  if (hourValue < 1000 && minuteValue < 1000)
  {
    if (alarmOnOff == false) {
      alarmOnOff = true;
    } else {
      alarmOnOff = false;
    }
  }
  //read button value to set time.
  if (modeClock == true)
  {
    if (hourValue < 1000)
    {
      hourClock++;
      if (hourClock == 24) {
        hourClock = 0;
        minuteClock = 0;
      }
      secondClock = 0;
      rtc.setTime(hourClock, minuteClock, secondClock);
      delay(200);
    }
    //read button value to set minutes.

    if (minuteValue < 1000)
    {
      minuteClock++;
      if (minuteClock == 60) {
        hourClock++;
        if (hourClock == 24) hourClock = 0;
        minuteClock = 0;
      }
      secondClock = 0;
      rtc.setTime(hourClock, minuteClock, secondClock);
      delay(200);
    }
  } else
    //set hour and minute for the alarm clock
  {
    if (hourValue < 1000)
    {
      hourAlarm++;
      if (hourAlarm == 24) {
        hourAlarm = 0;
        minuteAlarm = 0;
        secondAlarm = 0;
        delay(200);
      }
 } //ADDED
      //set minutes

      if (minuteValue < 1000)
      {
        minuteAlarm++;
        if (minuteAlarm == 60) {
          hourAlarm++;
          if (hourAlarm == 24) hourAlarm = 0;
          minuteAlarm = 0;
          secondAlarm = 0;
          delay(200);
        }
      }
    // REMOVED
  }
}

    void playTone(int tone, int duration)
    {
      for (long i = 0; i < duration * 1000L; i += tone * 2) {
        digitalWrite(speakerPin, HIGH);
        delayMicroseconds(tone);
        digitalWrite(speakerPin, LOW);
        delayMicroseconds(tone);
      }
    }

    void playNote(char note, int duration)
    {
      char names[] = { 'c', 'd', 'e', 'f', 'g', 'a', 'b', 'C' };
      int tones[] = { 1915, 1700, 1519, 1432, 1275, 1136, 1014, 956 };

      // play tone according to notes
      for (int i = 0; i < 8; i++) {
        if (names[i] == note) {
          playTone(tones[i], duration);

        }
      }
    }

    void playAlarm()
    {
      if ((secondClock == secondAlarm && minuteClock == minuteAlarm && hourClock == hourAlarm && alarmOnOff == true))
      {
        for (int i = 0; i < length; i++)
        {
          playNote(notes[i], beats[i] * tempo);
          delay(tempo / 4);
          //pressing modechange (alarm) button while the tone is playing, will cause the alarm to switch off.
          alarmValue = analogRead(buttonAlarm);
          if (alarmValue < 1000)
          {
            alarmOnOff = false;
            return;
          }
        }
      }
return;
    }

    //main cycle, recall of main functions.
    void loop()
    {
      timing();
      lightOnLed(modeClock);
      readButtons();
      playAlarm();
    }

Cheers.

14

is it worth having the return function?

return is a STATEMENT, not a function.

The return STATEMENT is required if the function returns a value. It is optional if the function does not.

The return statement can be used to return from a function before the end of the function.

            alarmOnOff = false;

Does false mean that the alarm is on or off? Setting alarmOn to false would clearly indicate that the alarm is not on. Who knows that the heck setting alarmOnOff to false means.

15

Who knows that the heck setting alarmOnOff to false means.

You think that's bad. I once used a system where the help text for one of the setup options read

Answer YES if you don't want this not to happen

16

UKHeliBob:
You think that's bad. I once used a system where the help text for one of the setup options read

That was funny :slight_smile: Yes. alarmOnOff =False means alarm is off. On for on and off for off.

I have removed the "return"s and everything seems to be working. Having that feature didn't make sense anyway. It is a binary clock in a wooden building the top of which comes off. Nobody would be doing that to switch off the alarm. It is easier to unplug the clock. Does the same thing. Plus keeps time with RTC.

17

Yes. alarmOnOff =False means alarm is off. On for on and off for off.

The alarm is either on (true) or off (false). The NAME of the variable should be alarmOn with true meaning that it is on and false meaning off, or it should be alarmOff, with true meaning that it is off and false meaning "Oh, give it a rest. Shut the f**k up!"

What does waterHotCold = true mean? Do NOT deliberately introduce ambiguity into your code. Someday, you'll need to maintain that code, and wonder what idiot wrote it.

18

PaulS:
What does waterHotCold = true mean? Do NOT deliberately introduce ambiguity into your code. Someday, you'll need to maintain that code, and wonder what idiot wrote it.

You are right. :slight_smile:

The thing is that I did not write this code from scratch. I wouldn't have done that. I am an OCD sufferer. I am very picky about details.

But I'm a beginner when it comes to coding. And let alone write it, I had enough challenge editting the code to suit my needs. So, I only editted what I had to edit.

19