Need some help figuring out my code

Projects General Guidance
41

Are you suggesting polling for the button state?

Advantages and disadvantages of polling versus interrupt depend (as far as I see it) on the application. Using interrupts for buttons seems to be not unusual in the microcontroller world. There are even libraries that take over the tasks of debouncing and the detection of single or double clicks, not to mention the waste amount of examples ... I know what is widely spead must not necessarily be ok. Could/would you mind to describe your concerns about using interrupts with buttons, so that one can follow your arguments against it? I am open to learn ...

42

Sure. Why not?

We're over two hours into this topic, and we haven't seen any code.

Mostly because they're not the get-out-of-jail-free-card many noobs seem to think they are.

43

Consider this... you have to poll an interrupt because not all code can (or should) run inside an ISR because interrupts are (sensibly) turned off.

44

The microcontroller world on what planet?

a7

45

Sorry this took so long I had to write the code again because It isn't stored on this laptop.

#incude "pitches.h"

const int buttonPin = 10;
int buttonState;
const int melodyPin = 11;
const int rLed1 = 1;
const int rLed2 = 2;
const int rLed3 = 3;
const int rLed4 = 4;
const int rLed5 = 5;
const int rLed6 = 6;
const int wLed1 = 7;
const int wLed2 = 8;
const int wLed3 = 9;
const int wLed4 = 12;
const int wLed5 = 13;
const int wLed6 = 0;


int wish_melody[] = {
  NOTE_B3, 
  NOTE_F4, NOTE_F4, NOTE_G4, NOTE_F4, NOTE_E4,
  NOTE_D4, NOTE_D4, NOTE_D4,
  NOTE_G4, NOTE_G4, NOTE_A4, NOTE_G4, NOTE_F4,
  NOTE_E4, NOTE_E4, NOTE_E4,
  NOTE_A4, NOTE_A4, NOTE_B4, NOTE_A4, NOTE_G4,
  NOTE_F4, NOTE_D4, NOTE_B3, NOTE_B3,
  NOTE_D4, NOTE_G4, NOTE_E4,
  NOTE_F4
};

int wish_tempo[] = {
  4,
  4, 8, 8, 8, 8,
  4, 4, 4,
  4, 8, 8, 8, 8,
  4, 4, 4,
  4, 8, 8, 8, 8,
  4, 4, 8, 8,
  4, 4, 4,
  2
};

void setup() {
  pinMode(rled1, OUTPUT); // all led pins = output
  pinMode(rled2, OUTPUT);
  pinMode(rled3, OUTPUT);
  pinMode(rled4, OUTPUT);
  pinMode(rled5, OUTPUT);
  pinMode(rled6, OUTPUT);
  pinMode(wled1, OUTPUT);
  pinMode(wled2, OUTPUT);
  pinMode(wled3, OUTPUT);
  pinMode(wled4, OUTPUT);
  pinMode(wled5, OUTPUT);
  pinMode(wled6, OUTPUT);
  pinMode(button, INPUT);
  pinMode(melodyPin, OUTPUT);
  

}

void loop() {
  int song = 1;

  void sing(int s) {
    song = s;
    int size = sizeof(wish_melody) / sizeof(int);
    for (int thisNote = 0; thisNote < size; thisNote++) {

      // to calculate the note duration, take one second
      // divided by the note type.
      //e.g. quarter note = 1000 / 4, eighth note = 1000/8, etc.
      int noteDuration = 1000 / wish_tempo[thisNote];

      buzz(melodyPin, wish_melody[thisNote], noteDuration);

      // to distinguish the notes, set a minimum time between them.
      // the note's duration + 30% seems to work well:
      int pauseBetweenNotes = noteDuration * 1.30;
      delay(pauseBetweenNotes);

      // stop the tone playing:
      buzz(melodyPin, 0, noteDuration);

    }
  }

}

pitches.h:

#define NOTE_B0  31
#define NOTE_C1  33
#define NOTE_CS1 35
#define NOTE_D1  37
#define NOTE_DS1 39
#define NOTE_E1  41
#define NOTE_F1  44
#define NOTE_FS1 46
#define NOTE_G1  49
#define NOTE_GS1 52
#define NOTE_A1  55
#define NOTE_AS1 58
#define NOTE_B1  62
#define NOTE_C2  65
#define NOTE_CS2 69
#define NOTE_D2  73
#define NOTE_DS2 78
#define NOTE_E2  82
#define NOTE_F2  87
#define NOTE_FS2 93
#define NOTE_G2  98
#define NOTE_GS2 104
#define NOTE_A2  110
#define NOTE_AS2 117
#define NOTE_B2  123
#define NOTE_C3  131
#define NOTE_CS3 139
#define NOTE_D3  147
#define NOTE_DS3 156
#define NOTE_E3  165
#define NOTE_F3  175
#define NOTE_FS3 185
#define NOTE_G3  196
#define NOTE_GS3 208
#define NOTE_A3  220
#define NOTE_AS3 233
#define NOTE_B3  247
#define NOTE_C4  262
#define NOTE_CS4 277
#define NOTE_D4  294
#define NOTE_DS4 311
#define NOTE_E4  330
#define NOTE_F4  349
#define NOTE_FS4 370
#define NOTE_G4  392
#define NOTE_GS4 415
#define NOTE_A4  440
#define NOTE_AS4 466
#define NOTE_B4  494
#define NOTE_C5  523
#define NOTE_CS5 554
#define NOTE_D5  587
#define NOTE_DS5 622
#define NOTE_E5  659
#define NOTE_F5  698
#define NOTE_FS5 740
#define NOTE_G5  784
#define NOTE_GS5 831
#define NOTE_A5  880
#define NOTE_AS5 932
#define NOTE_B5  988
#define NOTE_C6  1047
#define NOTE_CS6 1109
#define NOTE_D6  1175
#define NOTE_DS6 1245
#define NOTE_E6  1319
#define NOTE_F6  1397
#define NOTE_FS6 1480
#define NOTE_G6  1568
#define NOTE_GS6 1661
#define NOTE_A6  1760
#define NOTE_AS6 1865
#define NOTE_B6  1976
#define NOTE_C7  2093
#define NOTE_CS7 2217
#define NOTE_D7  2349
#define NOTE_DS7 2489
#define NOTE_E7  2637
#define NOTE_F7  2794
#define NOTE_FS7 2960
#define NOTE_G7  3136
#define NOTE_GS7 3322
#define NOTE_A7  3520
#define NOTE_AS7 3729
#define NOTE_B7  3951
#define NOTE_C8  4186
#define NOTE_CS8 4435
#define NOTE_D8  4699
#define NOTE_DS8 4978

The problem I'm running into is the activation of the leds I have no clue on where to place them in the code. That's why I thought of interrupts to interrupt the code every say 1000ms to change the state of the LEDs.

46

Forget it. I'll wait for the real code... unless you tested it, and you are going to stick with it in this thread...

47

Get rid of this and use millis()

48

It's basically the same. I just don't understand how to use an interrupt in here every x amount of ms to change the state of all the LEDs. That's the problem I have. (plus the button starting and stopping it all)

49

That's fine because you should not use an interrupt...

50

Something like this?

#incude "pitches.h"

const int buttonPin = 10;
int buttonState;
const int melodyPin = 11;
const int rLed1 = 1;
const int rLed2 = 2;
const int rLed3 = 3;
const int rLed4 = 4;
const int rLed5 = 5;
const int rLed6 = 6;
const int wLed1 = 7;
const int wLed2 = 8;
const int wLed3 = 9;
const int wLed4 = 12;
const int wLed5 = 13;
const int wLed6 = 0;

unsigned long previousMillis = 0;


int wish_melody[] = {
  NOTE_B3, 
  NOTE_F4, NOTE_F4, NOTE_G4, NOTE_F4, NOTE_E4,
  NOTE_D4, NOTE_D4, NOTE_D4,
  NOTE_G4, NOTE_G4, NOTE_A4, NOTE_G4, NOTE_F4,
  NOTE_E4, NOTE_E4, NOTE_E4,
  NOTE_A4, NOTE_A4, NOTE_B4, NOTE_A4, NOTE_G4,
  NOTE_F4, NOTE_D4, NOTE_B3, NOTE_B3,
  NOTE_D4, NOTE_G4, NOTE_E4,
  NOTE_F4
};

int wish_tempo[] = {
  4,
  4, 8, 8, 8, 8,
  4, 4, 4,
  4, 8, 8, 8, 8,
  4, 4, 4,
  4, 8, 8, 8, 8,
  4, 4, 8, 8,
  4, 4, 4,
  2
};

void setup() {
  pinMode(rled1, OUTPUT); // all led pins = output
  pinMode(rled2, OUTPUT);
  pinMode(rled3, OUTPUT);
  pinMode(rled4, OUTPUT);
  pinMode(rled5, OUTPUT);
  pinMode(rled6, OUTPUT);
  pinMode(wled1, OUTPUT);
  pinMode(wled2, OUTPUT);
  pinMode(wled3, OUTPUT);
  pinMode(wled4, OUTPUT);
  pinMode(wled5, OUTPUT);
  pinMode(wled6, OUTPUT);
  pinMode(button, INPUT);
  pinMode(melodyPin, OUTPUT);
  

}

void loop() {
  int song = 1;

  void sing(int s) {
    song = s;
    int size = sizeof(wish_melody) / sizeof(int);
    for (int thisNote = 0; thisNote < size; thisNote++) {

      unsigned long currentMillis = millis();
      
      // to calculate the note duration, take one second
      // divided by the note type.
      //e.g. quarter note = 1000 / 4, eighth note = 1000/8, etc.
      int noteDuration = 1000 / wish_tempo[thisNote];

      buzz(melodyPin, wish_melody[thisNote], noteDuration);

      // to distinguish the notes, set a minimum time between them.
      // the note's duration + 30% seems to work well:
      long pauseBetweenNotes = noteDuration * 1.30;
      if (currentMillis - previousMillis >= pauseBetweenNotes) {
        previousMillis = currentMillis;

      // stop the tone playing:
      buzz(melodyPin, 0, noteDuration);

    }
  }

}
51

It's identical to the one I had before. Buzzer works fine I just can't for the life of me find a proper way to change the LEDs state during the song is playing since you said I should not use interrupts

52

Have a good long read here:
https://forum.arduino.cc/t/demonstration-code-for-several-things-at-the-same-time/217158

53

@thenoobjoker you might want to back up a bit so you don't make mistakes like trying to include a function within a function. It's C++, not Pascal!

void loop() {
  int song = 1;

  void sing(int s) {
    song = s;
.
.
.

Just wait 'til you get comfy and can share code that compiles. We here. We'll wait.

TIA

a7

54

This is a way to use interrupt if you are only interested in the falling edge and do not expect EM interference. I know that debouncing and the correct method of reading buttons is complicated and you have to take less care if you do polling. In the case coded below, you do not poll an interrupt, you are "polling " a boolean variable.

How different approaches are, can be seen in the articles of hackaday
and others (e.g. the possibly most advanced paper http://www.ganssle.com/debouncing.htm

#include <arduino.h>
// EIFR = 0x01

const byte        ledPin = 13;
byte              ledState = LOW;

const byte        interruptPin = 2;
      byte        interruptUsed = 0;
volatile boolean  edgeDetected = false;
volatile boolean  ISRactive = true;
volatile long     lastEdgemillis = 0;

void EdgeISR() {
  disableButtonISR();
  edgeDetected   = true;
}

void enableButtonISR(){
  Serial.println("ISR Active");
  edgeDetected = false;
  ISRactive    = true;
  // Now we clear a possibly outstanding interrupt for interruptUsed before attaching.
  // This is necessary, because it is not done in attachInterrupt() ...
  EIFR |= (1 << interruptUsed);           
  attachInterrupt(interruptUsed, EdgeISR, FALLING);
  }

void disableButtonISR(){
  detachInterrupt(interruptUsed);
  ISRactive   = false;
  }

void setup() {
  Serial.begin(115200);

  // Internal LED used just for visual feedback
  pinMode(ledPin, OUTPUT);
  digitalWrite(ledPin, ledState);  // not neccessary, but to be on the safe side

  // Button from interruptPin to ground
  pinMode(interruptPin, INPUT_PULLUP);
  // Recommended way to get the Interrupt No 
  interruptUsed = byte(digitalPinToInterrupt(interruptPin));
  // Attach Button to EdgeISR() and enable ISR
  enableButtonISR();
}



void loop() {
  // Loop until EdgeISR detects a falling edge
  if (edgeDetected){
    // In THIS example, we go into this part of if{} only once .. 
   // after one falling edge (and probably later again after re-enabling the button ISR)
    // but there are numerous ways to design the application
    // e.g. one could use a switch/case construct to do
    // different things or make it dependend on a counter or 
    // create a state machine ...
    edgeDetected = false;
    
    // This is where the "real" application would be going to start
    // In this example we do nothing than toggling the LED and ... 
    // ... wait, but could do a lot of things:
    lastEdgemillis = millis();
    Serial.println("ISR Inactive");
    ledState = !ledState;
    digitalWrite(ledPin, ledState);
    while(millis()-lastEdgemillis <= 200){}; 
    // This is the end of the application part
    
    // Now this is to call when we like to re-activate the button again 
    if (!ISRactive) {enableButtonISR();} 
  }
}
55

Yeah… I was exhausted, I'm taking a look through this thread again. People kept saying I shouldn't use interrupts but don't explain what I should use. Do you know what they want me to use instead?

56

But you are still polling
The only reason you have a interrupt there is to catch the case where the falling edge is a fleeting event which wouldn't be caught by a sub-millisecond (to pluck a number from the ether) main polling loop.

We're talking about a switch here.
Operated by a human.

Just let that sink in.

57

@thenoobjoker Sry I don't have handy the three URLs always recommended in this circumstance. Perhaps someone will supply the classics at hand.

But google

Arduino switch denouncing

and

Arduino two things at the same time

and

Arduino blink without delay

Stay busy with those for some time. It's hard until it's easy. :expressionless:

As for interrupts, yes no you shouldn't even think about it. At a noob or even better level of skill, interrupts are a distraction. Interrupts have their place, but require some care and attention that is well served by a solid footing with non-interrupt regular programming.

I use interrupts when necessary. I hardly use them at all. Logical conclusion: for what I do, interrupts are rarely necessary.

Certainly not for what you want to be up to.

I too recommend http://www.ganssle.com/debouncing.htm although it may be more deeper and wider than you need or might be able to fathom.

a7

58

Bunk. Reading and debouncing switches is dead simple. A monkey could do it. Detect a state change, register a switch event, do not poll again until the defined debounce period has elapsed.

Here is Arduino.cc's take on it:
https://www.arduino.cc/en/Tutorial/BuiltInExamples/Debounce

59

Alright, thanks a lot. Back to researching I go.

60

J'accuse.

"Debouncing"