PDA

View Full Version : Recursive function crashing progam



N3wb
11th November 2010, 17:29
I'm having trouble with a recursive function crashing my program. I'll post the function and then explain what it does:


void MainWindow::sendGlobalCoordinates()
{
if(myThread->TXFlag == 1)
{
return sendGlobalCoordinates(); // try try again
}
else // TXFlag == 0
{
if(globalCoordinatesSent == 0) // if x&y not sent, Send the x
{
myThread->TXBytes[0] = 56; // setGlobalPosX
myThread->TXData = (int)(pointVector[calPoint2].gX * (double)4000);
myThread->bytesToSend = 4; // command(1) + data(2) + sign(1)
myThread->TXFlag = 1;
globalCoordinatesSent++;
}
else if(globalCoordinatesSent == 1) // If x sent, but y not sent, sent the y
{
myThread->TXBytes[0] = 57; // setGlobalPosY
myThread->TXData = (int)(pointVector[calPoint2].gY * (double)4000);
myThread->bytesToSend = 4; // command(1) + data(2) + sign(1)
myThread->TXFlag = 1;
globalCoordinatesSent++;
}
else if(globalCoordinatesSent == 2) // if x&y sent
{
startButton->setEnabled(1); // let the drilling begin!
return; // the recursive loop must be broken.
}
return sendGlobalCoordinates(); // keep going
}
}

OK, so here's the deal. I have a QThread running - named myThread - and its sole purpose is to send and receive data over the serial port. The function you see above was written to solve a problem. I need to send two sets of data over the serial port, but the second set of data can't be sent until the first one has been sent. So my idea was to write this recursive function which first sends the first set of data (if nothing is currently being sent over the serial port) and then, after that, sends the second set of data. If the serial port is busy sending data then the function just returns calling itself and eventually the job should get done. Brilliant idea, right? Unfortunately, the program crashes when I run it, and I know that the recursion is causing the problem (commenting out " return sendGlobalCoordinates();" keeps the program from crashing).

Any advice as to what I should do?

SixDegrees
11th November 2010, 17:55
What's the return value of a void function?

Timoteo
11th November 2010, 18:28
What's the return value of a void function?

The answer is: nothing, which is what a function with a void return type expects. Now for your question: what does this have to do with the question?:D

To the OP: Can you show more code? Like the complete definition of MainWindow?

tbscope
11th November 2010, 18:31
I need to send two sets of data over the serial port, but the second set of data can't be sent until the first one has been sent.

Use a queue.

N3wb
11th November 2010, 19:34
To the OP: Can you show more code? Like the complete definition of MainWindow?

Sure, I can post more code. I'm not sure what's relevant to the problem though and there's a couple thousand lines in the program, too long to post on here. But here's the definition of MainWindow:


#ifndef MAINWINDOW_H
#define MAINWINDOW_H

#include <QMainWindow>
#include "baction.h"
#include "caction.h"
#include "point.h"
#include "serial.h"
#include <QInputDialog>
#include "arrowbutton.h"

#include "vector"
using namespace std;

class MainWindow : public QMainWindow
{
Q_OBJECT // Q_OBJECT macro manditory for classes containing signals and slots.

public:
MainWindow(); // Constructor

private slots:
void setBaud(int);
void setCom(int);
void setComCustom();
void open();
void arrowClicked(QString number);
void setCalPoint1();
void setCalPoint2();
void startDrilling();
void pauseDrilling();
void abortDrilling();
void keyPressEvent(QKeyEvent *key);
void testSlot(QString message);
void sendData();
//void calPoint1Changed(int value);
//void calPoint2Changed(int value);
void changeAngle(int);
void SignalUpdate();

private:
void createMenus(); // Creates the menus
void createStatusBar(); // Creates the status bar
void paintEvent( QPaintEvent *event );
void getDrillCoordinates(QString);
void calculateGlobalCoordinates();

void mousePressEvent(QMouseEvent * e); // new

void sendGlobalCoordinates();

QMenu *fileMenu;
QMenu *baudMenu;
QMenu *comMenu;
QMenu *helpMenu;

// Declare file menu actions
QAction *fileExitAction;
QAction *fileOpenAction;
// Declare baud menu actions
BAction *baud110Action;
BAction *baud600Action;
BAction *baud2400Action;
BAction *baud9600Action;
BAction *baud56000Action;
BAction *baud115200Action;
// Declare com menu actions
CAction *com1Action;
CAction *com2Action;
CAction *com3Action;
CAction *com4Action;
CAction *com5Action;
CAction *com6Action;
CAction *comCustomAction;
// Declare help menu actions
QAction *helpContentsAction;
QAction *helpAboutAction;

QTextEdit *debugOutput;
QTextEdit *serialOutput;

QVector<int> *xC; // Hole coordinates
QVector<int> *yC;

//QVector<point> *pointVector;
vector<point> pointVector;

bool drillFileLoaded;
bool drillCalibrated;

int desiredX; // desired position of drill from origin in steps
int desiredY;
int desiredZ;

QLabel *statusLabel;
QLabel *drillLoadedLabel;
QLabel *calibrateStatusLabel;
QLabel *drillStatusLabel;
QLabel *boardHeightLabel;
QLabel *boardWidthLabel;
QLabel *calibrateLabel;
QLabel *point1Label;
QLabel *point2Label;
QLabel *desiredZLabel;

Thread *myThread;

QRadioButton *stepRadio1;
QRadioButton *stepRadio2;
QRadioButton *stepRadio3;
QRadioButton *stepRadio4;
QRadioButton *stepRadio5;
int stepRadio1Value;
int stepRadio2Value;
int stepRadio3Value;
int stepRadio4Value;
int stepRadio5Value;

int calPoint1Set;
int calPoint2Set;

QPushButton *point1Button;
QPushButton *point2Button;
unsigned int calPoint1;
unsigned int calPoint2;

QSpinBox *calPoint1SpinBox;
QSpinBox *calPoint2SpinBox;

QPushButton *startButton;
QPushButton *pauseButton;
QPushButton *abortButton;

ArrowButton *upButton;
ArrowButton *downButton;
ArrowButton *leftButton;
ArrowButton *rightButton;
ArrowButton *zUpButton;
ArrowButton *zDownButton;

double rotate;

int flipBoard;

int clickedX;
int clickedY;
int mouseClicked;


double boardWidth;
double boardHeight;

int globalCoordinatesSent; // 0 = none, 1 = x sent, 2 = y sent.
};

#endif // MAINWINDOW_H



Use a queue.

I thought about implementing some sort of buffer, but it would rather complicate things and I was just looking for a simple and quick solution..

Timoteo
11th November 2010, 20:36
Cleaned this up a little for you.


void MainWindow::sendGlobalCoordinates()
{
if(myThread->TXFlag == 0)
{

if(globalCoordinatesSent == 0) // if x&y not sent, Send the x
{
/*is this actually safe? what are the assumptions about the validity of these writes?*/
/*synchronization?*/
myThread->TXBytes[0] = 56; // setGlobalPosX
myThread->TXData = (int)(pointVector[calPoint2].gX * (double)4000);
myThread->bytesToSend = 4; // command(1) + data(2) + sign(1)
myThread->TXFlag = 1;
globalCoordinatesSent++;
}
else if(globalCoordinatesSent == 1) // If x sent, but y not sent, sent the y
{
myThread->TXBytes[0] = 57; // setGlobalPosY
myThread->TXData = (int)(pointVector[calPoint2].gY * (double)4000);
myThread->bytesToSend = 4; // command(1) + data(2) + sign(1)
myThread->TXFlag = 1;
globalCoordinatesSent++;
}
else if(globalCoordinatesSent == 2) // if x&y sent
{
startButton->setEnabled(1); // let the drilling begin!
return; // the recursive loop must be broken.
}
}
sendGlobalCoordinates(); // keep going
}

SixDegrees
11th November 2010, 20:51
Use a queue.

...or a debugger.

N3wb
11th November 2010, 21:38
Cleaned this up a little for you.

Thanks, that is a bit clearer.

Unfortunately the program still crashes.


...or a debugger.

I actually tried using the debugger in Qt, but due to my lack of experience I accomplished nothing.

SixDegrees
11th November 2010, 23:11
What error messages are produced when the program fails?

N3wb
12th November 2010, 01:54
Well, the program window kind of greys out and a Windows dialog box pops up saying "test_project.exe has stopped working." I click "close the program" and in "application output" in Qt Creator it says "... test_project.exe exited with code -1073741571"


#
/*is this actually safe? what are the assumptions about the validity of these writes?*/
#
/*synchronization?*/

There is no confirmation that the data was sent and received correctly. I've never had data lost before so I just assume it transfers OK every time..

ChrisW67
12th November 2010, 03:52
Just looking at the few lines at the start:

void MainWindow::sendGlobalCoordinates()
{
if(myThread->TXFlag == 1)
{
return sendGlobalCoordinates(); // try try again
}

Assuming that TXFlag is 1 how many times is sendGlobalCoordinates() going to call itself before it returns? For every call there must be a matching return; are you sure you will get that?

Every time you call a recursive function it has to record where to return to and create any new local variables on the stack. If the function calls itself in a tight loop like this the stack grows very quickly and eventually hits a system limit or exhausts system memory. How quickly will that happen? Assuming the TXFlag changes after a 4 characters are sent via the serial port at 9600 baud, that will take about 4 msecs (it may be longer). In that time your tight loop may call itself several thousand times. If the data block is longer then the situation gets worse. If your returns don't match your calls then these resources are permanently tied up.

This problem does not strike me as a classic case for recursion. Have you thought about having the sender thread signal when it is done?

Incidentally, 4 bytes does not seem to be correct: 1 byte command, an int data (4 bytes, but you should use sizeof(int)), and the sign (wherever that is defined).

SixDegrees
12th November 2010, 07:55
Code -1073741571 indicates a stack overflow, in line with ChrisW67's post. Sounds like your recursion isn't terminating.

N3wb
13th November 2010, 02:21
I still haven't been able to figure this out, so I'm just going to forget trying to use a recursive function. I'm not sure why it's not working, but it's more trouble than it's worth. I'll just use some sort of signal like someone else suggested.

Thanks for all of your help guys!