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 ×ばつ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 ×ばつ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();

• user3629249: the posted code includes two 'home grown' header files, but fails to post the contents of those files.
• RobotMan: @user3629249 apologies, I posted them.
• Sean Burton: Whenever I see code where someone is manually parsing text like this, my first question has to be, why are not just using a proper lexer/parser? I'm sure this would be a lot more maintainable if you used Flex/Bison or similar. Personally I like Boost::Spirit though that's C++ only.
• RobotMan: @SeanBurton wow I can't believe I didn't think of that lol. I will definitely look into what you posted, but I still plan on finishing this project because the main goal is to gain a deeper understanding of C as well as programming in general.

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.

• alx - recommends codidact: For completeness: C17::6.11.6: "Function declarators The use of function declarators with empty parentheses (not prototype-format parameter type declarators) is an obsolescent feature."
• RobotMan: @Toby Speight hey no problem man, I'm grateful for any help I can get. I was a little concerned that this code would be hard to review since I'm assuming most of the people reading it probably don't work in robotics. It was a pleasant surprise to see so many people willing to help. And I do plan to apply the good suggestions people have provided, and repost it as soon as I get the time.

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

• hobbs: To be fair I think that element_type *foo = malloc(sizeof(element_type) * n_elements) is a perfectly good thing to write, and if the type happens to be char then so be it... it's consistent and there's absolutely no loss in understandability. It's not "clutter".
• alx - recommends codidact: @hobbs I think it's objectively safer to use the variable, and not its type, as argument of sizeof. ptr = malloc(sizeof(*ptr) * nmemb);
• slebetman: @user3629249 The code does not do nothing - it tells maintainers of the code what object we want multiples of. Code that conveys information to humans is significantly more important than code that gets compiled to machine instructions
• Rodney: ...poor programming practice to use dynamic memory allocation ... in an embedded application you got a citation for that? If the system has a heap then why not use it? I should add, don't use it like this though, it has a leak (which I'm going to add as an answer)
• Toby Speight: If we do pointlessly multiply by 1, we should at least multiply the whole size by 1: eg result = malloc(sizeof(char)*len + 1) should be result = malloc(sizeof *result * (len+1)) so that if it's changed to a wchar_t, we allocate a whole wchar_t for the terminating null.

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..

• RobotMan: Yes that does look like a better answer. I thought that function variables only existed in the function though, so I thought that the memory allocated inside the function would automatically free after the function is complete.
• Rodney: @RobotMan memory allocated with malloc() can only be freed using free(). The fact that you assigned the return value of malloc to a pointer which is local to that function does not mean the buffer it points to will be deallocated at the end of the function. Your buffer temp[] is automatically allocated and deallocated since temp is a buffer, not just a pointer to one. But you shouldn't return temp as this would be returning a pointer to a buffer that's been deallocated, which is undefined behaviour.