views:

63

answers:

1

Hello,

Consider this code running on my microcontroller unit(MCU):

while(1){

 do_stuff;
 if(packet_from_PC)
  send_data_via_gpio(new_packet); //send via general purpose i/o pins
 else
  send_data_via_gpio(default_packet); 
 do_other_stuff;

}

The MCU is also interfaced to a PC via a UART.Whenever the PC sends data to the MCU, the *new_packet* is sent, otherwise the *default_packet* is sent.Each packet can be 5 or more bytes with a pre defined packet structure.

My question is:

1.Should i receive the entire packet from PC using inside the UART interrut service routine (ISR)? In this case, i have to implement a state machine inside the ISR to assemble the packet (which can be lengthy with if-else or switch-case blocks).

                  OR

2.Have the PC send some sort of a REQUEST command (one byte),detect it in my ISR set a flag, disable UART interrupt alone and form the packet in my while(1) loop by checking for the flag and polling the UART?In this case the UART interrupt would be re-enabled in the while(1) loop after the entire packet is formed.

+1  A: 

Those are not the only two choices, and the second one seems suboptimal.

My first approach would be to a simple circular queue, and push bytes into it from the ISR and read bytes from in your main loop. That way you have a small and simple ISR and you and do the processing in your main loop without disabling interrupts.

The first choice is possible assuming you can code the ISR sensibly. You probably want to have timeouts when dealing with constructing packets; you need to be able to handle that correctly in your ISR. It depends on the line speed, the speed of your MCU and what else you need to do.

Update:

Doing it in the ISR is certainly reasonable. However, using a circular queue is pretty straightforward with a standard implementation in your bag of tricks. Here is a circular queue implementation; readers and writers can operate independently.

#ifndef ARRAY_ELEMENTS
#define ARRAY_ELEMENTS(x) (sizeof(x) / sizeof(x[0]))
#endif

#define QUEUE_DEFINE(name, queue_depth, type) \
        struct queue_type__##name { \
            volatile size_t m_in; \
            volatile size_t m_out; \
            type m_queue[queue_depth]; \
        }

#define QUEUE_DECLARE(name) struct queue_type__##name name

#define QUEUE_SIZE(name) ARRAY_ELEMENTS((name).m_queue)

#define QUEUE_CALC_NEXT(name, i) \
        (((name).i == (QUEUE_SIZE(name) - 1)) ? 0 : ((name).i + 1))

#define QUEUE_INIT(name) (name).m_in = (name).m_out = 0

#define QUEUE_EMPTY(name) ((name).m_in == (name).m_out)

#define QUEUE_FULL(name) (QUEUE_CALC_NEXT(name, m_in) == (name).m_out)

#define QUEUE_NEXT_OUT(name) ((name).m_queue + (name).m_out)
#define QUEUE_NEXT_IN(name) ((name).m_queue + (name).m_in)

#define QUEUE_PUSH(name) ((name).m_in = QUEUE_CALC_NEXT((name), m_in))
#define QUEUE_POP(name) ((name).m_out = QUEUE_CALC_NEXT((name), m_out))

Use it like this:

QUEUE_DEFINE(bytes_received, 64, unsigned char);
QUEUE_DECLARE(bytes_received);

void isr(void)
{
    /* Move the received byte into 'c' */
    /* This code enqueues the byte, or drops it if the queue is full */
    if (!QUEUE_FULL(bytes_received)) {
        *QUEUE_NEXT_IN(bytes_received) = c;
        QUEUE_PUSH(bytes_received);
    }
}

void main(void)
{
    QUEUE_INIT(bytes_received);

    for (;;) {
        other_processing();
        if (!QUEUE_EMPTY(bytes_received)) {
            unsigned char c = *QUEUE_NEXT_OUT(bytes_received);
            QUEUE_POP(bytes_received);
            /* Use c as you see fit ... */
        }
    }
 }
janm
I'm trying out my first choice. The ring buffer seems like lot of work in the program with regard to the head/tail book-keeping ...I'm not doing much computation in the main loop. The MCU clocks at 4MHz and the line baud is 9600bps
itisravi