I'm looking for advice or criticism on how to improve

Projects Programming
1

Hello there

I'm sure some may have seen me on here asking for help and thanks to those people I've done my most advanced project. I feel I've somewhat got a basic understanding now on how code works but there a lot I'm not sure on or don't even know exists.

Any advice is welcome just don't scold me too hard I'm only like 40hrs or less in :melting_face:

After all the comments, bar the butchery of my interupt routine anything elsere commended likestructures or?

#include <LiquidCrystal.h>

// initialize the library with the numbers of the interface pins
LiquidCrystal lcd(7, 8, 9, 10, 11, 12);

#include "pitches.h"

/*empire////////////////////////////////*/


#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 REST      0


// change this to make the song slower or faster
int tempo = 120;



// Ns of the moledy followed by the duration.
// a 4 means a quarter note, 8 an eighteenth , 16 sixteenth, so on
// !!negative numbers are used to represent dotted Ns,
// so -4 means a dotted quarter note, that is, a quarter plus an eighteenth!!
int M[] = {
  
  // Dart Vader theme (Imperial March) - Star wars 
  // Score available at https://musescore.com/user/202909/scores/1141521
  // The tenor saxophone part was used
  
  NOTE_A4,-4, NOTE_A4,-4, NOTE_A4,16, NOTE_A4,16, NOTE_A4,16, NOTE_A4,16, NOTE_F4,8, REST,8,
  NOTE_A4,-4, NOTE_A4,-4, NOTE_A4,16, NOTE_A4,16, NOTE_A4,16, NOTE_A4,16, NOTE_F4,8, REST,8,
  NOTE_A4,4, NOTE_A4,4, NOTE_A4,4, NOTE_F4,-8, NOTE_C5,16,

  NOTE_A4,4, NOTE_F4,-8, NOTE_C5,16, NOTE_A4,2,//4
  NOTE_E5,4, NOTE_E5,4, NOTE_E5,4, NOTE_F5,-8, NOTE_C5,16,
  NOTE_A4,4, NOTE_F4,-8, NOTE_C5,16, NOTE_A4,2,
  
  NOTE_A5,4, NOTE_A4,-8, NOTE_A4,16, NOTE_A5,4, NOTE_GS5,-8, NOTE_G5,16, //7 
  NOTE_DS5,16, NOTE_D5,16, NOTE_DS5,8, REST,8, NOTE_A4,8, NOTE_DS5,4, NOTE_D5,-8, NOTE_CS5,16,

  NOTE_C5,16, NOTE_B4,16, NOTE_C5,16, REST,8, NOTE_F4,8, NOTE_GS4,4, NOTE_F4,-8, NOTE_A4,-16,//9
  NOTE_C5,4, NOTE_A4,-8, NOTE_C5,16, NOTE_E5,2,

  NOTE_A5,4, NOTE_A4,-8, NOTE_A4,16, NOTE_A5,4, NOTE_GS5,-8, NOTE_G5,16, //7 
  NOTE_DS5,16, NOTE_D5,16, NOTE_DS5,8, REST,8, NOTE_A4,8, NOTE_DS5,4, NOTE_D5,-8, NOTE_CS5,16,

  NOTE_C5,16, NOTE_B4,16, NOTE_C5,16, REST,8, NOTE_F4,8, NOTE_GS4,4, NOTE_F4,-8, NOTE_A4,-16,//9
  NOTE_A4,4, NOTE_F4,-8, NOTE_C5,16, NOTE_A4,2,
  
};

// sizeof gives the number of bytes, each int value is composed of two bytes (16 bits)
// there are two values per note (pitch and duration), so for each note there are four bytes
int Ns = sizeof(M) / sizeof(M[0]) / 2;

// this calculates the duration of a whole note in ms
int wholenote = (60000 * 4) / tempo;

int divider = 0, ND = 0;


/*empire////////////////////////////////*/


// Defining frequency of each music note
#define NOTE_C4 262
#define NOTE_D4 294
#define NOTE_E4 330
#define NOTE_F4 349
#define NOTE_G4 392
#define NOTE_A4 440
#define NOTE_B4 494
#define NOTE_C5 523
#define NOTE_D5 587
#define NOTE_E5 659
#define NOTE_F5 698
#define NOTE_G5 784
#define NOTE_A5 880
#define NOTE_B5 988

// Music notes of the song, 0 is a rest/pulse
int notes[] = {
    NOTE_E4, NOTE_G4, NOTE_A4, NOTE_A4, 0,
    NOTE_A4, NOTE_B4, NOTE_C5, NOTE_C5, 0,
    NOTE_C5, NOTE_D5, NOTE_B4, NOTE_B4, 0,
    NOTE_A4, NOTE_G4, NOTE_A4, 0,

    NOTE_E4, NOTE_G4, NOTE_A4, NOTE_A4, 0,
    NOTE_A4, NOTE_B4, NOTE_C5, NOTE_C5, 0,
    NOTE_C5, NOTE_D5, NOTE_B4, NOTE_B4, 0,
    NOTE_A4, NOTE_G4, NOTE_A4, 0,

    NOTE_E4, NOTE_G4, NOTE_A4, NOTE_A4, 0,
    NOTE_A4, NOTE_C5, NOTE_D5, NOTE_D5, 0,
    NOTE_D5, NOTE_E5, NOTE_F5, NOTE_F5, 0,
    NOTE_E5, NOTE_D5, NOTE_E5, NOTE_A4, 0,

    NOTE_A4, NOTE_B4, NOTE_C5, NOTE_C5, 0,
    NOTE_D5, NOTE_E5, NOTE_A4, 0,
    NOTE_A4, NOTE_C5, NOTE_B4, NOTE_B4, 0,
    NOTE_C5, NOTE_A4, NOTE_B4, 0,

    NOTE_A4, NOTE_A4,
    //Repeat of first part
    NOTE_A4, NOTE_B4, NOTE_C5, NOTE_C5, 0,
    NOTE_C5, NOTE_D5, NOTE_B4, NOTE_B4, 0,
    NOTE_A4, NOTE_G4, NOTE_A4, 0,

    NOTE_E4, NOTE_G4, NOTE_A4, NOTE_A4, 0,
    NOTE_A4, NOTE_B4, NOTE_C5, NOTE_C5, 0,
    NOTE_C5, NOTE_D5, NOTE_B4, NOTE_B4, 0,
    NOTE_A4, NOTE_G4, NOTE_A4, 0,

    NOTE_E4, NOTE_G4, NOTE_A4, NOTE_A4, 0,
    NOTE_A4, NOTE_C5, NOTE_D5, NOTE_D5, 0,
    NOTE_D5, NOTE_E5, NOTE_F5, NOTE_F5, 0,
    NOTE_E5, NOTE_D5, NOTE_E5, NOTE_A4, 0,

    NOTE_A4, NOTE_B4, NOTE_C5, NOTE_C5, 0,
    NOTE_D5, NOTE_E5, NOTE_A4, 0,
    NOTE_A4, NOTE_C5, NOTE_B4, NOTE_B4, 0,
    NOTE_C5, NOTE_A4, NOTE_B4, 0,
    //End of Repeat

    NOTE_E5, 0, 0, NOTE_F5, 0, 0,
    NOTE_E5, NOTE_E5, 0, NOTE_G5, 0, NOTE_E5, NOTE_D5, 0, 0,
    NOTE_D5, 0, 0, NOTE_C5, 0, 0,
    NOTE_B4, NOTE_C5, 0, NOTE_B4, 0, NOTE_A4,

    NOTE_E5, 0, 0, NOTE_F5, 0, 0,
    NOTE_E5, NOTE_E5, 0, NOTE_G5, 0, NOTE_E5, NOTE_D5, 0, 0,
    NOTE_D5, 0, 0, NOTE_C5, 0, 0,
    NOTE_B4, NOTE_C5, 0, NOTE_B4, 0, NOTE_A4};

// Durations (in ms) of each music note of the song
// Quarter Note is 250 ms when songSpeed = 1.0
int durations[] = {
    125, 125, 250, 125, 125,
    125, 125, 250, 125, 125,
    125, 125, 250, 125, 125,
    125, 125, 375, 125,

    125, 125, 250, 125, 125,
    125, 125, 250, 125, 125,
    125, 125, 250, 125, 125,
    125, 125, 375, 125,

    125, 125, 250, 125, 125,
    125, 125, 250, 125, 125,
    125, 125, 250, 125, 125,
    125, 125, 125, 250, 125,

    125, 125, 250, 125, 125,
    250, 125, 250, 125,
    125, 125, 250, 125, 125,
    125, 125, 375, 375,

    250, 125,
    //Rpeat of First Part
    125, 125, 250, 125, 125,
    125, 125, 250, 125, 125,
    125, 125, 375, 125,

    125, 125, 250, 125, 125,
    125, 125, 250, 125, 125,
    125, 125, 250, 125, 125,
    125, 125, 375, 125,

    125, 125, 250, 125, 125,
    125, 125, 250, 125, 125,
    125, 125, 250, 125, 125,
    125, 125, 125, 250, 125,

    125, 125, 250, 125, 125,
    250, 125, 250, 125,
    125, 125, 250, 125, 125,
    125, 125, 375, 375,
    //End of Repeat

    250, 125, 375, 250, 125, 375,
    125, 125, 125, 125, 125, 125, 125, 125, 375,
    250, 125, 375, 250, 125, 375,
    125, 125, 125, 125, 125, 500,

    250, 125, 375, 250, 125, 375,
    125, 125, 125, 125, 125, 125, 125, 125, 375,
    250, 125, 375, 250, 125, 375,
    125, 125, 125, 125, 125, 500};


int melody[] = {
  NOTE_G4, NOTE_C5, NOTE_G4, NOTE_A4, NOTE_B4, NOTE_E4, NOTE_E4, 
  NOTE_A4, NOTE_G4, NOTE_F4, NOTE_G4, NOTE_C4, NOTE_C4, 
  NOTE_D4, NOTE_D4, NOTE_E4, NOTE_F4, NOTE_F4, NOTE_G4, NOTE_A4, NOTE_B4, NOTE_C5, NOTE_D5, 
  NOTE_E5, NOTE_D5, NOTE_C5, NOTE_D5, NOTE_B4, NOTE_G4, 
  NOTE_C5, NOTE_B4, NOTE_A4, NOTE_B4, NOTE_E4, NOTE_E4, 
  NOTE_A4, NOTE_G4, NOTE_F4, NOTE_G4, NOTE_C4, NOTE_C4, 
  NOTE_C5, NOTE_B4, NOTE_A4, NOTE_G4, NOTE_B4, NOTE_C5, NOTE_D5, 
  NOTE_E5, NOTE_D5, NOTE_C5, NOTE_B4, NOTE_C5, NOTE_D5, NOTE_G4, NOTE_G4, NOTE_B4, NOTE_C5, NOTE_D5,
  NOTE_C5, NOTE_B4, NOTE_A4, NOTE_G4, NOTE_A4, NOTE_B4, NOTE_E4, NOTE_E4, NOTE_G4, NOTE_A4, NOTE_B4,
  NOTE_C5, NOTE_A4, NOTE_B4, NOTE_C5, NOTE_A4, NOTE_B4, NOTE_C5, NOTE_A4, NOTE_C5, NOTE_F5,
  NOTE_F5, NOTE_E5, NOTE_D5, NOTE_C5, NOTE_D5, NOTE_E5, NOTE_C5, NOTE_C5,
  NOTE_D5, NOTE_C5, NOTE_B4, NOTE_A4, NOTE_B4, NOTE_C5, NOTE_A4, NOTE_A4,
  NOTE_C5, NOTE_B4, NOTE_A4, NOTE_G4, NOTE_C4, NOTE_G4, NOTE_A4, NOTE_B4, NOTE_C5
};

int noteDurations[] = {
  8, 4, 6, 16, 4, 8, 8, 
  4, 6, 16, 4, 8, 8, 
  4, 8, 8, 4, 8, 8, 4, 8, 8, 2,
  4, 6, 16, 4, 8, 8, 
  4, 6, 16, 4, 8, 8, 
  4, 6, 16, 4, 6, 16, 
  4, 6, 16, 8, 8, 8, 8, 
  2, 8, 8, 8, 8, 3, 8, 8, 8, 8, 8,
  2, 8, 8, 8, 8, 3, 8, 8, 8, 8, 8,
  4, 6, 16, 4, 6, 16, 4, 8, 8, 2,
  2, 8, 8, 8, 8, 3, 8, 2,
  2, 8, 8, 8, 8, 3, 8, 2,
  4, 6, 16, 4, 4, 2, 4, 4, 1
};

const float songSpeed = 1.0;

#define redLED 6
#define greenLED 5
#define blueLED 13

int red = 6; //select the pin for the red LED
int green =5;// select the pin for the green LED
int blue =13; // select the pin for the  blue LED


int CLK = 2;//CLK->D2
int DT = 3;//DT->D3
int SW = 4;//SW->D4
const int interrupt0 = 0;// Interrupt 0 在 pin 2 上
     
      volatile int  count = 0;//Define the count
      //This fixed the song issue volatile is crucial for interupts

int lastCLK = 0;//CLK initial value

int btn = 1;
int btncount;

int buzzerPin = A3;



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

   pinMode(btn,INPUT);
 //RGB LED set as output
 pinMode(redLED,OUTPUT);
 pinMode(greenLED,OUTPUT);
 pinMode(blueLED,OUTPUT);

 digitalWrite(redLED,LOW);
 digitalWrite(greenLED,LOW);
 digitalWrite(blueLED,LOW);


   pinMode(btn, INPUT);
 pinMode(buzzerPin, OUTPUT);
  noTone(buzzerPin);

  
   pinMode(SW, INPUT);
  analogWrite(SW, HIGH);
  pinMode(CLK, INPUT);
  pinMode(DT, INPUT);
  attachInterrupt(interrupt0, ClockChanged, CHANGE);//Set the interrupt 0 handler, trigger level change
  pinMode(btn, INPUT);

  
  // set up the LCD's number of columns and rows:
  lcd.begin(16, 2);
  // Print a message to the LCD.
  
  lcd.setCursor(5, 1);
  lcd.print("start");
  delay(5000); 
  lcd.clear();
    lcd.setCursor(4, 1);
    lcd.print("welcome");
    delay(2000);
   count = 20;
}

void loop() {


 /***********************  LEDS MENU ************************/

if(count <= 30 && count >= 15)
{
 
digitalWrite(redLED,LOW);
digitalWrite(greenLED,LOW);
digitalWrite(blueLED,LOW);
    lcd.setCursor(6, 0);
    lcd.print("Menu");
    lcd.setCursor(0, 1);
    lcd.print("30= LED 13=Song");
    delay(500);
    lcd.clear();

 /***********************  LEDS  ************************/
 
  while(count >=30)
  {
 digitalWrite(redLED,LOW);
 digitalWrite(greenLED,LOW);
 digitalWrite(blueLED,LOW);

  lcd.setCursor(3, 0);
  
  lcd.print("LED menu");
  delay(2000); 
  lcd.clear();

  //MENUABOVE
  lcd.setCursor(0, 1);
  lcd.print("39 for red");
   delay(3000); 
     lcd.clear();  
     while (count <= 39 && count >=35 )
     {
    delay(500); 
  lcd.clear();
    lcd.print("red");
    digitalWrite(redLED,HIGH);
 digitalWrite(greenLED,LOW);
 digitalWrite(blueLED,LOW);    

}     
   lcd.setCursor(0, 0);
    lcd.print("49 for green");
    delay(2000);
      lcd.clear();
    while (count <= 49 && count >=45 )
    {
     delay(500); 
  lcd.clear();
    lcd.print("Green");
     digitalWrite(redLED,LOW);
 digitalWrite(greenLED,HIGH);
 digitalWrite(blueLED,LOW);
 
}
  
  lcd.setCursor(1, 1);
  lcd.print("59 for blue");
   delay(2000);
      lcd.clear();
    while (count <= 59 && count >=55 )
    {
     delay(500); 
  lcd.clear();
    lcd.print("Blue");
     digitalWrite(redLED,LOW);
 digitalWrite(greenLED,LOW);
 digitalWrite(blueLED,HIGH);

}      
   
   lcd.setCursor(0, 0);
    lcd.print("69 for purple");
     delay(2000);
      lcd.clear();  
    while (count <= 69 && count >=65)
    {
     delay(500); 
  lcd.clear();
    lcd.print("Purple");
     digitalWrite(redLED,HIGH);
 digitalWrite(greenLED,LOW);
 digitalWrite(blueLED,HIGH);
 
}   

    lcd.setCursor(1, 1);
  lcd.print("79 for white");
   delay(2000);
      lcd.clear();
    while (count <= 79 && count >=75)
    {
     delay(500); 
  lcd.clear();
    lcd.print("White");
     digitalWrite(redLED,HIGH);
 digitalWrite(greenLED,HIGH);
 digitalWrite(blueLED,HIGH);
 
}  

   lcd.setCursor(0, 0);
    lcd.print("90 to menu");
       delay(3000);
      lcd.clear();
      while (count >= 85){
    lcd.print("going back");
    delay(2000);
   count = 20;
    lcd.clear();
    return;
     } 
    }   
}     

/*************SONG MENU + CODE*************/

 if(count <= 14 )
 {
lcd.setCursor(6, 0);
  lcd.print("Menu");
    lcd.setCursor(0, 1);
  lcd.print("9=cccp 5=pirate 1=em");
 
          digitalWrite(redLED,LOW);
          digitalWrite(greenLED,LOW);
          digitalWrite(blueLED,LOW);
  
  delay(1000);
  lcd.clear();
  
 }
 
/***************CCCP*********************/   
    
  if(count == 9 )
  {  
  // Execute once button has been released
  for ( int thisNote = 0; thisNote < sizeof( melody ) / 2; thisNote++ ) 
  {
    
    if (count != 9) 
    {
      // Break from loop once button is released    
       lcd.print("reeeeeed");
       delay(1000);
      count = 14;
      break;        
    }
  
    int noteDuration = 2000 / noteDurations[thisNote];
    tone( buzzerPin, melody[thisNote], noteDuration );      
    lcd.clear();
    int pauseBetweenNotes = noteDuration * 1.30;
    delay( pauseBetweenNotes );

      digitalWrite(redLED,HIGH);
digitalWrite(greenLED,LOW);
 digitalWrite(blueLED,LOW);   
    }
  }
 
/***************CCCP*********************/   


/**************PIRATES*****************/

   if(count == 5 ){

  const int totalNotes = sizeof(notes) / sizeof(int);
  // Loop through each note
  for (int i = 0; i < totalNotes; i++)
{

          if(count != 5)
          {
      delay(1000); 
  lcd.clear();
    lcd.print("yarrrr");
    count = 14;
    
    break;
    }
    
    const int currentNote = notes[i];
    float wait = durations[i] / songSpeed;
    // Play tone if currentNote is not 0 frequency, otherwise pause (noTone) 
  digitalWrite(redLED,HIGH);
 digitalWrite(greenLED,LOW);
 digitalWrite(blueLED,HIGH);
    if (currentNote != 0)
    {
      tone(buzzerPin, notes[i], wait); // tone(pin, frequency, duration) 
    }
    else
    {
      noTone(buzzerPin);
    }
    // delay is used to wait for tone to finish playing before moving to next loop
    delay(wait); 
  }
}


/***************PIRATES*******************/

/***************IMPERIAL*****************/

/*
if(count == 5 ){
   
   
   // iterate over the Ns of the M. 
  // Remember, the array is twice the number of Ns (Ns + durations)
  for (int TN = 0; TN < Ns * 2; TN = TN + 2) {



          if(count != 5)
          {
      delay(1000); 
  lcd.clear();
    lcd.print("yarrrr");
    count = 14;
    
    break;
    }



    // calculates the duration of each note
    divider = M[TN + 1];
    if (divider > 0) {
      // regular note, just proceed
      ND = (wholenote) / divider;
    } else if (divider < 0) {
      // dotted Ns are represented with negative durations!!
      ND = (wholenote) / abs(divider);
      ND *= 1.5; // increases the duration in half for dotted Ns
    }

    // we only play the note for 90% of the duration, leaving 10% as a pause
    tone(buzzerPin, M[TN], ND*0.9);

    // Wait for the specief duration before playing the next note.
    delay(ND);
    
    // stop the waveform generation before the next note.
    noTone(buzzerPin);
  }
}

*/
// this code works there just not enough memeory rn
/***************IMPERIAL*****************/

//VoidLoop end
}
//

//The interrupt handlers
void ClockChanged()
{
  int clkValue = digitalRead(CLK);//Read the CLK pin level
  int dtValue = digitalRead(DT);//Read the DT pin level
  if (lastCLK != clkValue)
  {
    lastCLK = clkValue;
    count += (clkValue != dtValue ? 1 : -1);//CLK and inconsistent DT + 1, otherwise - 1

   delay(500); 
  lcd.clear();
    lcd.print(count);
   
   
  }
}
2

image

4

Start by describing what your sketch should do

5

Oh right sorry so its a 2 part menu. all parts are divided and labelled top and bottom. Primarily count = a rotary encoder used to select 1 of the 5 LEDS or go to different menus to the select a song which can be stopped or reset if moved.

6

I don't know how to make text stay on LCD without delay + clear or it goes everywhere.

7
  1. Does it work? (i.e. do what you want it to do?)
  2. Why are you delaying and updating LCD in an interrupt? DON'T!!!
//The interrupt handlers
void ClockChanged()
{
  int clkValue = digitalRead(CLK);//Read the CLK pin level
  int dtValue = digitalRead(DT);//Read the DT pin level
  if (lastCLK != clkValue)
  {
    lastCLK = clkValue;
    count += (clkValue != dtValue ? 1 : -1);//CLK and inconsistent DT + 1, otherwise - 1

    delay(500);
    lcd.clear();
    lcd.print(count);
  }
}
  1. What Arduino are you using for this?
8

It works fully as i intended, its to stop the text going all over the LCD.

im using UNO.

9

You should not perform any delays or I/O which requires interrupts inside of an interrupt. Also, any multibyte variables shared between interrupt and non-interrupt code should only be accessed at non-interrupt level with interrupts disabled or you risk corruption.

These are common mistakes of people who don't understand interrupts and how to properly use them.

10

I've read about this i just don't see how it would work for me in my code, its also language like that is hard for me to understand as im new

like this

11

forgot to say thanks for the input btw greatly appreciated

12

Lastly the delay is solely for the LCD text to not spam all over the LCD

13

I get it. But interrupts are an advanced concept. They are appropriate for applications such as rotary encoders but you just can't add any code you want into an interrupt. Interrupts are used to process time critical events and should be as short and efficient as possible.

14

All other interrupts are turned off during an ISR execution so any functions that depend on interrupts, like delay() or print(), will not work right and will introduce "bugs" that can be hard to resolve.

15

noted thanks, i get things given to me at work to roughly design to increase my knowledge of components and code so i have to work around things i don't get which is why im on here quite alot :sweat_smile: :melting_face:

16

yeah ive had a few of these although many are ironed out using the delay clear which i know is equivalent of putting a plaster on a shotgun wound.

17

No scolding, we all started nowhere. And 40 hours is nothing. 40 weeks of diligent learning might really put you somewhere.

There's ppl here 40 years in who still find they can learn something everyday.

Hard until it is easy.

But I must ask, what parts of the sketch you are showing us here did you have anything to do with?

It really is something like I've never seen before. I've spent time looking at it or a similar one that was on another thread, still don't know how it works or why it didn't work when it wasn't. Working.

If you found it somewhere and beat on it awhile, I would say throw it in the trash.

If you wrote it mostly yourself, you're way over your skis trying to be creative. You should slow your roll, reduce your expectations and get a firm(er) grip on very basic coding, probably most effectively by reading reading reading and experimenting with baby steps… getting creative is some good number of hours from where you are.

IMO & YMMV always.

a7

18

let me give you 4 advices:

  • make your life easier and press CTRL-T in your IDE. Does it look nice now?
  • your program is worthless if you don't have a clear specification what it does. If you want me to read your code ... let me read your specification so I know up front what your program should do.
  • you are using arrays - fine ... next step learn about c++ structures
  • put your music sketch beside. Open the IDE example Blink without delay and expand it to blink three LEDs in different intervals like 1100ms, 2200ms and 3300ms. If this works - see again your music code and improve it. I guess you will start from scratch.

Take your time - hope to hear from you in 40 hours again :wink:

19

An efficient way of storing a melody would be to specify the frequency and the duration of a tone directly and to store these values in program memory.

struct Tone {
  uint16_t const frequency;
  uint16_t const duration;
};

Tone const melody[] PROGMEM = {{58, 4}, {73, 4}, {277, 16}};

We can retrieve the melody from program memory as follows.

for (Tone const& toneP: melody) {
  Tone toneR{};
  memcpy_P(&toneR, &toneP, sizeof(Tone));

  // Do something with `toneR.frequency` and `toneR.duration`.
}

This may not be the most user friendly method to write a melody, this is probably why you introduced the predefined notes.

We could write a program that generates the melody array on a computer, which we then can copy to the sketch. Alternatively, we could let the program generate a file that can be included from the sketch.

The predefined values seem to be based on a generic formula, described here. In C++, this formula looks as follows:

unsigned int frequency(int const note) {
  return 440 * pow(2, note / 12.0) + 0.5;
}

We can introduce user friendly notations (e.g., A#1, D2, C#4, etc.) with some additional helper functions.

size_t lcp(char const a[], char const b[]) {
  size_t i;
  for (i = 0; a[i] and b[i] and a[i] == b[i]; i++);
  return i;
}

int notationToNote(char const str[]) {
  static char const* const notation[] = {
    "C", "C#", "D", "D#", "E", "F", "F#", "G", "G#", "A", "A#", "B"};
  size_t len = strlen(str) - 1;
  for (size_t i = 0; i < 12; i++) {
    if (lcp(notation[i], str) == len) {
      return 12 * (str[len] - '0') + i - 57;
    }
  }
  return 0;
}

The notationToNote function can now be used to write a melody.

struct Tone {
  char const* const notation;
  unsigned long const duration;
};

int main() {
  Tone const melody[] = {{"A#1", 4}, {"D2", 4}, {"C#4", 16}};
  for (Tone const& tone: melody) {
    // Do something with `frequency(notationToNote(tone.notation))` and `tone.duration`.
  }
}
20

thanks for the amazing input, i out sourced most of the notes etc as i cant read them :melting_face:

definitely adding this to my study folder and will try to implement it.

hopefully.. :sweat_smile:

21

will do ive been trying to add this but ive not had much spare time at work, hopefully when i get some free time ill post an updated version :melting_face:

many thanks

red