C: Minimizing code duplication using functions in the header file

this is a bit of an odd use case, so finding an existing discussion is difficult. I am programming for embedded systems (Microchip PIC24 using XC16 compiler) and am currently implementing the communication protocol the same on 3 separate UART channels (each UART will grab data from the master data table).

The way I started to write the project was to have each UART handled by a separate module with a lot of code duplication according to the following pseudocode:

UART1.c:

static unsigned char buffer[128];
static unsigned char pointer = 0;
static unsigned char packet_received = 0;

void interrupt UART1Receive (void) {
    buffer[pointer++] = UART1RX_REG;
    if (end of packet condition) packet_received = 1;
}

void processUART1(void) {    // This is called regularly from main loop
    if (packet_received) {
        // Process packet
    }
}

      

UART2.c:

static unsigned char buffer[128];
static unsigned char pointer = 0;
static unsigned char packet_received = 0;

void interrupt UART2Receive (void) {
    buffer[pointer++] = UART2RX_REG;
    if (end of packet condition) packet_received = 1;
}

void processUART2(void) {    // This is called regularly from main loop
    if (packet_received) {
        // Process packet
    }
}

      

While the above works well and works well, in practice the communication protocol itself is quite complex, so duplicating it three times (just with changing UART register references) increases the possibility of errors. Having one function and passing pointers to it is not an option, as it will have a very large impact on speed. The code must be physically duplicated in memory for each UART.

I thought a lot and even though I knew the rules to never put functions in a header file, I decided to try a specific header file including duplicate code, with links as #defined values:

protocol.h:

// UART_RECEIVE_NAME and UART_RX_REG are just macros to be defined 
// in calling file
void interrupt UART_RECEIVE_NAME (void) {
    buffer[pointer++] = UART_RX_REG;
    if (end of packet condition) packet_received = 1;
}

      

UART1.c:

static unsigned char buffer[128];
static unsigned char pointer = 0;
static unsigned char packet_received = 0;

#define UART_RECEIVE_NAME UART1Receive
#define UART_RX_REG       UART1RX_REG

#include "protocol.h"

void processUART1(void) {    // This is called regularly from main loop
    if (packet_received) {
        // Process packet
    }
}

      

UART2.c:

static unsigned char buffer[128];
static unsigned char pointer = 0;
static unsigned char packet_received = 0;

#define UART_RECEIVE_NAME UART2Receive
#define UART_RX_REG       UART2RX_REG

#include "protocol.h"

void processUART2(void) {    // This is called regularly from main loop
    if (packet_received) {
        // Process packet
    }
}

      

I was a little surprised when the code compiled without errors! It seems to work, and after compiling, MPLAB X can even work out all character references so that every macro reference in UART1.c and UART2.c is not identified as an undecidable identifier. Then I realized that I should probably rename the protocol.h file to protocol .c (and update the #includes accordingly), but that doesn't really matter much.

There is only one drawback: the IDE has no idea what to do when executing code included from the .h protocol while simulating or debugging. It just stays on the calling command while the code is running, so debugging will be a little more difficult.

So how hacky is this solution? Will the C gods impress me even for that? Are there any better alternatives I've forgotten?

+3


source to share


2 answers


An alternative is to define a function macro that contains the body of the code. Some token input operators can automatically generate the required symbol names. Multi-line macros can be generated using \

at the end of all but the last line.



#define UART_RECEIVE(n) \
void interrupt UART##n##Receive (void) { \
    buffer[pointer++] = UART##n##RX_REG; \
    if (end of packet condition) packet_received = 1; \
}

UART_RECEIVE(1)
UART_RECEIVE(2)

      

+4


source


Using macros for this purpose seems like a bad idea to me. Making it impossible to debug is just one drawback. It also makes it difficult to understand, hiding the real meaning of the symbols. And interrupt routines should always be independent and short, and common functions are hidden in handler functions.

The first thing I would like to do is define a common buffer structure for each UART. This makes simultaneous communication possible. If each uart needs a separate message handler function, it can be included as a function pointer. The syntax is a bit tricky, but it leads to efficient code.

typedef struct uart_buf uart_buf_t;
struct uart_buf {
    uint8_t* buffer;
    int16_t  inptr;
    bool packet_received;
    void (*handler_func)(uart_buf_t*);
};

uart_buf_t uart_buf_1;
uart_buf_t uart_buf_2;

      

Then each interrupt handler will look like this:

void interrupt UART1Receive (void) {
  handle_input(UART1RX_REG, &uart_buf_1);
}

void interrupt UART2Receive (void) {
  handle_input(UART2RX_REG, &uart_buf_2);
}

      

And a normal handler would be:



void handle_input(uint8_t in_char, *buff) {
   buf->buffer[buf->inptr++] = in_char; 
   if (in_char=LF) 
       buf->packet_received = true;
       buf->handler_func(buf);
   }
}

      

And the message handler:

void hadle_packet(uart_buf_t* buf) {
    ... code to handle message
    buf->packet_received=0;
}

      

And the function pointers must be initialized:

void init() {
    uart_buf_1.handler_func=handler1;
    uart_buf_2.handler_func=handler1;
}

      

The resulting code is very flexible and can be easily modified. One-shot code is not a problem.

+1


source







All Articles