Need some help figuring out my code

This helps a lot! It's 12pm right now so I'll try to finish my code tomorrow. Thanks for the help. I have no clue as to how we got 80 posts.

See reply #2

I though #4 was the clincher. :rofl:

1 Like

and #10, and #28

/*************************************************
 * Public Constants define notes
 *************************************************/

#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




//define ledpins
const int rLed1 = 0;
const int rLed2 = 1;
const int rLed3 = 2;
const int rLed4 = 3;
const int rLed5 = 4;
const int rLed6 = 5;
const int wLed1 = 6;
const int wLed2 = 7;
const int wLed3 = 8;
const int wLed4 = 9;
const int wLed5 = 12;
const int wLed6 = 13;
const int buttonPin = 10;
const int buzzerPin = 11;
//define intervals
const int blinkDuration = 2000;
const int buttonInterval = 300;
const int wLedInterval = 2000;
const int wLedInterval2 = 6000;
const int rLedInterval = 4000;
const int rLedInterval2 = 8000;
const int aLedInterval = 10000;
//define millis
unsigned long currentMillis = 0;
unsigned long previousRLedMillis = 0;
unsigned long previousRLedMillis2 = 0;
unsigned long previousWLedMillis = 0;
unsigned long previousWLedMillis2 = 0;
unsigned long previousALedMillis = 0;
unsigned long previousButtonMillis = 0;
unsigned long previousMelodyMillis = 0;
//define pinstates
byte rLedState = LOW;
byte wLedState = LOW;
byte aLedState = LOW;
byte buttonState = LOW;
// melody of the song
int melody[] = {
  NOTE_E5, NOTE_E5, NOTE_E5,
  NOTE_E5, NOTE_E5, NOTE_E5,
  NOTE_E5, NOTE_G5, NOTE_C5, NOTE_D5,
  NOTE_E5,
  NOTE_F5, NOTE_F5, NOTE_F5, NOTE_F5,
  NOTE_F5, NOTE_E5, NOTE_E5, NOTE_E5, NOTE_E5,
  NOTE_E5, NOTE_D5, NOTE_D5, NOTE_E5,
  NOTE_D5, NOTE_G5

};
// tempo of the song
int tempo[] = {
  8, 8, 4,
  8, 8, 4,
  8, 8, 8, 8,
  2,
  8, 8, 8, 8,
  8, 8, 8, 16, 16,
  8, 8, 8, 8,
  4, 4

};


// make pins input/output
void setup() {
  Serial.begin(6900);
  Serial.println("Starting kerstboomprog.ino");
  Serial.print("buttonState = ");
  Serial.println(buttonState);

  pinMode(rLed1, 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(buttonPin, INPUT_PULLUP);
  pinMode(buzzerPin, OUTPUT);
}


void loop() {
//define currentmillis and calls functions
  currentMillis = millis();

  readButton();
  updatewLedState();
  updatewLedState2();
  updaterLedState();
  updaterLedState2();
  switchLeds();
  song();
 }
//updates ledstate of white leds
void updatewLedState() {
 if (buttonState == LOW){
   if (wLedState == LOW){
     if (currentMillis - previousWLedMillis >= wLedInterval){
       wLedState = HIGH;
       previousWLedMillis += wLedInterval;
      }
    }
    else {
     if (currentMillis - previousWLedMillis >= blinkDuration) {
       wLedState = LOW;
       previousWLedMillis += blinkDuration;
      }
    }
  }
}
//second update of white leds
void updatewLedState2() {
 if (buttonState == LOW){
   if (wLedState == LOW){
     if (currentMillis - previousWLedMillis2 >= wLedInterval2){
       wLedState = HIGH;
       previousWLedMillis2 += wLedInterval2;
      }
    }
    else {
     if (currentMillis - previousWLedMillis2 >= blinkDuration) {
       wLedState = LOW;
       previousWLedMillis2 += blinkDuration;
      }
    }
  }
}
//update red leds
void updaterLedState() {
 if (buttonState == LOW){
   if (rLedState == LOW){
     if (currentMillis - previousRLedMillis >= rLedInterval){
       rLedState = HIGH;
       previousRLedMillis += rLedInterval;
      }
    }
    else {
     if (currentMillis - previousRLedMillis >= blinkDuration) {
       rLedState = LOW;
       previousRLedMillis += blinkDuration;
      }
    }
  }
}

//updates second red led state
void updaterLedState2() {
 if (buttonState == LOW){
   if (rLedState == LOW){
     if (currentMillis - previousRLedMillis2 >= rLedInterval2){
       rLedState = HIGH;
       previousRLedMillis2 += rLedInterval2;
      }
    }
    else {
     if (currentMillis - previousRLedMillis2 >= blinkDuration) {
       rLedState = LOW;
       previousRLedMillis2 += blinkDuration;
      }
    }
  }
}
//updates state for all leds
void updateaLedState() {
 if (buttonState == LOW){
   if (aLedState == LOW){
     if (currentMillis - previousALedMillis >= aLedInterval){
       aLedState = HIGH;
       previousALedMillis += aLedInterval;
      }
    }
    else {
     if (currentMillis - previousALedMillis >= blinkDuration) {
       wLedState = LOW;
       previousALedMillis += blinkDuration;
      }
    }
  }
}

// turns on the leds
void switchLeds() {

      digitalWrite(rLed1, rLedState);
      digitalWrite(rLed2, rLedState);
      digitalWrite(rLed3, rLedState);
      digitalWrite(rLed4, rLedState);
      digitalWrite(rLed5, rLedState);
      digitalWrite(rLed6, rLedState);
      digitalWrite(wLed1, wLedState);
      digitalWrite(wLed2, wLedState);
      digitalWrite(wLed3, wLedState);
      digitalWrite(wLed4, wLedState);
      digitalWrite(wLed5, wLedState);
      digitalWrite(wLed6, wLedState);
      digitalWrite(rLed1, aLedState);
      digitalWrite(rLed2, aLedState);
      digitalWrite(rLed3, aLedState);
      digitalWrite(rLed4, aLedState);
      digitalWrite(rLed5, aLedState);
      digitalWrite(rLed6, aLedState);
      digitalWrite(wLed1, aLedState);
      digitalWrite(wLed2, aLedState);
      digitalWrite(wLed3, aLedState);
      digitalWrite(wLed4, aLedState);
      digitalWrite(wLed5, aLedState);
      digitalWrite(wLed6, aLedState);

}
//checks for button
void readButton(){
  if(millis() - previousButtonMillis >= buttonInterval){
    if(digitalRead(buttonPin) == LOW){
      buttonState = ! buttonState;
      Serial.print ("buttonState = ");
      Serial.println (buttonState);
    }
  }
}
 //define the pausebetween notes and note duration
void song(){
  if(buttonState == LOW){
  int size = sizeof(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 / tempo[thisNote];

      tone(buzzerPin, melody[thisNote], noteDuration);

      // to distinguish the notes, set a minimum time between them.
      // the note's duration + 30% seems to work well:
      unsigned long pauseBetweenNotes = noteDuration * 1.30;
      if (currentMillis - previousMelodyMillis >= pauseBetweenNotes) {
        previousMelodyMillis = currentMillis;
        noTone(buzzerPin);
   
      }
    }
  }
}
// make buzzer play note
void buzzer(int targetPin, long frequency, long length) {
  if(buttonState == LOW){
  digitalWrite(buzzerPin, HIGH);
  long delayValue = 1000 / frequency / 2; // calculate the delay value between transitions
  unsigned long previousMillis2 = 0;
  //// 1 second's worth of microseconds, divided by the frequency, then split in half since
  //// there are two phases to each cycle
  long numCycles = frequency * length / 1000; // calculate the number of cycles for proper timing
  //// multiply frequency, which is really cycles per second, by the number of seconds to
  //// get the total number of cycles to produce
  for (long i = 0; i < numCycles; i++) { // for the calculated length of time...
    if(currentMillis - previousMillis2>= delayValue){
      previousMillis2 = currentMillis;
    digitalWrite(targetPin, HIGH); // write the buzzer pin high to push out the diaphram
    }
    if(currentMillis - previousMillis2>= delayValue){
      previousMillis2 = currentMillis;
    digitalWrite(targetPin, LOW); // write the buzzer pin high to push out the diaphram
    }
  }
  digitalWrite(buzzerPin, LOW);

}
}


I'm stuck now. The code compiles but when I try it out it just does nothing close to what I want (turn white leds on for 2s then red for 2 then white for 2 then red for 2 then all for 2 repeat, and play the song at the same time once the button is pressed)

I stole the song from someone else but I feel this might be what is destroying the code. I'll have to write the song myself just using the tone function for simplicitys sake

does someone see what I did wrong here?

The code following that (the function buzzer() ) is not used; that code can be removed.

But your problem is in song(), which rapidly (nearly instantly) plays the entire song from start to finish every time you call it - you may hear some clicking or noise, but no tune?

song() has to be re-written to take a next step in the song, be it a note or a rest. One step per time you call it, and it has to handle timing the note. Not the entire song in a for loop.

You will have to follow the ideas elaborated upon here:

I didn't look at the other things the are supposed to happen simultaneous, the blonking of the LEDs, but the song() function as you have it does not fit the model wherein the loop() calls a bunch functions every time around (very frequently) and those functions mostly say "not time to do anything" OR take one step into whatever process they are performing. One step.

I'm too lazy to reread the entire thread, but I cannot be the only nor first person to rant about how to do (appear to be doing) more than one thing at a time.

I may have said it will take some work, an AHA! moment... then it will be easy easier.

It is a technique that supports a hefty fraction of the cool things ppl do with these tiny machines.

a7

I did in fact use the multiplethingsatonce example as a basis for my leds. I'll rewrite the song() and try again. I already expected that to be the problem.

OK, so a "for" loop can - only - ever be used to set up something that is essentially instantaneous and will not take any time other than the machine instructions themselves.

The whole point about this "doing many things at the same time" approach is never waiting for anything. So you cannot use a "for" loop in the code for anything that will take any time at all to perform.

You must "unravel" the "for" loop in two steps. The first is that the index of that loop is a "state variable" which is used in the piece of your code where the "for" loop was in exactly the same manner, but is only incremented when the second step is complete.

This second step is to see whether the action that was in the "for" loop has occurred or is completed. If it has not, then nothing happens, the loop variable is not advanced.

So in your code, you do not specify a duration in the tone() function. You re-write that function in the same way as the LED patterns, using millis() to determine when the tone has completed. You either go on to set the next tone of use noTone() for a pause.

OK, I think you have figured that out. Salient points - nothing in the code, including no function such as tone() with a duration, or delay() - that requires any time at all to perform. If you think something needs to take any time, then you need to figure out how to break it up.

3 Likes

My sensor shield appears to be broken, so I can't even properly try my code. For now, I'm dropping the buzzer since the deadline is Tuesday. I'll keep working on it, but I can't test it properly because pins 8-13 are dead on my sensor shield… Thanks for the advice, this will help a lot in future projects as they'll keep getting bigger and harder.

I noticed you have

  Serial.begin(6900);

You probably meant 9600, but you can use 21st century band rates like 115200.

Your readButton routine dosn't work. It makes no use of previousButtonMillis, but that isn't the end of its problems. I ended up just writing it correctly after having placed yours in isolation to find why it was not functioning.

// just the button thing

byte buttonState;

const byte buttonPin = 10;
const int buttonInterval = 25;

void setup() {
  Serial.begin(115200);
  Serial.println("Starting kerstboomprog.ino");
  Serial.print("buttonState = ");
  Serial.println(buttonState);

  pinMode(buttonPin, INPUT_PULLUP);
  pinMode(LED_BUILTIN, OUTPUT);
}

void loop() {
  readButton();   // 

  digitalWrite(LED_BUILTIN, buttonState);
}

//checks for button
void readButton()
{
  static byte previousButton = HIGH;
  static unsigned long previousButtonMillis = 0;

  unsigned long now = millis();

  if (now - previousButtonMillis < buttonInterval)
    return;   // too soon to look at the button again

  byte currentButton = digitalRead(buttonPin);
  if (currentButton != previousButton) {
    previousButtonMillis = now;
    previousButton = currentButton;

    if(currentButton) {
      buttonState = !buttonState;
      Serial.print ("buttonState = ");
      Serial.println (buttonState);
    }
  }
}

Play with it here:

Look at the differences to the original. Any defects were probably masked by the time your other functions take, so perhaps they should not be so characterized. Nevertheless it is good to have it work correctly as those circumstances might change and leave you looking around for what now has broken your code.

a7

Thank you! I see what I did wrong there, and yes I meant 9600 since our teacher forces us to use that… He himself is teaching us from watching tutorials on YT… Anyway. I appreciate it!

Seriously? WTH?

Yup, the funny thing is he makes us use j for index because i looks too much like 1... Laughable (i'm 99% sure my arduino is better than his and I'm pretty bad at arduino)

Obviously a newcomer.
The rest of us use j because identifiers i to n were automatically integers in FORTRAN.

1 Like

And of course in mathematics generally.

a7

But sparkies confuse things, and use j for the square root of -1 :open_mouth:

If my math teachers hears that she's boutta beat the shit out of him i think. Fun fact a 90° triangle (don't know the English word for it) with side 1 and i has an hypotenuse of 0

Imagine that!

a7

1 Like

Meanwhile, back at the code you posted

void switchLeds() {

      digitalWrite(rLed1, rLedState);  // and all red LEDs <- rLedState
...
      digitalWrite(wLed1, wLedState); // and all white LEDs <- wLedState
...
      digitalWrite(rLed1, aLedState); // and all red and white LEDs <- aLedState
...
      digitalWrite(wLed1, aLedState);
...
}

*aLedState* is never changed from its initial value; the function *updateaLedState*() is not used by your program.

Since aLedState has the last word in the switchLeds() function, so to speak, the LEDs will, depending on how you've wired them, always be on or off. All of them.

Technically the LEDs will briefly be switched according to rLedState and wLedState until overridden by aLedState. Briefly. Like too very fast to see by eye.

You have specified a relatively simple desired pattern:

turn white leds on for 2s then red for 2 then white for 2 then red for 2 then all for 2 repeat,

So what is this "all LEDs" code for, and why do the red and white LEDs each need two functions and two calls from loop()?

The red/white switching and timing can and should be expressed in one function that gets called over and over very frequently, usually doing nothing but when it is time, turn off the red and turn on the white or vice versa.

It is basically BWOD - blinking an LED without delay, only instead of blinking (alternate between ON and OFF) you are turning on the red (and off the white) or off the red (and on the white) LEDs.

Code writes itself. I think you can figure this out.

a7

Wow, i completely missed that. And you're right I should just do all of that in a function. I'll change it tomorrow