Completely useless first program

Ive just completed my first PROPER program, in other words a
program that does more than just turns on and off an LED every
half a second. It turns an LED off when you send a 1 to it and off
when you send a 0 to it. How useless! Whats wierd is I am proud
of it, no matter how useless it is. ;D ;D ;D ;D

It is loosely based on the pysicalpixel in the learning section,
however hugely and pointlessly elaborated.

/*
101 command lines! Simply program,
open serial monitor on 9600 bpm
and follow instructions
*/


const int ledPin = 13;
int incomingByte;     
void setup() {
  Serial.begin(9600);
  pinMode(ledPin, OUTPUT);
  Serial.println("                                                                                                            Automatic LED state changer");
  Serial.println();
  Serial.println("Type 1 to turn LED on");
  Serial.println("Type O to turn LED off");
  Serial.println("All other numbers will be ignored and turn LED off.");
}

void loop() {
  
  if (Serial.available() > 0) {
    incomingByte = Serial.read();
  }
    if (incomingByte == '1') {
      digitalWrite(ledPin, HIGH);
      Serial.println("LED state set as HIGH");
      Serial.println("LED state 1");
      delay(500);
      Serial.println();
    
    }
      if (incomingByte == '0') {
      digitalWrite(ledPin, LOW);
   Serial.println("LED state set as LOW");
        Serial.println("LED state 0");
      delay(500);
    Serial.println();
      }
       if (incomingByte == '2') {
         Serial.println("Number 2 not used in program.");
      Serial.println("Command not understood");
   digitalWrite(ledPin, LOW);
       Serial.println("      LED automatically set to LOW");
    Serial.println();
      delay(1000);
       }
             if (incomingByte == '3') {
                digitalWrite(ledPin, LOW);
                Serial.println("Number 3 not used in program.");
      Serial.println("Command not understood.");
            Serial.println("      LED automatically set to LOW.");
      delay(1000);
      Serial.println();
   
    }
           if (incomingByte == '4') {
             Serial.println("Number 4 not used in program.");
      Serial.println("Command not understood");
       digitalWrite(ledPin, LOW);
             delay(1000);
      Serial.println("      LED automatically set to LOW");
      Serial.println();
    }
           if (incomingByte == '5') {
             Serial.println("Number 5 not used in program.");
      Serial.println("Command not understood");
       digitalWrite(ledPin, LOW);
 Serial.println("           LED automatically set to LOW");
       delay(1000);
       Serial.println();
    }
           if (incomingByte == '6') {
             Serial.println("Number 6 not used in program.");
      Serial.println("Command not understood");
       digitalWrite(ledPin, LOW);
     Serial.println("       LED automatically set to LOW");
      delay(1000);
      Serial.println();
    }
           if (incomingByte == '7') {
             Serial.println("Number 7 not used in program.");
      Serial.println("Command not understood");
     digitalWrite(ledPin, LOW);
           Serial.println("      LED automatically set to LOW");
          delay(1000);
      Serial.println();
    }
           if (incomingByte == '8') {
             Serial.println("Number 8 not used in program.");
      Serial.println("Command not understood");
       digitalWrite(ledPin, LOW);
    Serial.println("      LED automatically set to LOW");
      delay(1000);
      Serial.println();
    }
           if (incomingByte == '9') {
             Serial.println("Number 9 not used in program.");
      Serial.println("Command not understood");
       digitalWrite(ledPin, LOW);
            Serial.println("      LED automatically set to LOW");
      delay(1000);
      Serial.println();
    }
           
    }

The whole reason I put this on here is not to show off what a useless
program I made, but to learn more, as that is why I made it in the
first place: to learn. Is there any way I could improve it, or do the
same function in less than 101 lines?

Thanks, Onions.

Congratulations. It does what you want, it doesn't spit on your shoes or goes up in smoke. That's the definition of success and nothing to be ashamed of.

Two small comments:

First learn to use the # button when posting code. It makes it more readable and appears mysterious grey. You can even edit your orginal posting by clicking on "modify", then select your code and click on the button with the # in the middle of the line under your subject. A tag will between [ ] will be added before and after your code.

Second, your coding with the many if-statements is the greatest way to do these kind of things. It seems you want to handle all cases except 0 and 1 with an error message. This is the perfect time to learn about ....
... the one and only ...
... most wonderful ...
... *** switch statement with its cute case bunnies and the daredevil breaks ***

switch (incomingByte) {
   case '1':
      digitalWrite(ledPin, HIGH);
      Serial.println("LED state set as HIGH");
      Serial.println("LED state 1");
      Serial.println();
      delay(500);
      break;  // Never forget the break at the end of a case

   case '0':
      digitalWrite(ledPin, HIGH);
      Serial.println("LED state set as HIGH");
      Serial.println("LED state 1");
      delay(500);
      Serial.println();
      break;  // Never forget the break at the end of a case
  
 default:
        Serial.print (incomingByte);
        Serial.println (" is a shifty character and not welcome in this town.");
        Serial.println("Command not understood");
        digitalWrite(ledPin, LOW);
        Serial.println("      LED automatically set to LOW");
        delay(1000);
}

Replace all your if( incomingByte ...) blocks with the switch statement above.

Korman

you should be proud !

one possible improvement:
you can use a case statement with the 1 and 0 command, then default to the error message if it's another number. something like :

switch (incomingByte) {
case 0:
Serial.println(" case 0 "); <add more statements; here if you need them>
break;
case 1:
Serial.println(" case 1 ");
break;
default:
// if nothing else matches, do the default
Serial.print(" case invalid number : ");
Serial.println(incomingByte);
}

with programming, when you notice you repeat yourself there often is
another, more terse way to write it...

now how about a program that reacts to a number with a rhythmic flashing ? i.e. 1 is 1 sec on, then 9 sec off, 2 = 2 sec on 8 sec off etc.

whoops, I'm too slow a typist :smiley:

I'm very new to Arduino but have experience in other languages so just thought I'd chime in.

To OP, using switch is a great way to cut down on your code for sure. But if you're not comfortable with switches or you just don't want to use it you could also use a multi-conditional if statement like so...

const int ledPin = 13;
int incomingByte;    
void setup() {
 Serial.begin(9600);
 pinMode(ledPin, OUTPUT);
 Serial.println("                                                                                                            Automatic LED state changer");
 Serial.println();
 Serial.println("Type 1 to turn LED on");
 Serial.println("Type O to turn LED off");
 Serial.println("All other numbers will be ignored and turn LED off.");
}

void loop() {
 
 if (Serial.available() > 0) {
   incomingByte = Serial.read();
 }
   if (incomingByte == '1') {
     digitalWrite(ledPin, HIGH);
     Serial.println("LED state set as HIGH");
     Serial.println("LED state 1");
     delay(500);
     Serial.println();
   
   }
     if (incomingByte == '0') {
     digitalWrite(ledPin, LOW);
  Serial.println("LED state set as LOW");
       Serial.println("LED state 0");
     delay(500);
   Serial.println();
     }
      if (incomingByte != '0' || incomingByte != '1') {
        Serial.println("Key pressed not used in program.");
     Serial.println("Command not understood");
  digitalWrite(ledPin, LOW);
      Serial.println("      LED automatically set to LOW");
   Serial.println();
     delay(1000);
      }
}

The last if is basically saying if incoming byte IS NOT a zero OR incoming byte IS NOT a 1 then do this cause they hit a wrong key.

You can separate conditions with && for AND and || for OR as much as you like so: let's say you tell someone to pick 2 random numbers but you want them both to be even or both to be odd. You could do something like this...

// pseudo code

if( (number1 == even && number2 == even) || (number1 == odd && number2 == odd) )  {
  // They did it right, do something here
} else {
  // they picked one even and one odd. try again
}

Nice work! Hope you're having fun and learning.

Thanks everyone! I have edited the program and leared lots.
After the suggestion from raalst I have made another program
to do a rythmic flashing of the input number as the delay in
seconds between high and low and visa versa. Then I tried editing
it down to much shorter, but it failed. Something else to work on!

Here is the time delay program:

/*
104 command lines! Simply program,
open serial monitor on 9600 bpm
and follow instructions
*/


const int ledPin = 13;
int incomingByte;     
void setup() {
  Serial.begin(9600);
  pinMode(ledPin, OUTPUT);  
  Serial.println("                                                                                                                         LED flasher");
  Serial.println();
  Serial.println("Type a number to set the LED flashing for that ammount of time");
}
void loop() {
  
  if (Serial.available() > 0) {
    incomingByte = Serial.read();
  }
    if (incomingByte == '1') {
      Serial.println("Flash interval of 1 second.");
      Serial.println();
     digitalWrite(ledPin, HIGH);
     delay(1000);
     digitalWrite(ledPin, LOW);
     delay(1000);
    }
        if (incomingByte == '0') {
        digitalWrite(ledPin, LOW);
        Serial.println("LED state set as LOW");
        Serial.println("LED state 0");
        Serial.println();
      }
        if (incomingByte == '2') {
        Serial.println("Flash interval of 2 seconds.");
        digitalWrite(ledPin, LOW);
        delay(2000);
        digitalWrite(ledPin, HIGH);
        delay(2000);
        Serial.println();
       }
        if (incomingByte == '3') {
        digitalWrite(ledPin, LOW);
        Serial.println("Flash interval of 3 seconds.");
        Serial.println();
        delay(3000);
        digitalWrite(ledPin, HIGH);
        delay(3000);
    }
        if (incomingByte == '4') {
        Serial.println("Flash interval of 4 seconds.");
        digitalWrite(ledPin, LOW);
        delay(4000);
        digitalWrite(ledPin, HIGH);
        delay(4000);
        Serial.println();
    }
        if (incomingByte == '5') {
        Serial.println("Flash interval of 5 seconds.");
        Serial.println();
        digitalWrite(ledPin, LOW);
        delay(5000);
        digitalWrite(ledPin, HIGH);
        delay(5000);
    }
        if (incomingByte == '6') {
        Serial.println("Flash interval of 6 seconds.");
        Serial.println();
        digitalWrite(ledPin, LOW);
        delay(6000);
        digitalWrite(ledPin, HIGH);
        delay(6000);
    }
        if (incomingByte == '7') {
        Serial.println("Flash interval of 7 seconds.");
        Serial.println();
        digitalWrite(ledPin, LOW);
        delay(7000);
        digitalWrite(ledPin, HIGH);
        delay(7000);
    }
        if (incomingByte == '8') {
        Serial.println("Flash interval of 8 seconds.");
        Serial.println();
        digitalWrite(ledPin, LOW);
        delay(8000);
        digitalWrite(ledPin, HIGH);
        delay(8000);
    }
        if (incomingByte == '9') {
        Serial.println("Flash interval of 9 seconds.");
        Serial.println();
        digitalWrite(ledPin, LOW);
        delay(9000);
        digitalWrite(ledPin, HIGH);
        delay(9000);
    }
           
    }

I tried editing it down to just the following, however it stops working
when any number is put in...

const int ledPin = 13;   
int incomingByte;
int outputValue;
void setup() {
  Serial.begin(9600);
  
}

void loop() {
if (Serial.available() > 0) {
    incomingByte = Serial.read();
  }
  outputValue = map(incomingByte, 1, 9, 1000, 9000);      
  Serial.print("LED delay time = " );                      
  Serial.print(incomingByte);
  Serial.println();  
  digitalWrite(ledPin, HIGH);
  delay(outputValue);
  digitalWrite(ledPin, LOW);
  delay(outputValue);                    
}

I think you forgot to set pin mode to output.

P.S. I think this saves you 2 lines.

const int ledPin = 13;
int incomingByte;
void setup() {
  Serial.begin(9600);
  pinMode(ledPin, OUTPUT);
}

void loop() {
if (Serial.available() > 0) {
    incomingByte = Serial.read();
  }
  Serial.print("LED delay time = " );
  Serial.print(incomingByte);
  Serial.println();
  digitalWrite(ledPin, HIGH);
  delay(incomingByte x 1000);
  digitalWrite(ledPin, LOW);
  delay(incomingByte x 1000);
}

just to be sure, test if the input byte is in the expected range.
and be sure to give the input byte an initial value.

now, the variable is in an unknown state, and if there is no
Serial.available() > 0 situation, you start working with
that unknown value in the rest of your program.

initializing can be as simple as
"int xyz = 10; " instead of "int xyz;"
and saves a lot of debugging time in general :smiley: