Error in sketch code

Hi JohnC.  Good attempt.  In respect to the current limits of each I/O pin,  I would suggest the need to multiplex the LEDs (only one on at a time.)  As a more correct example:

void setup(){

ledMode(led1, OUTPUT);

ledMode(led2, INPUT);   // look up about writing a 1 to the input pin to put it in high impedance mode.

ledMode(led3, INPUT);

ledMode(led4, INPUT);   // it was stated that pinMode is preferred;  I don't know

ledMode(led5, INPUT);   // and don't want to look it up right now.  sorry. 

ledMode(led6, INPUT);

ledMode(led7, OUTPUT);   //  this is the source of one of your errors  led7,

}

void loop(){

seconds++;

while(1){

   if (seconds == 1){

      digitalWrite(led1, HIGH);

      digitalWrite(led7, LOW);

      delay(10);

      digitalWrite(led7, HIGH);    //  in theory this will probably work

                                                 // use at your own risk since it is being adapted from PIC techniques

      delay(10);

      }  // fi     * really?  I am the first to note/share this technique

         //                  of marking what this closing brace matches  !?!

...

if (timeToUpdateDisplay) break;

}  // elihw     * ditto

}  // pool       * ICNBI

Thanks billabott,

When it comes to the actual BCD clock code it will have to have more than 1 led on at a time so I'm not sure how that would work with multiplexing. 

You say that ledMode(led7, OUTPUT); is the problem but I don't see what the problem is?  It is writen the same as all of the other ledMode statements.

Should I be using pinMode() rather than ledMode()?

I was also wondering if it would be better to use the switch statement rather than multiple if statements?

John,

  Yes, it's almost always better to use a switch statement than a long

list of if statements that test consecutive values like you have.

Ok,  that's great!  I'll change them to the switch statement and see how that works.

In my code, should I have used the pinMode() statement rather than ledMode() statement?

Thanks for your help and patience.

John,

  Yes, I believe pinMode() is what you want.

(also be sure to remember the break statement in each branch

of the switch statement).

I would also note that sometimes there is a better way than

a switch statement.  If all the branches of a switch statement

are similar, or if they are all computing the value of the same

variable, it may be better to use a table lookup rather than a

switch statement.  This doesn't seem to apply in your example,

but it may be helpful in the future.

John,

  Yes, I believe pinMode() is what you want.

(also be sure to remember the break statement in each branch

of the switch statement).

I would also note that sometimes there is a better way than

a switch statement.  If all the branches of a switch statement

are similar, or if they are all computing the value of the same

variable, it may be better to use a table lookup rather than a

switch statement.  This doesn't seem to apply in your example,

but it may be helpful in the future.

Thanks Coder27,

I have fixed all errors, converted to a switch statement and added more code so it will run from 0 seconds up to 1 minute. It compiles with no problems now.  All LEDs are working in the proper sequence and turns on the 1 minute LED when it hits 60 seconds.  Then the seconds counter drops back to zero and it starts over counting up to the next minute.

This part of the project was done using the UNO because I was just trying to get some idea of coding in 'C'.

Now that I have fugured out how to get that part of the code working I will start using my ArduinoMega2560 because it has more pins and more memory as well.

Writing the code to handle the seconds, minutes and hours all the way up to 24 hours would make that switch statement quite long so I would be very interested in learning more about this table lookup that you mentioned. Where would I find a tutorial on how to code such a lookup table?

Thanks.  You have been very helpful.

John,

  A table is just a fancy name for array, although in

some languages it might be a fancy type of array such

as an associative array.

  If you have 10 digits, each involving 4 led settings,

you might have a struct that contains 4 boolean values for

those leds, and then an array of 10 of these structs.

To find the led settings for digit n, you would use n to index

the array to lookup the struct with the 4 led settings for n.

This would take the place of the switch statement.

Here are two tutorials:

http://embeddedgurus.com/stack-overflow/2010/01/a-tutorial-on-lookup-tables-in-c/

http://stackoverflow.com/questions/2641473/initialize-static-array-of-structs-in-c

The reason a table lookup is preferable to a switch statement

is that it makes your code simpler, so there are fewer things

that can go wrong.  For example, you can't forget a break statement.

It also makes it easier to check that all the values in the table are correct

since they are in one place rather than scattered in the switch statement.

Thanks Coder27,

When you mentioned a 'lookup table' the first thing that came to my mind was a 'lookup table' such as in dBase which is held in a database but of course that wouldn't be possible in Arduino so it got me wondering what it was.

I will take a look at those tutorials that you rcommended and see what I can come up with.  Keeping the code as short as possible is always a good thing. 

Thanks again, and have a very Merry Christmas and a Great New Year.

Hi Coder27,

I am reading up a bit on those arrays and I have a question for you if you don't mind?

Is it possible to use an array element in digitalWrite() like in the following example?

digitalWrite(myArray[2], HIGH);  or   digitalWrite(myArray[2], myArray[3]);

Or,... if a multi-dimensional array was used such as myArray[8][8] with, say, myArray[2] holding the pin number and the second dimension of the array [3] holding the HIGH or LOW, how would the syntax for that work?  I would think that the syntax would be incorrect to write it as

digitalWrite(myArray[2], [3]);

Don't know if my question is clear or not but,.... you might get the picture of what I am trying to do here.  I am looking at having separate arrays for seconds, minutes and hours.

Thanks,

John

John,

  Yes, you can use an array element in digitalWrite, either as the first

or second parameter (or both), essentially anywhere you might otherwise

use a simple number or variable or algebraic expression.

  As you seem to understand, you can either have more than one

single-dimensional array, sometimes called parallel arrays, or you

can have an array of structs, or an array of arrays.

> digitalWrite(myArray[2], [3]);

The "[3]" isn't right.  Each parameter to digitalWrite is treated separately,

so the "[3]" needs to have the name of the array variable in front of it,

like you have for myArray[2], except it would probably have a different array name,

because the second parameter of digitalWrite is the LOW/HIGH setting,

rather than the pin number.  Read up on arrays so you are clear how it works.

I think I am getting the idea of how to set up an array, or even a multi-dimensional array but I am struggling a bit with how to use it in the BCD clock.  If it was a matter of simply turning on or off one individual LED at a time I don't think I would have a problem but for displaying the seconds portion of the time, I am using 7 LEDs which will sometimes have  only one LED turned on while at other times it will have two and sometimes three LEDs turned on at the same time.

It works fine using the switch statement but I can't figure out how to do the same thing using an array.

The following is a portion of the code that I used in the switch statement showing the first 9 of the 60 seconds needed.

void loop(){

  seconds++;

  switch(seconds){

   case 1:

      digitalWrite(led1, HIGH);  //led1 ON

      delay(1000);

      break;

   case 2:

      digitalWrite(led2, HIGH);  //led2 ON

      digitalWrite(led1, LOW);  

      delay(1000);

      break;

   case 3:

     digitalWrite(led1, HIGH);  //led1 and led2 ON

     delay(1000);

     break;

   case 4:

     digitalWrite(led3, HIGH);  //led3 ON

     digitalWrite(led2, LOW);

     digitalWrite(led1, LOW);

     delay(1000);

     break;

   case 5:

     digitalWrite(led1, HIGH);  //led3 and led1 ON

     delay(1000);

     break;

   case 6:

     digitalWrite(led2, HIGH);  //led3 and led2 ON

     digitalWrite(led1, LOW);

     delay(1000);

     break;

   case 7:

     digitalWrite(led1, HIGH);  //led3, led2 and led1 ON

     delay(1000);

     break;

   case 8:

     digitalWrite(led4, HIGH);  //led4 ON

     digitalWrite(led3, LOW);

     digitalWrite(led2, LOW);

     digitalWrite(led1, LOW);

     delay(1000);

     break;

   case 9:

     digitalWrite(led1, HIGH);  //led4 and led1 ON

      delay(1000);

      break;

)

As you can see, with each increment of the seconds different pins are set HIGH and others are set LOW.  I can't seem to wrap my head around how to come up with a matrix that will do the same thing.  Any thoughts?

Thanks,

John

John

If you set up a single array (0-9) with the decimal number that matches the binary value of the leds you need to turn on. (ie 0 = none, while 127= all 7 leds)

At each second you could read the array element matching the second, and then turn on the leds.

You can use the port manipulation ( http://www.arduino.cc/en/Reference/PortManipulation ) to set multiple pins at once.

Decimal numbers also work, so direct reference to your array should work.

Does this make sense.??

Mark

Hi Mark,

As you can see, coding in 'C' and the Arduino are both new to me and I am in a learning process.  Coder27 suggested using arrays rather than the switch statement and that makes sense since the code for a 24 hour clock could get quite large and more difficult to read with the switch statement.  I looked at a couple of short tutorials on arrays and I think I get the general idea.

Your suggestion certaiinly makes sense and is something that I would never have thought of. I will follow your link and read up on the port manipulation to see if I can get an understanding of how that works as well.

Thank you very much for your help.  It is very much appreciated.

Have a very Merry Christmas and all the best in the 2013.

John

Hi Mark,

I have been looking at some info on the manipulation of the ports on the Arduino.  I don't have it all down pat yet but I am getting a better understanding of how it works. One thing that I am unsure of is whether I can use a variable in the port designation.  Since I will likely be using pins 1 to 13.

Pins 1 to 7 is on PORTD and pins 8 to 13 is on PORTB,  so could I use a variable such as PORTx or PORTy?

For example:

char x='B'

char y='D'

for( int i=1; i< 13; i++ ) {

    if( i < 8) {

       PORTx = pinArray[3];   // PORTD     

     }

}

else {

     PORTy = pinArray[9];    // PORTB

    }

}

I am practicing with the UNO but once I get to the point where I can get things working properly I will start using the Mega because it has more pins and of course more memory.

Thanks

John

Ok, I tried to use a variable for the PORTD and PORTB and that won't work but I figured out another way to achieve what I needed.

I re-wrote the code, eliminating the switch statement, and used  PORTD and PORTB which is controlled by the secsArray[], the minsArray[], and the hrsArray[].  I ran it using the Arduino Simulator and it is working fine so far.  I only have it coded to where it counts the seconds and minutes so I have to work on the hours now. If I can get that working properly in the simulator, I will upload it to the UNO and see how it runs there.  Using the arrays and the PORTS sure shortens up the code a lot.

It has been quite a learning experience but I'm getting a handle on this 'Arduino C' coding now.

Thanks to Coder27, billabott, and Mark Beckett.  You guys are great!

John