Please help

For makers who have purchased an Adafruit Starter Pack, get help with the tutorials here!

Moderators: adafruit_support_bill, adafruit

Please be positive and constructive with your questions and comments.
Spinsycle
 
Posts: 10
Joined: Sun Jul 31, 2011 4:21 pm

Re: Please help

Post by Spinsycle »

Getting code to compile and getting the logic correct are not necessarily the same. You needed an extra curly brace because they need to be balanced overall. But the correct place to insert it was probably back where you inserted the extra if/else clause.
thank you i will look into that.

I think my brain is gunna start blinking soon.

User avatar
chuckm
 
Posts: 159
Joined: Thu Dec 24, 2009 3:31 am

Re: Please help

Post by chuckm »

Simplifying the code will help you quite a bit I'm sure, for example the code you wrote for the button press is below (I've re-formatted it to use a 'regular' indent pattern.

Code: Select all

    val2 = digitalRead(switchPin);     // read the input again to check for bounces
    if (val == val2) {                 // make sure we got 2 consistant readings!
        if (val != buttonState) {          // the button state has changed!
            if (val == LOW) {                // check if the button is pressed
                if (lightMode == 0) {          // if its off
                lightMode = 1;               // make it all on!
            } else {
                if (lightMode == 1) {        // if its all on
                    lightMode = 2;             // make it blink!
                } else {
                    if (lightMode == 2) {      // if its blinking
                    lightMode = 3;           // make it wave!
                    } else {
                        if (lightMode == 3) {    //  if its waving,
                            lightMode = 4;         // make it dubble wave!
                        } else {
                            if (lightMode == 4) {  // if its dubble waving
                                lightMode = 5;       // make it reverse wave!
                            } else {
                                if (lightMode ==5) {
                                    lightMode =0;
                                }
                            }
                        }
                    }
                }
            }
            buttonState = val;                 // save the new state in our variable
        }
Something to be aware of is that the last brace here (the } character) is still two braces short of closing off the top level "if" statement. What is worse, I don't believe this code is doing what you think it is doing.

This code is what switch statements were designed for, the form of a switch statement is as follows:

Code: Select all

switch (<expression>) {
case <value-1>:
                <statements>;
                break;
case <value-2>:
                <statements>;
                break;
case <value-n>:
                <statements>;
                break;
default:
              <statements>;
}
In English, the compiler evaluates the expression that is inside the parenthesis and gets back a number. Then it looks for a label of the form "case n:" where n is the same number, so if the expression evaluated to the number 1 it would look for "case 1:" and then it does a 'goto' to that label and starts running the statements it finds. Until it sees the statement 'break' at which point it jumps all the way down to the next statement after the closing brace. Finally, the "special" label is default: this tells the compiler that if you can't find the value in the various case statements, do the one labelled default.

So for your bit of code you probably wanted to write:

Code: Select all

switch (lightMode) {                // 'lightMode' is the expression here
    case 0: lightMode = 1;          // if it was 0 make it 1
               break;
    case 1: lightMode = 2;          // if it was 1 make it 2
               break;
    case 2: lightMode = 3;          // if it was 2 make it 3
               break;
    case 3: lightMode = 4;          // if it was 3 make it 4
               break;
    case 4:  lightMode = 0;          // if it was 4 make it go back to 0
               break;
    default: lightMode = 0;           // if it was anything else, make it 0
               break;
 }
If you used a switch statement (and thus simpler braces and indentation) you would see that your program (as written) has unbalanced braces and, as written, won't do anything unless you hold the button down.

--Chuck

Spinsycle
 
Posts: 10
Joined: Sun Jul 31, 2011 4:21 pm

Re: Please help

Post by Spinsycle »

Thank you i really appreciate the help. because im new i dont know when to use an if statement and when to use array's and switch statement. im gunna try all this out tomorrow.

User avatar
adafruit_support_bill
 
Posts: 88038
Joined: Sat Feb 07, 2009 10:11 am

Re: Please help

Post by adafruit_support_bill »

Actually, the whole 'lightMode' logic could be simplified even further:

Code: Select all

    lightmode = lightMode + 1; // increment the light mode
    if (lightMode > 4)   // if it exceeds the maximum
    {
        lightMode = 0;  // reset to zero
    }

User avatar
chuckm
 
Posts: 159
Joined: Thu Dec 24, 2009 3:31 am

Re: Please help

Post by chuckm »

And even easier (but more complicated to read)

Code: Select all

lightMode = (++lightMode) % 5;
Sadly its terribly easy to obfuscate with C, can be fun if you know it well, sort of like telling bawdy puns in English.

That being said, if it is a variation on a state machine then the simplest implementation in C is the array so

Code: Select all

int next_states[] = {1, 2, 3, 4, 0 };

lightMode = next_state[lightMode];
Which maintains a lot of flexibility since you can go to any state from any state, but has the downside that if lightMode becomes corrupted and falls outside the range of allowed states then you are in trouble. Also if you wanted a sparse set of state values you can waste a bit of space with an array which, by neccesity, has memory between its minimum and maximum index.

Don't worry Spin, none of this is critical for your issues at the moment, but later you may find it useful.

--Chuck

User avatar
adafruit_support_bill
 
Posts: 88038
Joined: Sat Feb 07, 2009 10:11 am

Re: Please help

Post by adafruit_support_bill »

And even easier (but more complicated to read)
Best to stick with the basics at this stage. It may be fewer keystrokes, but if it is complicated to read, it really isn't easier now is it? :D

Locked
Please be positive and constructive with your questions and comments.

Return to “Arduino Starter Pack”