Hi,

As my first project I wanted to make my 6 leds work like those on KITT car from "Knight Rider"

And I sucsesfuly made all these six leds work like those on KITT(runing forward and backwords) only modifying code from "led_blink.c"

But in my opinion my code is too long, I know it can be modified and made like two times shorter.

// PIN DEFINITIONS:
//
// PC4 -- LED anode

int main() {
// LED as output
DDRC |= (1<<PC0);
DDRC |= (1<<PC1);
DDRC |= (1<<PC2);
DDRC |= (1<<PC3);
DDRC |= (1<<PC4);
DDRC |= (1<<PC5);

// loop keeps looking forever
while(1) {

PORTC |= (1<<PC0);
delay_ms(50);
PORTC &= ~(1<<PC0);

PORTC |= (1<<PC1);
delay_ms(50);
PORTC &= ~(1<<PC1);

PORTC |= (1<<PC2);
delay_ms(50);
PORTC &= ~(1<<PC2);

PORTC |= (1<<PC3);
delay_ms(50);
PORTC &= ~(1<<PC3);

PORTC |= (1<<PC4);
delay_ms(50);
PORTC &= ~(1<<PC4);

PORTC |= (1<<PC5);
delay_ms(50);
PORTC &= ~(1<<PC5);

PORTC |= (1<<PC5);
delay_ms(50);
PORTC &= ~(1<<PC5);

PORTC |= (1<<PC4);
delay_ms(50);
PORTC &= ~(1<<PC4);

PORTC |= (1<<PC3);
delay_ms(50);
PORTC &= ~(1<<PC3);

PORTC |= (1<<PC2);
delay_ms(50);
PORTC &= ~(1<<PC2);

PORTC |= (1<<PC1);
delay_ms(50);
PORTC &= ~(1<<PC1);

PORTC |= (1<<PC0);
delay_ms(50);
PORTC &= ~(1<<PC0);   
 }
 return 0;
}

I figured, that all six DDRC calls can be replaced with DDRC |= 63 0r DDRC |=(0111111) am I wright... and might be there is some other better way to replace it??

and what could be the possible way of optimize LED animation. I`m pretty sure it can be done in some bitwise arithmetical way... or this is as simple(short) as it gets... ??

Thanks in advance

Your right with the first call, It can all be done in 1 go, with |=63 or you can stick with it like you are for readability ?

The trick here is to bitshift itself.

PORTC |= (1<<PC0);
delay_ms(50);
PORTC = PORTC<<1;
delay_ms(50);

That Turns the first one on, Delays then turns 0000001 to 0000010 delays again.

Loop this forwards and backwards, and you have significantly less lines (4 for the forwards, 4 the for backwards.)

I can give working code if that helps ? you will probably understand whats happening better if you write the code yourself though - Hope it works as expected

or this is as simple(short) as it gets... ??

Here's some short for you (and totally unmaintainable, unexpandable, and incomprehensible):

for (i = -11; i <= 11; i += 2)
{
    PORTC = 5868 / (223 - i * i) - 25;
    delay_ms(50);
}

@bretm: just to satisfy my programmer curiosity: could you explain how you got to this sequence of instructions...

OK, but apologies to archee for derailing the thread a little bit.

The KITT animation seems to come up as a topic, and it always bugged me for some reason that there had to be two separate loops, one for forward scan and one for backward scan. I wanted just one loop, which implies a function that looks like this:

X:  0  1  2  3   4   5   6   7  8  9  10  11
Y:  1  2  4  8  16  32  32  16  8  4   2   1

where X is the loop index and Y is the series of values (1<<PC0) through (1<<PC5) and back again. You could just put these values into an array, but that didn't seem as fun. ^_^

I knew (from mathematics background) that the function would probably come out simpler if the X values were centered around zero since the Y values are symmetrical, so I shifted the X values by -5.5:

X:  -5.5 -4.5 -3.5 -2.5 -1.5 -0.5  0.5  1.5  2.5  3.5  4.5  5.5
Y:    1    2    4    8   16    32   32   16   8    4    2    1

I wanted to stick to integer math if possible, so I doubled all of the X values:

X:   -11  -9  -7  -5  -3  -1   1   3  5  7  9  11
Y:     1   2   4   8  16  32  32  16  8  4  2   1

If you graph these points, it looks like a steep vertical peak. I guessed (math background again) that I might get a better fit if the curve were smoother, and I also knew that the KITT animation looks the same backwards as it does forwards, so I reversed the Y values so that it starts from PC5 and goes to PC0 instead of the other way around:

X:   -11  -9  -7  -5  -3  -1   1  3  5  7   9  11
Y:    32  16   8   4   2   1   1  2  4  8  16  32

This results in a nice, smooth bowl shape. Next, I knew that rational functions are a powerful way to approximate data using a small number of coefficients, so I plugged these pairs of values into a rational function solver at ZunZun.com.

I chose Rational Functions as the family of functions to search. I knew that I didn't have to get an exact fit; it just had to be close enough that the division would be within 0.5 of the right value, because the integer division would truncate the result. The solver came up with a bunch of candidates. I picked a simple one, y = (a ) / (1.0 + b + c( x^2 ) ) + Offset:

a =  1.1118061733201769E+03
b =  4.1245899963866670E+01
c = -1.8947538330490038E-01
Offset = -2.5519040468275300E+01

I divided by -c in the numerator and denominator to remove one of the coefficients. I rounded the coefficients and got lucky that the results stayed within range after truncation.

The for loop generates the X values and the function generates the Y values.

You can use the same function for 6 or fewer LEDs, and the same technique to add a 7th, or even 8th, but it gets a bit ridiculous. You can also use the same technique to come up with a function that doesn't repeat the "end" values, e.g. that goes 1, 2, 4, 8, 16, 32, 16, 8, 4, 2 with only one 32 and one 1. In that case you don't have to double the X values to center them on zero as integers.

for (i = -4; i <= 5; i++)
{
    PORTC = 28740 / (880 - 17 * i * i) - 31;
    delay_ms(50);
}

@bretm: Thanks for the wonderfully detailed explanation. Math is beautiful!

@exussum: thanks for answer I made code as you adviced and it was much smaller than my version. I uploaded it and it worked perfectly(at first it crashed my PC, but I think that my WIN7 just played jokes with me and comited a suicide)

Sample from code

PORTC = 1;
delay_ms(50);

PORTC = 2;
delay_ms(50);

PORTC = 4;
delay_ms(50);

PORTC = 8;
delay_ms(50);

@bretm: thanks your short code version realy surprised me. I knew it could be shorter, but that it could be this short.. I tried your code. I uploaded it to MCU and it worked perfectly...(and the main thing, because of your long explanation I perfectly understand how it works, and feel like fool for not figuring out that by myself)

Just for my curiosity:

1) DDRC = 63; <-- in this line we decler that PC0 to PC5 will work as outputs.. ?

2) if DDRC all registers values are all left at 0 all PC ports will work as inputs.. ?

3) if there is values like 3.1 or 3.5 or 3.9 in integer value they all will be trunced down to 3, 3 and 3... ?

OK this far it would be done, now I have to program MCU to make "Knight Rider" intro music, in midi format.. I suppose it will be hard for noob like me, but I like the challange...

Cheers,

Arturs

To make the code you have smaller, You can use code that actually looks like mine

for(i=1;i<7;i++){ PORTC = PORTC<<1; delay_ms(50); }

That will save you repeating yourself.

And you can do a reverse loop to get back down to 1 from 6.

When your happy with that code, Look on to using Bretm's code.

to 2)

Yes - This will tell you if you have a Binary 1 (5V) or binary 0(0V) on that pin.

3) Look up type casting - for ints yes, It takes the whole number part.

1.9 = 1 1.1 =1

Forgot to indent code again ...

If you would prefer instead of bitshifting the number,

PORTC *= 2;

That times it by 2 each time its run, Which is what the bitshift is doing

Just to be clear, I recommend that you don't use my version. You'll come back to it later, or someone else will see your code, and have no idea how it works.

Yes, DDRC = 63 will configure PC0 through PC5 as outputs. It will also configure PC6 and PC7 as inputs. If you do DDRC |= 63 instead, it will leave PC6 and PC7 configuration unchanged. (Although the Atmega168 doesn't have a PC7 pin.)

Yes, if DDRC is left at 0 all PC pins will be inputs.

And yes, if you assign a floating-point value to a register or other integer variable, it will be truncated to an integer. The code I posted uses integer division, so it does not result in fractional values. But even integer division is really slow (although it doesn't matter since it's immediately followed by a much longer delay).

for(i=1;i<7;i++){ PORTC = PORTC<<1; delay_ms(50); }

If PORTC starts at (1<<PC0), this will shift it six times and end up at (1<<PC6). The code would have to look more like

PORTC = 1;

for (i = 0; i < 5; i++)
{
    delay_ms(50);
    PORTC <<= 1;
}

delay_ms(50); // pause at PC5

for (i = 0; i < 5; i++)
{
    delay_ms(50);
    PORTC >>= 1;
}

delay_ms(50); // pause at PC0

because the original code had it sitting at PC0 and PC5 for two times 50ms.

I find the following approach to be the most self-explanatory:

uint8_t pattern[12] PROGMEM = 
    {PC0, PC1, PC2, PC3, PC4, PC5, 
     PC5, PC4, PC3, PC2, PC1, PC0};

for (i = 0; i < 12; i++)
{
    PORTC = 1 << pgm_read_byte(&pattern[i]);
    delay_ms(50);
}

This approach also lets you easily define more complex patterns.

@exussum:

for(i=1;i<7;i++){ PORTC = PORTC<<1; delay_ms(50); }

I dont uderstand how this will be working... What I see is that PORTC will be set to 0000001 for 7 times..?

in my mind it looks like --> PORTC = 1; and this line done 7 times.

Your offered final code should look like this:

while(1)
{
PORTC = 1;
delay_ms(50);
int i;
for(i=1;i<4;i++)
  { 
    PORTC *= 2;
    delay_ms(50); 
  }
  //reverse countdown
  for(i=1;i<5;i++)
  { 
    PORTC /= 2;
    delay_ms(50); 
  }
}

@bretm: Im not woried about forgetting what each line means in your code, Im woried that it will decrese battery`s life due to many calculations it have to do all the time

Thanks for answering to my 3 questions

@ archee "I dont uderstand how this will be working... What I see is that PORTC will be set to 0000001 for 7 times..?"

If PORTC=1 (00000001) then PORTC bitshifted one value left is 00000010

I didnt know you could do <<= Learn something new all the time.

Bitshift 00000010 one to the left is 00000100.

The way in nerdkits code is usually 1<<value

That means take binary 1 and bitshift it (value) positions, so PC4 if you echo it locally will give you that value which you can confirm by doing a manual bitshift.

Hope that made seance

Ouuu... thats what it is

for (i = 0; i < 5; i++)
{
    delay_ms(50);
    PORTC <<= 1;
}

loop cycle goes throu and shifts bits from 0000001 to:

i=0; 0000010

i=1; 0000100

i=2; 0001000

i=3; 0010000

i=4; 0100000

'PORTC<<=1' this is a bitshift.? No realy Im so green at this. Im learning something new from every replay you give to me. I didn`t knew that its possible to do something like bitshift

still have to study this part of bretm`s code

pgm_read_byte(&pattern[i]);

but I dont want to bother you. You have helped me a lot, I ll try some usual methods like google.

Im woried that it will decrese battery`s life due to many calculations it have to do all the time

The calculations in my funny code won't significantly decrease battery life. They take microseconds. The power will be used mostly in lighting the LED and running delay_ms(50). Whereas the calculations can take a hundred or so clock cycles, one delay_ms(50) call will burn hundreds of thousands of clock cycles.

If you really want to extend your battery life, I would recommend doing these things:

1. Use a timer interrupt to update the LED instead of the main() loop.
2. In the main loop, set a sleep mode and keep going to sleep in order to minimize power consumption (see avr/sleep.h). The timer will keep waking you up. Just keep going back to sleep again.
3. Don't leave the LED on for the full 50ms. Make it twice as bright but only keep it on for 5 or 10 milliseconds. The human eye's persistance of vision will make it look like it stays on longer.
4. Turn off all of the subsystems you don't need, like ADC, SPI, etc.

Just off the top of my head (not compiled or tested):

uint8_t pattern[12] PROGMEM =
  {PC0, PC1, PC2, PC3, PC4, PC5,
   PC5, PC4, PC3, PC2, PC1, PC0};

volatile uint8_t ledState;
volatile uint8_t patternIndex;

// every 5 milliseconds
ISR(TIMER0_COMPA_VECT)
{
    ledState++;

    if (ledState == 1)
    {
        // turn on the currently selected LED
        PORTC = 1 << pgm_read_byte(pattern[patternIndex]);
    }
    else if (ledState == 2)
    {
        // turn it off 5ms later
        PORTC = 0;
    }
    else if (ledState == 10)
    {
        // after 50ms switch to the next LED
        ledState = 0;
        patternIndex++;

        if (patternIndex == 12)
        {
            patternIndex = 0;
        }
    }
}

void main()
{
    // set Power Reduction Register     
    PRR = (1<<PRTWI)     // turn off TWI
        | (1<<PRTIM1)    // turn off Timer/Counter1
        | (1<<PRTIM2)    // turn off Timer/Counter2
        | (1<<PRSPI)     // turn off SPI
        | (1<<PRUSART0)  // turn off USART (will turn on again when reset)
        | (1<<PRADC);    // turn off ADC

    DDRC = 63;

    // ... initialize the 5ms timer interrupt here

    ledState = 0;
    patternIndex = 0;

    set_sleep_mode(SLEEP_MODE_IDLE);
    sei();

    while(1)
    {
        sleep_mode(); // go low-power
    }
}

  int lightArray = 0, inc = 1;
        for(; inc = (!lightArray) ? 1 : (lightArray == 6) ? -1 : inc; lightArray += inc) {
                showLights(1 << lightArray);
                sleep(100);
        }

That should work OK too

I just got my nerd kit a couple of days ago and have only gone through the temp sensor and LED blink projects and while I don't have a complete understanding of what I've done I was able to come up with this solution using only 3 LEDs to test. I'll add that I have never programmed in C but am quite familiar with web based languages like Javascript, PHP, and Actionscript.

// led_blink.c
// for NerdKits with ATmega168
// hevans@nerdkits.edu

#define F_CPU 14745600

#include <avr/io.h>
#include <inttypes.h>

#include "../libnerdkits/delay.h"
#include "../libnerdkits/lcd.h"

// PIN DEFINITIONS:
//
// PC4 and PC5 -- LED anode

int main() {
  // LED as output
  int count = 0;
  int fore = 1;
  int maxLEDs = 2;
  DDRC = 63;
  PORTC |= (1<<PC3);

  while(1) {
    delay_ms(500);

    if(count < maxLEDs && fore == 1){
        count++;
        PORTC = PORTC<<1;
    } else {
        count--;
        fore = 0;
        PORTC = PORTC>>1;
        if(count == 0){
            fore = 1;
        }
    }
  } 
  return 0;
}

I have a lot of questions about some of the parameters but for fear of hijacking this thread, I'll make a new post.

-Makoto

DDRC = 63;

You dont want that many outputs, And really it should be

DDRC |= 7

7 in binary is 00000111

so only the ones you want are outputs.

But the rest seems fine :)

there is so many solutions for this one little animation, but how to know which solution is the best for the job.. ?

My personal favorite is the one where all otput port adresses are stored in array... In my head it should be the best solution.

@:Makoto DDRC=63; was for my six leds 63 -> 0111111

Ok, I am starting to get a clearer picture now, this is good. Thank you exussum and archee for the explanations.

Its funny, my initial intention for the LED array was to generate a Knight Rider light setup with several different patterns that it could cycle thru, I guess most nerds think alike. :)

I will be trying to create a 3x40 grid for my project and incorporate the brightness action applied in the valentines demo.

-Makoto

the "best" solution taking everything in to consideration would be a timer based interrupt firing every 50 ms doing either the array style one or a bitshift. I highly doubt theres much performance difference between the two