-3

I am currently working on a project where we have to use an AVR ATMEGA328 micro-controller, specifically the USART peripheral, to control 8 LED's. We have to send commands to the micro-controller that will turn on, off, and blink the LED's at different rates. I have written a program in C that I think will do the job, but I would like someone to look at it and help me fix any mistakes that I may have. Your help will be greatly appreciated!

*P.S. Each command in the commands array associates with its corresponding LED state in the LED array. The LED's are connected to PORTB of the micro-controller.

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

/* Arrays that contain all input commands and LED states */
const char *commands[] = {"ON0","ON1","ON2","ON3","ON4","ON5","ON6","ON7","ON8","OFF0","OFF1","OFF2","OFF3","OFF4","OFF5","OFF6","OFF7","OFF8","BLINK0","BLINK1","BLINK2","BLINK3","BLINK4","BLINK5","BLINK6","BLINK7","BLINK8","STOP"\0}
int LEDs[28] = {0X01,0X02,0X04,0X08,0X10,0X20,0X40,0X80,0XFF,0XFE,0XFD,0XFB,0XF7,0XEF,0XDF,0XBF,0X7F,0,0X01,0X02,0X04,0X08,0X10,0X20,0X40,0X80,0XFF,0}

int i;
int j;

int Blinky(int j); // Function to execute blinking commands where j is the argument
{
    PORTB = LEDs[j];
    _delay_ms[250 + (j-18) * 50];  /* Calculates blinking delay times */
    PORTB = 0;
    _delay_ms[250 + (j-18) * 50];
}

int main(void)
{
    DDRB=0XFF; // PORTB is set to output
    DDRD=0X02; // Turns on the transmit pin for the USART in PORTD

    /* Setup USART for 9600,N,8,1 */
    USCR0B = 0X18;
    USCR0C = 0X06;
    UBRR0 = 51;

    sei(); // Enable Global Interrupts

    char input;

    if(UCSR0A & 0X80) /* Reads data from the USART and assigns the contents to the character input */
        input = UDR0;

    j=28;

    char cmd;
    cmd = *commands[i];

    for(i=0; i<28; i++)
    {
        if(input==cmd) /* If the contents of UDR0 are equal to one of the commands */
            j = i;
    }

    while(1)
    {
        if(j<18)
            PORTB=LEDs[j]; // Executes the "ON" and "OFF" commands

        else if(j<27)
            Blinky(j);  // Executes the blinking command by calling the Blinky function

        else if(j=27)
            PORTB=0;  // Executes the "STOP" command

        else
            PORTB=0; // Accounts for typing errors
    }
    return(0);
}
Bence Kaulics
  • 7,066
  • 7
  • 33
  • 63
Andrew Adams
  • 1
  • 1
  • 1
    You should test your code before posting. If it's not working, give all the details so we can help you debug it. If it is working, you can post it on https://codereview.stackexchange.com for suggestions on how to improve it. – David Grayson Mar 01 '17 at 02:38

1 Answers1

1

There is a lot wrong with this program, but code review is not the purpose of Stack Overflow. See the FAQ for how to ask an appropriate question.

That said, some of the obvious problems are:

  • The _delay_ms() function needs to be called with a compile-time constant. It won't work correctly if the parameter needs to be calculated at run-time.

  • if you don't read any char from USART, then you still go through the rest of the loop.

  • char cmd declares a character variable, but then you assign a pointer to it.

  • i is used before it is set to a meaningful value.

  • input== cmd will likely never be true as one side is a character and the other is a pointer.

This question will likely close soon. Good luck, and come back if you have a question better suited to Stack Overflow.

UncleO
  • 8,299
  • 21
  • 29