Piezo Buzzer for loop not working  

Page 2 / 2
  RSS

rms84
(@rms84)
Active Member
Joined: 2 months ago
Posts: 8
2019-11-14 9:35 pm  

Thank you everyone that contributed to this thread, I have managed to solve the problem. 🤗 🤗 🤗 

A big thank you for educating me that i was being re-set to 0 every time the code looped. I was completely unaware of this and was tearing my hair out wondering what was wrong with my code!

For some reason when I declared i in the global setup code it wouldn't work (not sure why, probably something a newbie like me overlooked). So I did some research into how to stop i being reset to 0 and came across a way to stop that is to declare i as a "static int". I also changed the code to an if / else statement and it finally beeped the correct number of times then stopped. 

I will post the code below in case it can help anybody reading this with a similar problem, or if anyone has any further advice on how to improve. I've kept my delay code in there, so wondering why that's bad? Some feedback as to why its ugly would be really appreciated and I will see if I can improve on that next. As I wanted it to beep for 2 mins it seemed this was the best way to do it (with my limited knowledge).

 

Here is my new code. I've kept the rest of my project code out to avoid confusion.....

void setup() {
// put your setup code here, to run once:

}
void loop() {

static int i = 0;
int pin = 8;
int frequency = 2000;
int duration = 500;

// beeps for 2 minutes (120 seconds)
if (i < 120){
i++;
tone(pin, frequency, duration);
delay(1000);
}

else
noTone(pin);
}

 


ReplyQuote
rms84
(@rms84)
Active Member
Joined: 2 months ago
Posts: 8
2019-11-26 8:03 pm  

@pugwash

I've just learned why delay is a problem as it was starting to affect something else in the project. Thanks for educating me, I've now changed the script to millis as I've done some research into it and now understans why it's better to use millis rather than delay 😀 


ReplyQuote
Pugwash
(@pugwash)
Honorable Member
Joined: 6 months ago
Posts: 561
2019-11-26 8:12 pm  
Posted by: @rms84

@pugwash

I've just learned why delay is a problem as it was starting to affect something else in the project. Thanks for educating me, I've now changed the script to millis as I've done some research into it and now understans why it's better to use millis rather than delay 😀 

My pleasure!! May you have years of unproblematic interrupt requests! 😎 

SteveC - Oh Lord, please don't let me be misunderstood! (Eric Burdon and the Animals)


ReplyQuote
byron
(@byron)
Trusted Member
Joined: 6 months ago
Posts: 92
2019-11-27 2:05 am  

rms84

I just perused your code snippet  a couple of posts back whilst as I was finishing my bed time Horlicks.  I did not look at the whole thread but my quick comments on your code as as follows.

I see you now appreciate millis instead of delay and see that you now see that your code was looping back to the start of the loop and so the i variable was being reset to zero.  As its now changed to a static variable the loop function remembers the value and as it's already declared it now ignores the declaration that initially assigns a 0 value.  

However whilst I presume this was just a little test script, normally a lot more goes on in the main loop so it would be best to get into the habit of making this beeping as a function that is then called from the main loop as required.   Also rather than use variable that is checked with an if statement and then incremented with a delay  consider the following (just a suggestion)

int beenBeeped = 0; // this is just so the function is only called once in the loop()

const int frequency = 2000; 

const int duration = 500;

void setup() {

}

void loop() {

if (beenBeeped == 0) { 

   beenBeeped = doBeeb(120000); // 2 minutes I hope!!

  }

}

int doBeeb(long beep_time) {

tone(8,frequency,duration);

while (millis() < beep_time) { }

noTone(8);

return 1;

}

 

If the frequency and duration variables do not change (vary) then consider using a constant for these values.  As the example above place them  at the top of the program so their values can be easily modified should the tone not be to your taste

As I'm very prone to typos I thought I would put the code into Arduino IDE and check.  Well bugger me the Arduino no longer works on my my Mac and I find its because I updated to the latest os Catalina.   I cant get to bed until this is fixed so I hope its not going to be a long night.  Oh bother.

This post was modified 2 weeks ago by byron

ReplyQuote
Pugwash
(@pugwash)
Honorable Member
Joined: 6 months ago
Posts: 561
2019-11-27 8:29 am  

@byron

"Horlicks" my Dad used to drink that stuff, but I found it revolting, then again the reverse was true about Marmite. It is an associative thing, as I have always linked Horlicks to old people, and I am trying to convince myself that my own 64 years is not OLD.

Back to the topic, I disagree about using an extra function. tone() is already a function and a one-liner at that, therefore it would unnecessarily bloat the code. And I think that there is confusion about tone() and noTone().

noTone() is only called if the tone duration is not specified. Therefore

tone(8,frequency,duration);
while (millis() < beep_time) { }
noTone(8);

makes no sense. tone() will shut off automatically after the duration has been achieved.

Also putting millis() in an external function is not a good idea, all timing and flow control should be in the main loop() function. Keep external functions for procedures that are called at different points in the program and/or passed different variables on each loop.

SteveC - Oh Lord, please don't let me be misunderstood! (Eric Burdon and the Animals)


ReplyQuote
Pugwash
(@pugwash)
Honorable Member
Joined: 6 months ago
Posts: 561
2019-11-27 8:59 am  

@byron, @rms84

I put the following code together to demonstrate the advantage of millis(). In this code, I am controlling the blink rate of 5 different LEDs, using 5 control loops.

unsigned long eventInterval[] = {500, 100, 400, 300, 200}; // stores the half wave values in microseconds
unsigned long previousTime[] = {0, 0, 0, 0, 0}; // stores the temporary time value
byte ledState[] = {0, 0, 0, 0, 0}; // stores the current pin state
byte outputPin[] = {3, 4, 5, 6, 7}; // stores the output pin number 

void setup(){  
  pinMode(outputPin[0], OUTPUT);
  pinMode(outputPin[1], OUTPUT);  
  pinMode(outputPin[2], OUTPUT);
  pinMode(outputPin[3], OUTPUT);  
  pinMode(outputPin[4], OUTPUT);
}

void loop(){
  
  unsigned long currentTime= millis(); 
  
  //control routine for the frequency of digital pin 3
  if ( currentTime - previousTime[0] >= eventInterval[0]){    
    toggleLED(0);    
    previousTime[0] = currentTime;
  }
  
  //control routine for the frequency of digital pin 4
  if ( currentTime - previousTime[1] >= eventInterval[1]){    
    toggleLED(1);    
    previousTime[1] = currentTime;
  }

  //control routine for the frequency of digital pin 5
  if ( currentTime - previousTime[2] >= eventInterval[2]){    
    toggleLED(2);    
    previousTime[2] = currentTime;
  }
  
  //control routine for the frequency of digital pin 6
  if ( currentTime - previousTime[3] >= eventInterval[3]){    
    toggleLED(3);    
    previousTime[3] = currentTime;
  }

  //control routine for the frequency of digital pin 7
  if ( currentTime - previousTime[4] >= eventInterval[4]){    
    toggleLED(4);    
    previousTime[4] = currentTime;
  }

  // add more control routines here
}

void toggleLED(int idx) {
  if(ledState[idx] == 0){
    digitalWrite(outputPin[idx], HIGH);
    ledState[idx] = 1;
    }
   else if(ledState[idx] != 0){
    digitalWrite(outputPin[idx], LOW);
    ledState[idx] = 0;
    }  
}

Now you could easily replace toggleLED() with tone() if you change the pin allocation to the PWM pins. And with various durations and frequencies, produce an unpleasant cacophony of sound. 🤣 

SteveC - Oh Lord, please don't let me be misunderstood! (Eric Burdon and the Animals)


ReplyQuote
byron
(@byron)
Trusted Member
Joined: 6 months ago
Posts: 92
2019-11-27 11:34 am  

@rms84 @Pugwash

Well I'm not too bleary eyed after staying up to fix my Arduino / mac Catalina issue (which is now resolved after a bit of googling) and I see the errors in my suggestions that has proved to be a not too helpful I fear.  I quite agree on your point about not using millis in the way I put it as its utterly superfluous as tone has a duration argument.  And, as millis() starts when the Ardunino is fired up my code would not work if some other task meant that the funtion was not called after 2 minutes was up.  All in all a right balls up. 😥 

So I now distill my suggestions as follows

1. consider using a constance if the variable value does not change in the program.

2. Use functions as much as possible is good practice.  However do not use a function just to call another function, (silly me). 

3. If you are needing to use time delays then don't loop with if statements and incrementing a variable, millis() is incrementing away quite nicely as it is.

4. if the function has a duration argument such as tone() then use it, it will do just that.  

And I see from rms84 lastest posted code your programming now looks much better and you are learning very fast indeed. 👍   (woops I edit this post as see it was Pugwash that posted the code not rms84 - I must keep taking the tablest!)

Fixing up my Arduino IDE and the googling around somewhat reminded my of the antiquity of good old Arduino uno and the programming environment.   For Python I started to use Visual Studio Code and really like it so I will load the platformIO into it and have a go at that for Arduino code to see how I like it.    That also brings to mind my recent forays into using micropython for boards that can use it in place of the Arduino C.  Whilst C runs a lot faster then python, in my use cases of home control, weather station sensor recording, and robot control, has proved fast enough and much better to use.  I should not bang on about this in this post and sometime I will start a new thread on this when I get the time to see if it sparks any interest.

Coincidentally yesterday my wife was mooting what to put in her Christmas groceries list and found some marmite flavoured crisps and biscuits in her online shopping site. In place of the flavoured biscuits I suggested a jar of marmite and plain old biscuits so I could then spread it on nice and thick.  I wonder if a bit of marmite in my Horlicks would be a good idea. 😲 

This post was modified 2 weeks ago 2 times by byron

ReplyQuote
Pugwash
(@pugwash)
Honorable Member
Joined: 6 months ago
Posts: 561
2019-11-27 11:44 am  

@byron

However, do not use a function just to call another function.

There is absolutely nothing wrong with calling functions from other functions, in fact, recursive functions are a science in their own right.

"Horlicks and Marmite" entirely redefines the term "an acquired" taste"! 😆 😆 

SteveC - Oh Lord, please don't let me be misunderstood! (Eric Burdon and the Animals)


ReplyQuote
byron
(@byron)
Trusted Member
Joined: 6 months ago
Posts: 92
2019-11-27 11:55 am  

@Pugwash - I was not meaning to say functions calling functions is wrong, just was meaning to say that creating a function just to call another function, and thats all it does, is a waste and I was concerned my previous post could be construed to indicate I was advocating this.   But you are right to point this out and I think I will refrain from making more suggestion as I only seem to potentially mislead which is not good.


ReplyQuote
Page 2 / 2

Please Login or Register