tags:

views:

176

answers:

5

Hi all,

I have an application which runs on a controlling hardware connected with different sensors. On loading the application, it checks the individual sensors one by one to see whether there is proper communication with the sensor according to predefined protocol or not.

Now, I have implemented the code for checking the individual sensor communication as a singleton thread and following is the run function, it used select system call and pipe for interprocess communication to signal the end of thread.

void SensorClass::run()
{
    mFdWind=mPort->GetFileDescriptor();
    fd_set readfs;
    int max_fd = (mFdWind > gPipeFdWind[0] ? mFdWind : gPipeFdWind[0]) + 1;
    int res;

    mFrameCorrect=false;
    qDebug("BEFORE WHILE"); 
    while(true)
    {
        qDebug("\n IN WHILE LOOP"); 
        usleep(50);
        FD_ZERO(&readfs);
        FD_SET(mFdWind,&readfs);
        FD_SET(gPipeFdWind[0],&readfs);

        res=select(max_fd,&readfs,NULL,NULL,NULL);
        if(res < 0)
            perror("Select Failed");
        else if(res == 0)
            puts("TIMEOUT");
        else
        {
            if(FD_ISSET(mFdWind,&readfs))
            {
                puts("*************** RECEIVED DATA ****************");
                mFrameCorrect=false;
                FlushBuf();
                //int n=mPort->ReadPort(mBuf,100);
                int n=mPort->ReadPort(mBuf,100);

                if(n>0)
                {
                    Count++;


                    QString str((const char*)mBuf);
                    //qDebug("\n %s",qPrintable(str));
                    //See if the Header of the frame is valid
                    if(IsHeaderValid(str))
                    {
                        if( (!IsCommaCountOk(str)) || (!IsChecksumOk(str,mBuf)) ||  (!CalculateCommaIndexes(str))  ) 
                        {

                            qDebug("\n  not ok");
                            mFrameCorrect=false;
                        } //if frame is incorrect
                        else 
                        {
                            qDebug("\n  OK");
                            mFrameCorrect=true;
                        }//if frame is correct(checksum etc are ok)
                    }//else if header is ok
                }//if n > 0
            }//if data received FD_ISSET
            if(FD_ISSET(gPipeFdWind[0],&readfs))
                break;
        }//end nested else res not <= 0
    }//infinite loop  
}

The above thread is run started from the main GUI thread. This runs fine. The problem is I have given an option to the user to retest the subsystem at will. For this I delete the singleton instance using

delete SensorClass::instance();

and then restart the singleton using

SensorClass::instace()->start();

The problem is this time the control comes out of while loop in run() function immedeately upon entering the while loop, my guess is the pipe read has again read from the write pipe which was written to the last time. I have tried to use the fflush() to clear out the I/O but no luck. My question is

  1. Am I thinking on the right track?

  2. If yes then how do we clear out the pipes?

  3. If not can anyone suggest why is the selective retest not working?

Thanks in advance..

+1  A: 

fflush clears the output buffer. If you want to clear the input buffer, you're going to need to read the data or seek to the end.

I'm not convinced the "Singleton" pattern is appropriate. There are other ways of ensuring at most one instance for each piece of hardware. What if you later want multiple threads, each working with a different sensor?

outis
+1  A: 

Let's assume that you're creating this thread by inheriting from QThread (which you don't specify). From the documentation of QThread::~QThread ():

  • Note that deleting a QThread object will not stop the execution of the thread it represents. Deleting a running QThread (i.e. isFinished() returns false) will probably result in a program crash.

So the statement delete SensorClass::instance(); is probably a really, really bad idea. In particular, it's going to be tough making any sense of this program's behavior given this flaw. Before continuing, you might want to find a way to remove the instance and ensure that the thread goes away, too.

Another problem comes to mind. When you run delete SensorClass::instance(), you get rid of some object (on the heap, one hopes). Who tells the singleton holder that its object is gone? E.g. so that the next call to SensorClass::instance() knows it needs to allocate another instance? Is this handled properly in SensorClass::~SensorClass?

Suppose that's not a problem. That likely means that the pointer to the instance is held in a global variable (or, e.g. a class level static member). It probably doesn't matter for this situation, but is access to that member properly synchronized? I.e. is there a mutex that's locked for each access to it?

Managu
There are no crashes, tested that many times. That is the first thing I generally check. And when I write to the pipe in the main thread it signals this thread to stop by reading from the read pipe and control breaks out of the run function. The problem isn't the delete statement I suppose. It is when the thread is run the second time the pipe reads again and breaks out of run() defying the desired behavior. Do correct me if I am wrong.
rocknroll
You got it right with "Suppose that's not a problem. That likely means that the pointer to the instance is held in a global variable (or, e.g. a class level static member)." I am sure that that is not the problem because otherwise crashes or unwanted behavior would have resulted. It is only with the read write to pipes which is creating a problem I guess
rocknroll
Is one of these pipes used just to signal when the thread should die? Why not use a simpler mechanism, like setting a flag and then waiting for the thread to see it?
Managu
Also, being careless with thread synchronization can cause problems that don't pop up right away. They'll come later, and leave you totally mystified as to what's happening.
Managu
+1  A: 

You really don't want to run your initialization in thread. That is issue number one that dramatically complicates your problem and which is the kind of thing for some reason no one points out.

Just make the initialization its own function, then have a guard variable and lock, and have everything that uses it separately initialize it when they start up.

Charles Eli Cheese
thanks for replying but it doesn't solve the issue. outis has got the understanding of what the issue is. For the record "When I restart to check for communication with the sensor, the run() somehow doesn't read the incoming data, but finds the read file descriptor of the pipe written to and exits the thread.". I want to defy this behavior.
rocknroll
A: 

So you're signaling by writing something to the pipe, and the pipe is only created once - i.e. reused in the later threads?

Read the signaling away from the pipe. Assuming you signal by writing a single byte, then instead of just breaking out, you'd do something like (NB, no error checking etc below):

if(FD_ISSET(gPipeFdWind[0],&readfs)) {  
    char c;  
    read(gPipeFdWind[0], &c, 1);  
    break;  
}

There are also Qt classes for handling socket I/O, e.g. QTcpSocket, which would make the code not only cleaner, also more cross-platform. Or at least QSocketNotifier to abstract the select away.

Timo Metsälä
A: 

My suggestion is to have global function pointers for reading the sensors. This allows the functions to be redirected for: 1) Emulations, 2) Error handling when the sensors fail, and to the primary reading function. These pointers should be initialized by main() before any threads start. The global pointers could be default-initialized too.

For an embedded system, Singletons may be overkill. The function pointer eliminates the extra cost of checking a flag before deciding which method to use.

I've used this system with configuration files. The function pointers are initialized base on settings in the configuration file. This allows adjusting the functionality of the program without recompiling or rebuilding it.

Thomas Matthews