SparkFun Forums 

Where electronics enthusiasts find answers.

Your source for all things Atmel.
By TabbyArtifact
#111438
Hi everybody. I am making a 64 led pov globe because i think they are one of the coolest things ever. :) I am using a Atmega328p as the brains, and 8 8-bit shift registers to run the leds. Now I am not a very big programmer so my code is very basic. I was wondering if there was a more efficient way than I have to write the code, to flash the leds. My code is
Code: Select all
#define F_CPU 16000000UL

#include <avr/io.h>
#include <util/delay.h>

int cycle(int data);
int latch(void);

int main(void)
{
    

    DDRB = 0b000111;
    PORTB = 0b000000;

    while(1)
    {
        cycle(1);
        cycle(0);
        cycle(1);
        cycle(0);
        cycle(1);
        cycle(0);
        cycle(1);
    cycle(0);
        cycle(1);
        cycle(0);
        cycle(1);
        cycle(0);
        cycle(1);
        cycle(0);
        cycle(1);
    cycle(0);
        cycle(1);
        cycle(0);
        cycle(1);
        cycle(0);
        cycle(1);
        cycle(0);
        cycle(1);
    cycle(0);
        cycle(1);
        cycle(0);
        cycle(1);
        cycle(0);
        cycle(1);
        cycle(0);
        cycle(1);
    cycle(0);
        cycle(1);
        cycle(0);
        cycle(1);
        cycle(0);
        cycle(1);
        cycle(0);
        cycle(1);
    cycle(0);
        cycle(1);
        cycle(0);
        cycle(1);
        cycle(0);
        cycle(1);
        cycle(0);
        cycle(1);
    cycle(0);
        cycle(1);
        cycle(0);
        cycle(1);
        cycle(0);
        cycle(1);
        cycle(0);
        cycle(1);
    cycle(0);
        cycle(1);
        cycle(0);
        cycle(1);
        cycle(0);
        cycle(1);
        cycle(0);
        cycle(1);
    cycle(0);
        
     
    latch();
    

    _delay_ms(2.5);

        cycle(1);
        cycle(0);
        cycle(1);
        cycle(0);
        cycle(1);
        cycle(0);
        cycle(1);
    cycle(0);
        cycle(1);
        cycle(0);
        cycle(1);
        cycle(0);
        cycle(1);
        cycle(0);
        cycle(1);
    cycle(0);
        cycle(1);
        cycle(0);
        cycle(1);
        cycle(0);
        cycle(1);
        cycle(0);
        cycle(1);
    cycle(0);
        cycle(1);
        cycle(0);
        cycle(1);
        cycle(0);
        cycle(1);
        cycle(0);
        cycle(1);
    cycle(0);
        cycle(1);
        cycle(0);
        cycle(1);
        cycle(0);
        cycle(1);
        cycle(0);
        cycle(1);
    cycle(0);
        cycle(1);
        cycle(0);
        cycle(1);
        cycle(0);
        cycle(1);
        cycle(0);
        cycle(1);
    cycle(0);
        cycle(1);
        cycle(0);
        cycle(1);
        cycle(0);
        cycle(1);
        cycle(0);
        cycle(1);
    cycle(0);
        cycle(1);
        cycle(0);
        cycle(1);
        cycle(0);
        cycle(1);
        cycle(0);
        cycle(1);
    cycle(0);
        
     
    latch();

    _delay_ms(2.5);

        }
return 1;
}
        

int latch(void)
{
    PORTB |= (1 << PIN2);
    PORTB &= ~(1 << PIN2);

    return 1;
    }
int cycle(int data)
{


    if (data == 0)
    {
        PORTB &= ~(1 << PIN0);
        PORTB |= (1 << PIN1);
        PORTB &= ~(1 << PIN1);


    }

    else
    {
         PORTB |= (1 << PIN0);
         PORTB |= (1 << PIN1);
         PORTB &= ~(1 << PIN1);
    }
    return 1;
}
        
As you can tell it is very long and takes up some memory. And it is just a piece of test code to see if all the leds work.
User avatar
By thebecwar
#111478
I think now is the time to introduce you to someone you may not know... The for loop

Your example could be reduced to:
Code: Select all
#define F_CPU 16000000
#include <avr/io.h>
#include <util/delay.h>

void cycle(void); //the return type for these should be void, since you ignore the return values...
void latch(void);

int main(void) {
    DDRB = 0b000111;
    PORTB = 0b000000;

    while (1) {
        for (x=0;x<63;x++) { cycle(); } //I'll explain this below
        latch();
        _delay_ms(3); //removed the float... Floats are ugly on 8 bit micros... they hate them.
        //Removed the second iteration of this, because the while(1) already repeats the code
    }
return 1;
}
void latch(void) {
    PORTB |= (1 << PIN2);
    PORTB &= ~(1 << PIN2);
}
void cycle(void) { 
    //I'm guessing that pin1 is your clock... I left it as-is
    PORTB ^= (1 << PIN0); //I'll explain this below
    PORTB |= (1 << PIN1);
    PORTB &= ~(1 << PIN1);
}
Cut from about 150 lines to about 20. Now, for the lines I said I'd explain:

1) Your best friend and mine, the for loop. If you have some task you need repeated a finite number of times, it's one of the easiest ways to do it.
for (x=0;x<=63;x++) { cycle(); }

The x=0; sets up your counting variable. If you hate counting from 0 you can set this to be x=1; (I've been programming long enough I have to remind myself to start from 1 when I'm counting things in real life. :wink:) The x<63; just tells us the exit condition for your loop. In this case it's 63, meaning that the loop will exit when x is no longer less than 63. x++ just says, "every time you loop this code, I want you to increment x by 1."

2) void latch(void); Why'd I change the return type? Because you ignored the results so there's no reason to allocate some SRAM just to dump it. Incidentally this is why I always set my compiler to warn me about every insignificant thing. Makes the code cleaner and more efficient.

3) The last function, cycle();, I changed to have a return type of void (same reason as #2) and I also changed your function's input to void. The XOR (^) operator allows you to do some cool tricks with binary fields. The truth table for XOR is:
0 ^ 0 = 0
0 ^ 1 = 1
1 ^ 0 = 1
1 ^ 1 = 0
When you take the register and XOR it with a mask (your 1<<PIN0) it changes the value of only the places in the mask where there's a 1. An example:

Lets say you have a register with some random data in it:
0b10010100
Say now that you only want to change the 5th bit. The XOR looks like this:
0b10010100 (register data)
0b00010000 (mask)
^^^^^^^^
0b10000100 (new register value)

That's a lot of information, I know but I think that learning is important. Forgive me if I broke things down into too simple for you. My intent is not to make you feel bad. If there's something you need clarification on please let me know.

(And the code was not that bad in the grand scheme of things. Odds are your bank has worse code running either the ATMs or their website.)
By TabbyArtifact
#111543
Oh man you are great. :) Please do not think you are being to simple for me, I just started teaching myself c off the internet and any help is great. I really do like that for loop you have introduced me to. :lol: Thanks for telling me I dont always have to return something at the end of each function, I did not know that. :)
By TabbyArtifact
#111590
Thanks. But as it turns out 8 shift registers with 64 resistors (even surface mount) is heavy and takes up a lot of space, so sadly I dont think I will be able to go that route with build. I was however going to make it so I can control the leds with just the avr chip. I have a basic code that I am hoping will work for this. I shall post it below.

basically the leds will be hooked up like this. a ( l ) is an led, and a ( - or a | ) is a wire.

||||||||
l-l-l-l-l-l-l-l-
||||||||
l-l-l-l-l-l-l-l-
||||||||
l-l-l-l-l-l-l-l-
||||||||


The code i have so far is pretty basic. Pins 3 and 4 are hooked up to 3 leds positive sides each, and pins 0, 1, and 2 are hooked up to an leds negative side ( one led connected to pin 3 and one led connected to pin 4) for a total of 6 leds.
Code: Select all
#define F_CPU 1000000UL

#include <avr/io.h>
#include <util/delay.h>

// positive and negative stand for the pins that will be hooked
// up to the anode and cathode of the led
int cycle(int positive, int negative);

int main(void)
{
    DDRB = 0b0011111;
    // I had a problem with all 3 leds Turing on when the program first
    // started, and i discovered that pin 1 and 2 needed to be turned high
    // in order to fix it.
    PORTB = 0b0000110;

    while(1)
    { 
     cycle (3,0);
     cycle (3,1);
     cycle (3,2);
     cycle (4,2);
     cycle (4,1);
     cycle (4,0);
    }
return 1;
}
int cycle(int positive, int negative)
{
     PORTB |= (1 << positive);
     PORTB &= ~(1 << negative);

     _delay_ms(1000);
    
     PORTB &= ~(1 << positive);
     PORTB |= (1 << negative);
     }
User avatar
By thebecwar
#111609
int cycle(int positive, int negative);

Should Be:

void cycle(int positive, int negative);

Since you're working with left shifts you can probably change both of those input variables to char. (char would allow you to left shift a 1 WAY beyond the range of any of the capabilities of the AVR.)

Function definition syntax is as follows:

return_type function_name([argument_type argument_name, ...]);

The return type and argument types can be anything the compiler knows about, either one of the built in types (char, int) or any type defined by a typedef statement (uint8_t, uint16_t, etc...). The function name can be almost anything. There are some rules, but the compiler will fail if you don't follow them. Wikipedia has a lot of information at: http://en.wikipedia.org/wiki/C_variable ... clarations

One other thing, if you're using AVR-GCC to compile your code, I'd recommend you add the compiler option -Wall. This will warn you about a lot of iffy coding practices. It will still compile the code, but it will warn you if you have, for example, an int function that you ignore the results from.
By TabbyArtifact
#111629
Ok it makes sense that the function should be void as it is not returning any values. And you would change the int to char to save on memory. Once again thanks for all the help. :)
By TabbyArtifact
#111956
Ok it seems that for the life of me I cant get a single led to even turn on and I am getting super angry at myself. I cant discover what the heck I am doing wrong. I have 64 leds wired up into 8 banks of 8 leds each. Every bank of leds has all the negatives wired up to the 8 pins of PORTB. All of the positive sides of the leds are wired up to PORTD, so every first led in every bank is wired up to PIND0, and so forth. Now I just want to see an led turn on! so I wrote this code.
Code: Select all
#include <avr/io.h>

int main (void)
{
	DDRB = 0b11111111;
	DDRD = 0b11111111;

	while (1)
	{
		PORTB = 0b00000000;
		PORTD = 0b10101010;

		}

		return 1;
		}
Now with this I should see every other led in every bank of leds light up right? I mean I know the leds are hooked up right. I can turn them on and off with an external battery plugged into the proper leads of the microchip socket without the microchip in place, so I know that everything is soldered correctly. What am I doing wrong?
User avatar
By thebecwar
#112011
Are you planning on charlieplexing to get your 64 outputs? If not, just tie one side of each LED to ground, and raise the pin high.

Keep in mind too, that each pin should only be driving about 40ma (source or sink, doesn't matter) If you have more than 2 leds hooked up to a pin you may be triggering that pin's self protection fearures. Also keep in mind that resistance divides in parallel, so if you've got multiple resistors hooked up to the pin the current goes up, not down. 8 leds, each attached to it's own 330ohm resistor and placed in parallel gives you only 41.25 ohms. 41.25 ohms at 5v is 121mA. Each LED is safe, since the current divides evenly to each (15.125mA). You'll want to drive the load with a transistor straight off your 5v rail. (And another transistor between the led and gnd). Kind of like the setup you'd use for a transistor based and gate.
By tecoist
#112013
You may want to consider these limitations from the 328 datasheet:

* no pin is guaranteed to sink or source more than 20mA at 5V (10mA at 3.3V).
* pins are divided into 3 groups, and in each group, the source current (Ioh) total should not exceed 150mA
* the total Vcc or ground current must never exceed 200mA.
By TabbyArtifact
#112227
I now know that what I am trying to do is totally possible its just appears that my chip is being weird. Ok I had some time to take some measurements and it seems that I am not getting any voltage on any of the I/O pins. I can confirm that 5 volts is getting to the chip, because I can measure 5 volts on those pins. I tried this with 2 different Atmega 328p chips, One on the stock settings and one running at 16mhz. The code I used to test this was
Code: Select all
#define F_CPU 1000000UL

#include <avr/io.h>

int main (void)
{
   DDRB = 0b11111111;
   DDRD = 0b11111111;
   DDRC = 0b11111111;

   while (1)
   {
   PORTB = 0b11111111;
   PORTD = 0b11111111;
   PORTC = 0b11111111;
   }
return 1;
} 
I just dont understand how to get voltage to come out of the I/O pins. Do I have to hook up the AREF pin to something?
User avatar
By thebecwar
#112228
1 -> Are you measuring the voltage between the pin and gnd?
2 -> Try this:
Code: Select all
void main(void) {
  DDRB = 0xFF; //Same as 0b11111111
  DDRC = 0xFF;
  DDRD = 0xFF;
  PORTB = 0xFF;
  PORTC = 0xFF;
  PORTD = 0xFF;

  while(1);

}
I think that by setting and re-setting those registers repeatedly you may be preventing the output from happening. It's just a guess, but should be worth a try.

3 -> AREF is the reference voltage for the ADC. You shouldn't need anything there, but I'd advise against leaving that pin float. If (2) doesn't work try wiring AREF to 5v.
By TabbyArtifact
#112230
Ok I tried your code and nothing has changed on either chip. I am measuring the voltage with one pin to ground and I am touching the other lead to the various pins of the chip. 5 volts is getting to the vcc pins I am 100 percent certain of that. I even tried tying AREF to 5v and still no luck.
By n1ist
#112250
That should work unless either your reset pin is tied low, or your clock settings are wrong. Can you post your schematic and fuses?

AREF should not be tied either to +5 or GND unless your program turns off the internal reference first. It's OK to leave it floating if you aren't using the ADC, but it is better to connect it through a 100nf cap to ground.
AVCC, on the other hand must be tied to VCC; without that, port C may not work.

/mike
By metalmagic
#145116
I am interested in building a pov globe as well. I was going to use the 595 shift registers to run the leds, but I want to make sure that I understand that the 596 registers are the same thing and I can elliminate the current limiting resistors on the leds? Also, having some trouble learning the code for operating shift registers. Does anyone know of a good place to look on the internet for this? I know the basics of coding, just not familiar with programming for shift registers.

Thanks