Parse source code of the RAPID robot-automation language

Question

I am new to C, and I have very little formal education on programming (Although I am currently in college). I work as a Robot Automation technician, and me and my team are required to document a lot of things. I wrote a program that we run on our robot computers to do this, but the language (RAPID by ABB Robotics) had its limitations, and the versions of operating systems vary from machine to machine. To get to the point, I decided that I need to re-write the application in a general language that would be better suited for this.

This code works, although I did not include the header files or other C files that are included to make this run because I am mainly looking for constructive criticism on the readability and design of this code. If necessary I can provide the other files, but I don't think they will be needed for what I'm asking.

Basically, this program reads the source code that I have pasted in a text file and writes all of the data to a csv file, which is then pasted into an Excel workbook that is formatted as a report.

In the text file that contains the source code that this program reads, there is an 80×15 array that I need all of the data from, and then there Is a procedure in the program that I look for after that.

In the 80×15 array, each array represents a robot position, and each index is a value that is used for that position. 4 of the digits are whole numbers that actually represent a binary byte, while the other 2 digits are a float and a whole number. All of the data that I get from the array goes into a struct array that is in another C file.

After the array is read, it looks for a procedure called SPRAY_POSITIONS(). In this function we have all the statements that tell the robot how to move and etc, For each statement, I increment my counter, and when the RETURN statement is reached, it will exit and store the total positions used in that function. Most of this data is also stored in the same struct array.

The syntax of the source code being read is similar to C, so it may help to just imagine that I'm parsing a C source code for an array declaration and a function declaration, and am extracting the necessary information from the code.

I apologize in advance if this is not a good question for this site, but I'm the only one at my work that knows how to program anything besides an if/else statement or a loop, and I definitely want to get some feedback on my code so that I can improve. Thank you all for your time!

 /******************************************************************************
 *  Title: Rapid Parser
 *  Author: 
 *  Date: 7/5/2019
 *
 *
 *
 *  Purpose:
 *  1) Parses a RAPID source file and extracts the following data ->
 *      ! Sweep Patterns
 *      ! Dwell Patterns
 *      ! Sweep Speeds
 *      ! Dwell Times
 *      ! Movement type 'j,l,c'
 *      ! Total Positions Used
 *
 *  2) Sets all data from purpose #1 to the sprayPositions structure array
 *
 *
 **********************************************************************************/


#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <ctype.h>

#include "spray_positions.h"
#include "Rapid_Parser.h"

#define RAPID_FILEPATH "rapid_program.txt"
#define MAX_STR_LENGTH 1000
#define COVER_HALF 1
#define EJECTOR_HALF 2
#define TRUE 1
#define FALSE 0

typedef int bool;

//Prototypes
static char* removeWhiteSpace(char* string);
static char* getDwellTime(char* line, int linePosition);
static char* getSweepSpeed(char* line, int linePosition);
static char* getDwellPattern(char* line, int linePosition, int side);
static char* getSweepPattern(char* line, int linePosition, int side);
static char* reverseString(char* input);
static char* getNozzlePattern(int wholeNumber, int side);
static void getAdditionalCommands(char* line);
static void getZoneType(char* line);
static void getMoveType(char* line);
static void updateLineIndex(int counter);
static bool checkForReturn(char* line);
static bool findWord(char* string, char* substring);
static void readSrcFile();
static void openSrcFile();
int getTotalPositions();


static FILE *fp;

static int totalSprayPositions = 1;
static int currentLineIndex = 0;    //Used to update line index

static bool foundSprayFunctionArray = FALSE;
static bool foundSprayPositions = FALSE;

static void readSrcFile()
{
    char ch;
    char dwellTime[5] = {0};
    char sweepPattern[10] = {0};
    char dwellPattern[10] = {0};
    char sweepSpeed[5] = {0};

    char currentLine[MAX_STR_LENGTH] = {0};

    //Boolean type integers
    bool insideBrackets = FALSE;

    //Counters
    int i;
    int currentPosition = 0;    //Same as dim1, but used for readability
    int dim1 = 0;               //Array dimension 1
    int dim2 = 0;               //Array dimension 2

    //Array Indexes for dimension 2
    int sweepA = 0;
    int sweepB = 1;
    int dwellA = 5;
    int dwellB = 6;
    int speed = 10;
    int time = 11;

    while (fgets (currentLine, MAX_STR_LENGTH, fp) != NULL)
        {
            //Convert line to uppercase to simplify string comparison
            strupr (currentLine);

            if (findWord (currentLine, "PERS NUM SPRAY_FUNCTION{80,15}"))
                foundSprayFunctionArray = TRUE;

            if (findWord (currentLine, "PROC SPRAY_POSITIONS()"))
                foundSprayPositions = TRUE;

            //If the sprayFunction array is found, read it
            if (foundSprayFunctionArray)
                {
                    // Loop through current line
                    for (i = 0; i < strlen(currentLine); i++)
                        {
                            ch = currentLine[i];

                            //If we hit a ';' we know it's the end of the declaration
                            if (ch == ';')
                                {
                                    foundSprayFunctionArray = FALSE;
                                    break;
                                }

                            //Used to tell if we're in an array or not
                            if (ch == '[')
                                insideBrackets = TRUE;
                            else if (ch == ']')
                                insideBrackets = FALSE;

                            //If we're inside an array, and not at a comma
                            if ((insideBrackets) && (ch != '[') && (ch != ']') && (ch != ','))
                                {
                                    //Sweep Pattern @ Index 0 & 1
                                    if (dim2 == sweepA)
                                        strcpy(sweepPattern, getSweepPattern(currentLine, i, COVER_HALF));
                                    else if (dim2 == sweepB)
                                        strcat(sweepPattern, getSweepPattern(currentLine, i, EJECTOR_HALF));

                                    //Dwell Pattern @ Index 5 & 6
                                    else if (dim2 == dwellA)
                                        strcpy(dwellPattern, getDwellPattern(currentLine, i, COVER_HALF));
                                    else if (dim2 == dwellB)
                                        strcat(dwellPattern, getDwellPattern(currentLine, i, EJECTOR_HALF));

                                    //Sweep Speed @ Index 10
                                    else if (dim2 == speed)
                                        strcpy(sweepSpeed, getSweepSpeed(currentLine, i));

                                    //Dwell Time @ Index 11
                                    else if (dim2 == time)
                                        strcpy(dwellTime, getDwellTime(currentLine, i));

                                    //Update Index counter
                                    dim2++;
                                    //Increment I to the current line index
                                    i += currentLineIndex;
                                    //Reset Index 
                                    currentLineIndex = 0;

                                    //If all 15 elements have been read in array
                                    if (dim2 == 15)
                                        {
                                            //Copy all data to position struct
                                            setPositionSweepPattern(currentPosition, sweepPattern);
                                            setPositionDwellPattern(currentPosition, dwellPattern);
                                            setPositionSweepSpeed(currentPosition, sweepSpeed);
                                            setPositionDwellTime(currentPosition, dwellTime);

                                            ////Reset sweepPattern, dwellPattern, sweepSpeed, Dwelltime
                                            memset(dwellTime,0,sizeof(dwellTime));
                                            memset(sweepSpeed, 0, sizeof(sweepSpeed));
                                            memset(sweepPattern, 0, sizeof(dwellPattern));
                                            memset(dwellPattern, 0, sizeof(dwellPattern));

                                            //Update Current Array position, and reset dim2
                                            currentPosition++;
                                            dim1++;
                                            dim2 = 0;
                                        }
                                }
                        }
                }


            if (foundSprayPositions)
                {

                    printf("%s\n", removeWhiteSpace(currentLine));

                    //If line isn't commented, read it
                    if (!findWord (currentLine, "!"))
                        {
                            if (checkForReturn(currentLine))
                                {
                                    foundSprayPositions = FALSE;
                                    //Print data for debugging
                                    printArrayDataByPosition(totalSprayPositions);
                                    system("pause");
                                    break;
                                }
                            else
                                {
                                    getMoveType(currentLine);
                                    getZoneType(currentLine);
                                    getAdditionalCommands(currentLine);
                                }
                        }
                }
        }
}


/* Checks to see if a certain substring exists in the line */
static bool findWord (char* string, char* substring)
{
 if (strstr (string, substring) != NULL)
        return TRUE;
    else
        return FALSE;
}

/* Opens the RAPID source file for reading */
static void openSrcFile()
{
    fp = fopen (RAPID_FILEPATH, "r");

    if (fp == NULL)
        {
            perror ("Error while opening the file\n");
            exit (EXIT_FAILURE);
        }
}

/* Closes the RAPID source file */
static void closeSrcFile()
{
    printf("CLOSING RAPID SOURCE FILE\n");
    fclose(fp);
}


/**********************************************************************
*   Takes a whole number, checks the '1' bits to determine which
*   Nozzles are turned on, then returns it as a string in readable format
*
*   EX: (input)2            (input) 255
*       (byte)00000010      (byte) 11111111
*       (output) 2          (output) 12345678
*
*   @param wholeNumber = The number representing the nozzle pattern
*   @param side = cover or ejector half, to determing the starting nozzle
**********************************************************************/
static char* getNozzlePattern(int wholeNumber, int side)
{
    char nozzlePattern[9] = {0};
    char* ptr = malloc(sizeof(char)*sizeof(nozzlePattern));

    int i,j=0;
    int startingNozzle = 0;

    memset(ptr, 0, sizeof(nozzlePattern));

    if (side == EJECTOR_HALF)
        startingNozzle = 2;

    for (i = 8; i > 0; i--)
        {
            if (wholeNumber & 1 << (i-1))
                {
                    nozzlePattern[j] = (i + startingNozzle) + '0';
                    j++;
                }
        }

    strcpy(ptr, nozzlePattern);
    strcpy(ptr, reverseString(ptr));

    return ptr;
}

/**********************************************************************
*   Takes a string as input, returns that same string reversed
*   Used to reverse the order a nozzle pattern to read from left to right
*   But also may be used to reverse any string
**********************************************************************/
static char* reverseString(char* input)
{
    int i,j;
    int stringSize = strlen(input);
    char* output = malloc(sizeof(char)*stringSize+1);

    memset(output, 0, stringSize+1);

    for (i = stringSize, j = 0; i > 0; i--, j++)
        output[j] = input[i-1];

    return output;
}


/**********************************************************************
*   Checks current line for 'MOVEJ', 'MOVEL', 'MOVEC' commands
*   If present, sets the movement type for the current spray position
**********************************************************************/
static void getMoveType(char* line)
{
    int i;
    char* moveType[] = {"MOVEJ", "MOVEL", "MOVEC"};
    size_t arraySize = sizeof(moveType)/sizeof(moveType[0]);

    for (i = 0; i < arraySize; i++)
        {
            if (findWord (line, moveType[i]))
                {
                    setPositionMoveType(totalSprayPositions-1, moveType[i]);
                    break;
                }
        }
}

/**********************************************************************
*   Checks the current line for zone/fine commands
*   If present, sets the zone type for the current spray position
**********************************************************************/
static void getZoneType(char* line)
{
    int i;
    char* zoneType[] = {"FINE", "Z200", "Z150", "Z100", "Z80", "Z60", "Z50", "Z40", "Z30", "Z20", "Z15", "Z10", "Z5", "Z1"};
    size_t arraySize = sizeof(zoneType)/sizeof(zoneType[0]);

    for (i = 0; i < arraySize; i++)
        {
            if (findWord (line, zoneType[i]))
                {
                    setPositionZoneType(totalSprayPositions-1, zoneType[i]);
                    totalSprayPositions++;
                    break;
                }
        }
}

/**********************************************************************
*   Checks the current line for other commands such as ->
*   Commands for cores in, blasters, etc
*   If present, it will put that line in the additional commands
*   Attribute for the current position
**********************************************************************/
static void getAdditionalCommands(char* line)
{
    int i;
    char* otherCommands[] = {"PULSEDO", "SET", "RESET", "WAITDO", "WAITDI"};
    size_t arraySize = sizeof(otherCommands)/sizeof(otherCommands[0]);

    for (i = 0; i < arraySize; i++)
        {
            if (findWord (line, otherCommands[i]))
                {
                    setPositionAdditionalCommands(totalSprayPositions-1, removeWhiteSpace(line));
                    break;
                }
        }
}

/************************************************************************
*   Checks to see if the *RETURN statement is present in the line
*   If it is, decrement the total positions and return true
************************************************************************/
static bool checkForReturn(char* line)
{
    if (findWord (line, "RETURN"))
        {
            totalSprayPositions--;
            printf ("TOTAL POSITIONS: %d\n", totalSprayPositions);
            return TRUE;
        }
    else
        return FALSE;
}


/**********************************************************************
*   Gets the whole number that represents the *SWEEP* nozzle pattern
*   Breaks it down to binary, then flips it and formats it to read
*   as we would write it '1234'
**********************************************************************/
static char* getSweepPattern(char* line, int linePosition, int side)
{
    int i;
    char temp[5] = {0};
    char* pattern = malloc(sizeof(char) * 10);

    memset(pattern, 0, 10);

    for (i = 0; line[linePosition+i] != ','; i++)
        temp[i] = line[i+linePosition];
    updateLineIndex(i);

    //Sweep A
    if (side == COVER_HALF)
        strcpy(pattern, getNozzlePattern(atoi(temp), COVER_HALF));
    //Sweep B
    else
        strcpy(pattern, getNozzlePattern(atoi(temp), EJECTOR_HALF));

    return pattern;
}

/**********************************************************************
*   Gets the whole number that represents the *DWELL* nozzle pattern
*   Breaks it down to binary, then flips it and formats it to read
*   as we would write it '1234'
**********************************************************************/
static char* getDwellPattern(char* line, int linePosition, int side)
{
    //printf("Getting dwell pattern side %d\n", side);
    int i;
    char temp[5] = {0};
    char* pattern = malloc(sizeof(char) * 10);

    memset(pattern, 0, 10);

    for (i = 0; line[linePosition+i] != ','; i++)
        temp[i] = line[i+linePosition];
    updateLineIndex(i);

    //DWELL A
    if (side == COVER_HALF)
        strcpy(pattern, getNozzlePattern(atoi(temp), COVER_HALF));
    //DWELL B
    else
        strcpy(pattern, getNozzlePattern(atoi(temp), EJECTOR_HALF));

    return pattern;
}


/**********************************************************************
*   Starts at the current line position, reads it until it hits a comma
*   Then sets the sweep speed attribute for the current spray position
*   Updates the line position also
**********************************************************************/
static char* getSweepSpeed(char* line, int linePosition)
{
    int i;
    char temp[5] = {0};
    char* speed = malloc(sizeof(char) * 6);

    memset(speed, 0, sizeof(temp));

    //printf("Getting sweep speed\n");

    for (i = 0; line[linePosition+i] != ','; i++)
        temp[i] = line[i+linePosition];
    updateLineIndex(i);

    strcpy(speed, temp);

    return speed;
}


/**********************************************************************
*   Starts at the current line position, reads it until it hits a comma
*   Then sets the dwell time attribute for the current spray position
*   Updates the line position also
**********************************************************************/
static char* getDwellTime(char* line, int linePosition)
{
    int i;
    char temp[5] = {0};
    char* time = malloc(sizeof(char) * 5);

    memset(time, 0, sizeof(temp));

    //printf("Getting Dwell Time\n");

    for (i = 0; line[linePosition+i] != ','; i++)
        temp[i] = line[linePosition + i];

    updateLineIndex(i);

    strcpy(time, temp);

    return time;
}

/**********************************************************************
*   Updates the line position after a function has parsed the line
*   looking for information such as sweep speed, dwell time, etc
*   to keep up with the position in the array
*
*   ex: starting character for sweep speed may be 5,
*       but the value may be 5000, so we would need to update
*       the line position after the other 3 characters were read
**********************************************************************/
static void updateLineIndex(int counter)
{
    currentLineIndex += counter;
}

/******************************************************************
*   Takes a string as input, returns it without tabs or spaces
*   Used to put whole line into the additional commands
*   Attribute, but can be used for any string
******************************************************************/
static char* removeWhiteSpace(char* string)
{
    int i;
    int j;
    int len = strlen(string);
    char ch;
    char carriageReturn = '\r';
    char lineFeed = '\n';
    char space = ' ';
    char tab = '\t';
    char* result = malloc(sizeof(char)*len+1);

    memset(result, 0, len+1);

    j=0;
    for (i=0; i<len; i++)
    {
        ch = string[i];

        if ((ch == carriageReturn) || (ch == lineFeed))
            break;

        if ((ch != space) && (ch != tab))
            {
                result[j] = ch;
                j++;
            }
    }

    result[j] = '\0';

    return result;
}

int getTotalPositions()
{
    return totalSprayPositions;
}

/* Global function that is called by main */
void getDataFromRapidSrcFile()
{
    openSrcFile();
    readSrcFile();
    closeSrcFile();
}

The Spray Positions header contents

/************************************************************************
*   The functions in this module are used
*   to set data for a spray position struct
*   There are 80 structures created, since
*   there are a max of 80 spray positions.
*
*   @param positionNumber is the current
*   spray position to be updated
*
*   @param movement type is the type of move
*   used in that position, L,J,C
*
*   @param zoneType is the type of zone used
*   in the position, FINE,Z1-100
*
*   @param additionalCommands is used to
*   store other commands associated with a certain
*   position such as SET, RESET, PULSDO etc
*
*   @param sweepPattern is the formatted version
*   of the binary byte that tells us which nozzles
*   are turned on during the sweep function for the
*   specified position.
*   Example: *0011* would translate to *12* and *12*
*   would be the pattern used
*
*   @param dwellPattern is the same as the sweepPattern,
*   but is used to show which nozzles are on during the
*   dwell function of the specified position
*
*   @param sweepSpeed is how fast the robot is moving
*   to the specified position
*
*   @param dwellTime is how long the robot stays
*   at the specified position before moving to the
*   next positon
**************************************************************************/

void setPositionMoveType(int positionNumber, char* movementType);
void setPositionZoneType(int positionNumber, char* zoneType);
void setPositionAdditionalCommands(int positionNumber, char* additionalCommands);
void setPositionSweepPattern(int positionNumber, char* sweepPattern);
void setPositionDwellPattern(int positionNumber, char* dwellPattern);
void setPositionSweepSpeed(int positionNumber, char* sweepSpeed);
void setPositionDwellTime(int positionNumber, char* dwellTime);

void writeSprayData(int totalSprayPositions, FILE* fp);


/* Used for debugging, prints all data for each position that is used */
void printArrayDataByPosition(int position);

The rapid parser header file (I don't think I actually should have included this in my main code because It comes from the same module)

/*********************************************************
*   Gets all of the data stored in the program file
*   Then updates each spray position struct
*   With the information obtained from that file
*********************************************************/
void getDataFromRapidSrcFile();

/* Returns the total amount of spray positions in use */
int getTotalPositions();

Answer 1

These declarations are not prototypes:

void getDataFromRapidSrcFile();

int getTotalPositions();

These declare functions that can be called with any number of arguments. It appears that they should take no arguments; we indicate that like this:

void getDataFromRapidSrcFile(void);

int getTotalPositions(void);

It's a good idea to make the same change where the functions are defined, too.

---

You could remove a repetitive (error-prone) construct using a macro to determine the number of elements in an array:

#define ARRAY_SIZE(x) (sizeof (x) / sizeof (x)[0])

---

Sorry I didn't get time for a full review of this - you do have a few other good answers now, though. If you ask a new question to review your improved code, then I hope to be able to look at that.

Answer 2

Functions should be very short: as a maximum, 2 or 3 screens (considering a screen size of 24 lines), but much less if possible; and shouldn't indent more than 2 or 3 levels of indentation normally. You should try to break that big fat function into a lot of small functions.

Answer 3

Regarding:

char* speed = malloc(sizeof(char) * 6);

1. The expression sizeof(char) is defined in the C standard as 1. Multiplying anything by 1 has no effect and just clutters the code, making it more difficult to understand, debug, etc.

2. When calling any of the heap allocation functions: malloc() calloc() realloc(), always check (!=NULL) the returned value to assure the operation was successful.

3. It is a poor programming practice to use dynamic memory allocation (the call to malloc()) in an embedded application.

---

Regarding:

char temp[5] = {0};
char* speed = malloc(sizeof(char) * 6);

memset(speed, 0, sizeof(temp));

The call to malloc() allocated 6 bytes, but sizeof temp is only 5 bytes. The result is that the last byte of speed is not initialized.

Suggest calling calloc() rather than malloc(), which will initialize the allocated memory to all 0x00.

Regarding the numbers 5, 6. 10.

These are 'magic' numbers. 'magic' numbers have no basis. Suggest using an enum statement or #define statements to give those 'magic' numbers meaningful names, then use those meaningful names throughout the code.

regarding:

typedef int bool;

there is already the header file: stdbool.h which defines bool, true, false

Answer 4

You have a memory leak. Well several.

All of the static char * functions allocate memory using malloc() which is never freed.

Now let's take a look at how they are used, for example

strcpy(sweepSpeed, getSweepSpeed(currentLine, i));

So getSweepSpeed() is returning a pointer to a string, which is immediately copied into another string sweepSpeed, then the returned pointer is discarded and the buffer allocated by getSweepSpeed() is leaked.

An alternative which would work here would be to get getSweepSpeed() to write its result directly into the buffer sweepSpeed. So the call becomes

getSweepSpeed(currentLine, i, sweepSpeed);

and the function definition becomes

static void getSweepSpeed(char* line, int linePosition, char * speed)

I'm sure you can do the rest..