Search code examples
cparsingavrreadability

Ugly Triple Indirection: Extensible Buffer Management Struct


I'm currently trying to build a string parser for an AVR based stepper motor controller. The idea is to observe some input string over UART, then breaking apart this string into several sub-buffers that will eventually find their way into a set of motor control behaviors. Considering the following delimiters:

'<' - START
'%' - CHANGE BUFFER
'>' - STOP
'A' - Clockwise
'B' - Counterclockwise

I would take in a somewhat arbitrarily wide string that looks something like <1234A%7743B> and turn that into "1234 steps clockwise on motor 1, 7743 steps counterclockwise on motor B".

So far, I've come up with the following solution

#include <stdio.h>
#include <stdint.h>

#define TARGET 12

typedef struct 
{
    uint8_t STRING_SEL;

    char  X    [32]; // buffer X
    char  Y    [32]; // buffer Y
    uint8_t  iX; // buffer X iterator
    uint8_t  iY; // buffer Y iterator
    uint8_t  PTR_CTR;

    char* Xptr; // pointer to buffer X
    char* Yptr; // pointer to buffer Y

    uint8_t * iXptr; // pointer to buffer X iterator
    uint8_t * iYptr; // pointer to buffer Y iterator

    char ** PTR_ARR [2];   //pointer to array of buffer initial value pointers
    uint8_t ** CTR_ARR[2]; //pointer to array of buffer iterator pointers

} parser_manager;

static parser_manager mgr =  {
                              .STRING_SEL = 0,
                              .X          = {[0 ... 31] = 0},
                              .Y          = {[0 ... 31] = 0},
                              .iX         = 0,
                              .iY         = 0,
                              .PTR_CTR    = 0,
                              .Xptr       = &mgr.X[0],
                              .Yptr       = &mgr.Y[0],
                              .iXptr      = &mgr.iX,
                              .iYptr      = &mgr.iY,
                              .PTR_ARR    = {&mgr.Xptr, &mgr.Yptr},
                              .CTR_ARR    = {&mgr.iXptr, &mgr.iYptr}
                             };

Which lands me in three star programming hell, and lends itself to yucky member accesses like:

char EXPECT_CHAR = *(*(*(mgr.PTR_ARR + mgr.PTR_CTR)) + (***(mgr.CTR_ARR + mgr.PTR_CTR)));
(mgr.Y[TARGET] == EXPECT_CHAR) ? printf("1\n") : printf("0\n");

So why do something this ugly? I went about this with the rationale that while parsing through the incoming characters, I would increment PTR_CTR for every '%' character, thus switching iterators and buffers cleanly. I could then extend this out for some N number of motors by adding the buffers and pointers to the struct, then widening the PTR and CTR arrays.

I can see that I could save myself a level of indirection with the counter array, as it doesn't need to be an array of double pointers.

This feels like an extremely inelegant way of going about this problem. How do I make this process cleaner? My only thought so far is to move to fixed width packets, nixing the need for this faux-dynamism.


Solution

  • You have X and Y as member of structure parser_manager:

        char  X    [32]; // buffer X
        char  Y    [32]; // buffer Y
    

    You have also mentioned

    I could then extend this out for some N number of motors by adding the buffers and pointers to the struct, then widening the PTR and CTR arrays.

    How are you planning to do it? (by adding more members like X and Y in parser_manager structure!)

    Better to create a different structure having member X and Y. Take a pointer of that structure in parser_manager structure so that you can extend it by reallocating memory. With this, you can do away with multiple level of indirection's when accessing X and Y buffers.

    Sample code for demonstration purpose:

    #include <stdio.h>
    #include <stdlib.h>
    #include <string.h>
    
    typedef struct {
        char X[32];
        char Y[32];
    } buffers;
    
    typedef struct {
        buffers * ptr_x_y;
        int numele;   // to keep track of number of elements
    } parser_manager;
    
    int main (void)
    {
        parser_manager mgr = {NULL, 0};
    
        mgr.numele = 1;
        mgr.ptr_x_y = malloc (sizeof (buffers) * mgr.numele);
        if (!mgr.ptr_x_y) {
            exit (EXIT_FAILURE);
        }
    
        strcpy (mgr.ptr_x_y[0].X, "DUMMY_X1");
        strcpy (mgr.ptr_x_y[0].Y, "DUMMY_Y1");
        printf ("%s, %s\n", mgr.ptr_x_y[0].X, mgr.ptr_x_y[0].Y);
        
        mgr.numele = 2;
        buffers * temp;
        mgr.ptr_x_y = realloc (temp = mgr.ptr_x_y, mgr.numele * sizeof (buffers));
        if (!mgr.ptr_x_y) {
            free (temp);
            exit (EXIT_FAILURE);
        }
    
        strcpy (mgr.ptr_x_y[1].X, "DUMMY_X2");
        strcpy (mgr.ptr_x_y[1].Y, "DUMMY_Y2");
        printf ("%s, %s\n", mgr.ptr_x_y[0].X, mgr.ptr_x_y[0].Y);
        printf ("%s, %s\n", mgr.ptr_x_y[1].X, mgr.ptr_x_y[1].Y);
        
        // free dynamically allocted memory
        
        return 0;
    }
    

    Output:

    DUMMY_X1, DUMMY_Y1
    DUMMY_X1, DUMMY_Y1
    DUMMY_X2, DUMMY_Y2
    

    iX and iY are iterators of buffer X and Y, therefore you can make them member of buffers structure as well.

    If for some reason, you cannot use dynamic memory allocation functions malloc or realloc, then you can do:

    #include <stdio.h>
    
    #define MAX_BUFFERS 10
    
    typedef struct {
        char X[32];
        char Y[32];
    } buffers;
    
    typedef struct {
        buffers * ptr_x_y[MAX_BUFFERS];
        int numele;   // to keep track of number of elements
    } parser_manager;
    
    int main (void)
    {
        parser_manager mgr = {{NULL}, 0};
        
        buffers buf1 = {.X = "DUMMY_X1", .Y = "DUMMY_Y1"};
        
        mgr.numele = 1;
        mgr.ptr_x_y[0] = &buf1;
        
        printf ("%s, %s\n", mgr.ptr_x_y[0]->X, mgr.ptr_x_y[0]->Y);    
    
        buffers buf2 = {.X = "DUMMY_X2", .Y = "DUMMY_Y2"};
        
        mgr.numele = 2;
        mgr.ptr_x_y[1] = &buf2;
        
        printf ("%s, %s\n", mgr.ptr_x_y[1]->X, mgr.ptr_x_y[1]->Y);
       
        return 0;
    }
    

    Output:

    DUMMY_X1, DUMMY_Y1
    DUMMY_X2, DUMMY_Y2