CODEWITHSUNDEEP
cmicrochipmplabxc16
Zac Soden
Zac Soden

Reputation: 31

C: Minimising code duplication using functions in a header file

this is a bit of a strange use case so searching for existing discussion is difficult. I'm programming for embedded systems (Microchip PIC24 using XC16 compiler) and am currently implementing a communication protocol identically across 3 separate UART channels (each UART will grab data from a master data table).

The way I started out writing the project was to have each UART handled by a separate module, with a lot of code duplication, along the lines of 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 is neat and works well, in practice the communication protocol itself is quite complex, so having it duplicated three times (simply with changes to references to the UART registers) is increasing the opportunity for bugs to be introduced. Having a single function and passing pointers to it is not an option, since this will have too great an impact on speed. The code needs to be physically duplicated in memory for each UART.

I gave it a lot of thought and despite knowing the rules of never putting functions in a header file, decided to try a specific header file that included the duplicate code, with references 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 slightly surprised when the code compiled without any errors! It does seem to work though, and post compilation MPLAB X can even work out all of the symbol references so that every macro reference in UART1.c and UART2.c don't get identified as an unresolvable identifier. I did then realise I should probably rename the protocol.h file to protocol.c (and update the #includes accordingly), but that's not practically a big deal.

There is only one downside: the IDE has no idea what to do while stepping through code included from protocol.h while simulating or debugging. It just stays at the calling instruction while the code executes, so debugging will be a little more difficult.

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

Upvotes: 3

Views: 433

Answers (2)

user206622
user206622

Reputation: 96

Using macros for this purpose seems for mee to be a bad idea. Making debugging impossible is just one disadvantage. It also makes it difficult to understand, by hiding the real meaning of symbols. And interrupt routines should realy be kept independant and short, with common functions hidden in handler functions.

The first thing I would do is to define a common buffer struct for each UART. This makes it possible with simultanous communications. If each uart needs a separate handler function for the messages, it can be included as a function pointer. The syntax is a bit complicated, but it results in 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 be 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 the common handler will 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 is:

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 changed. Single-steping the code is no problem.

Upvotes: 2

Mark Ransom
Mark Ransom

Reputation: 308500

An alternative is to define a function macro that contains the body of code. Some token pasting operators can automatically generate the symbol names required. Multi-line macros can be generated by 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)

Upvotes: 4

Related Questions