CODEWITHSUNDEEP
cstm32stm32f4
AJT
AJT

Reputation: 59

C macro with #ifdef

I'm wondering if there is a better way of implementing this code :

if (strcmp(port_p, all_ports_a[inc++]) == 0)
    {
        #ifdef GPIOA
            HAL_GPIO_WritePin(GPIOA, all_pins_a[offset] , atoi(args[OFF_VALUE_WRITE]));
            SER_send("Value written\r", strlen("Value written\r"));
            return 1;
        #endif
        //b
    }else if (strcmp(port_p, all_ports_a[inc++]) == 0){
        #ifdef GPIOB
            HAL_GPIO_WritePin(GPIOB, all_pins_a[offset] , atoi(args[OFF_VALUE_WRITE]));
            SER_send("Value written\r", strlen("Value written\r"));
            return 1;
        #endif
        //c
    }else if (strcmp(port_p, all_ports_a[inc++]) == 0){
        #ifdef GPIOC
            HAL_GPIO_WritePin(GPIOC, all_pins_a[offset] , atoi(args[OFF_VALUE_WRITE]));
            SER_send("Value written\r", strlen("Value written\r"));
            return 1;
        #endif
        //etc.

especially this part :

#ifdef GPIOA
      HAL_GPIO_WritePin(GPIOA, all_pins_a[offset] , atoi(args[OFF_VALUE_WRITE]));
      SER_send("Value written\r", strlen("Value written\r"));
      return 1;
#endif

i want to replace that by a macro, something like :

MY_MACRO(GPIOx, MESSAGE) :

#ifdef GPIOx
      HAL_GPIO_WritePin(GPIOx, all_pins_a[offset] , atoi(args[OFF_VALUE_WRITE]));
      SER_send(MESSAGE, MESSAGE);
      return 1;
#endif

I know that we can't add #ifdef in a macro but maybe I missed something ? One line instead of 5 would be really nice !

Thanks !

AJT

Upvotes: 1

Views: 719

Answers (2)

user694733
user694733

Reputation: 16047

You could do something completely different. Arrange data in tables, then find row with matching name, and use values on the row.

First define table with all necessary data:

// Definition of single row
typedef struct {
    char name[NAME_MAX];
    GPIO_T gpio;
    char message[MESSAGE_MAX];
} Port_T;

// Table with all rows
Port_T const ports[] = {
#ifdef GPIOA
    { "GPIOA", GPIOA, "GPIOA message" },
#endif
#ifdef GPIOB
    { "GPIOB", GPIOB, "GPIOB message" },
#endif
 ...
};
// Number of rows on the table
size_t const portCount = sizeof ports / sizeof *ports;

Then replace if/else chain with for loop:

for(size_t i=0; i<portCount; ++i) {
    Port_T const * port = &ports[i];
    if(strcmp(port_p, port->name) == 0) {
        HAL_GPIO_WritePin(port->gpio, all_pins_a[offset], atoi(args[OFF_VALUE_WRITE]));
        SER_send(port->message, strlen(port->message));
        return 1;
    }
}

You could also use more advanced search routines than simple for loop, like binary search.

Upvotes: 3

Ian Abbott
Ian Abbott

Reputation: 17503

Here is one possible solution, although it is probably not an improvement on the original code!

#ifdef GPIOA
#define MY_MACRO_GPIOA(MESSAGE) MY_MACRO_(GPIOA, MESSAGE)
#else
#define MY_MACRO_GPIOA(MESSAGE) do; while (0)
#endif

#ifdef GPIOB
#define MY_MACRO_GPIOB(MESSAGE) MY_MACRO_(GPIOB, MESSAGE)
#else
#define MY_MACRO_GPIOB(MESSAGE) do; while (0)
#endif

#ifdef GPIOC
#define MY_MACRO_GPIOC(MESSAGE) MY_MACRO_(GPIOC, MESSAGE)
#else
#define MY_MACRO_GPIOC(MESSAGE) do; while (0)
#endif

#define MY_MACRO_(GPIOx, MESSAGE) \
    do { \
        HAL_GPIO_WritePin(GPIOx, all_pins_a[offset] , atoi(args[OFF_VALUE_WRITE])); \
        SER_send(MESSAGE, strlen(MESSAGE)); \
        return 1; \
    } while (0)

#define MY_MACRO(GPIOx, MESSAGE) MY_MACRO_##GPIOx(MESSAGE)

...

    if (strcmp(port_p, all_ports_a[inc++]) == 0)
        MY_MACRO(GPIOA, "Value written\r");
    else if (strcmp(port_p, all_ports_a[inc++]) == 0)
        MY_MACRO(GPIOB, "Value written\r");
    else if (strcmp(port_p, all_ports_a[inc++]) == 0)
        MY_MACRO(GPIOC, "Value written\r");

Upvotes: 3

Related Questions