Serial data acquisition program reading from buffer

Question

I have developed an application in Visual C++ 2008 to read data periodically (50ms) from a COM Port. In order to periodically read the data, I placed the read function in an OnTimer function, and because I didn't want the rest of the GUI to hang, I called this timer function from within a thread. I have placed the code below.

The application runs fine, but it is showing the following unexpected behaviour: after the data source (a hardware device or even a data emulator) stop sending data, my application continues to receive data for a period of time that is proportional to how long the read function has been running for (EDIT: This excess period is in the same ballpark as the period of time the data is sent for). So if I start and stop the data flow immediately, this would be reflected on my GUI, but if I start data flow and stop it ten seconds later, my GUI continues to show data for 10 seconds more (EDITED).

I have made the following observations after exhausting all my attempts at debugging:

1. As mentioned above, this excess period of operation is proportional to how long the hardware has been sending data.
2. The frequency of incoming data is 50ms, so to receive 10 seconds worth of data, my GUI must be receiving around 200 more data packets.
3. The only buffer I have declared is abBuffer which is just a byte array of fixed size. I don't think this can increase in size, so this data is being stored somewhere.
4. If I change something in the data packet, this change, understandably, is shown on the GUI after a delay (because of the above points). But this would imply that the data received at the COM port is stored in some variable sized buffer from which my read function is reading data.
5. I have timed the read and processing periods. The latter is instantaneous while the former very rarely (3 times in 1000 reads (following no discernible pattern)) takes 16ms. This is well within the 50ms window the GUI has for each read.

The following is my thread and timer code:

UINT CMyCOMDlg::StartThread(LPVOID param)
{
    THREADSTRUCT *ts = (THREADSTRUCT*)param;

    ts->_this->SetTimer(1,50,0);

    return 0;

}

//Timer function that is called at regular intervals
void CMyCOMDlg::OnTimer(UINT_PTR nIDEvent)
{       

    if(m_bCount==true)
    {
        DWORD NoBytesRead;
        BYTE  abBuffer[45];

        if(ReadFile((m_hComm),&abBuffer,45,&NoBytesRead,0))
        {
            if(NoBytesRead==45)
            {
                if(abBuffer[0]==0x10&&abBuffer[1]==0x10||abBuffer[0]==0x80&&abBuffer[1]==0x80)
                {
                    fnSetData(abBuffer);
                }
                else
                {
                    CString value;                      
                    value.Append("Header match failed");
                    SetDlgItemText(IDC_RXRAW,value);    
                }
            }
            else
            {
                CString value;
                value.Append(LPCTSTR(abBuffer),NoBytesRead);
                value.Append("\r\nInvalid Packet Size");
                SetDlgItemText(IDC_RXRAW,value);
            }
        }
        else
        {
            DWORD dwError2 = GetLastError();
            CString error2;
            error2.Format(_T("%d"),dwError2);
            SetDlgItemText(IDC_RXRAW,error2);
        }

        fnClear();
    }
    else
    {
        KillTimer(1);
    }

    CDialog::OnTimer(nIDEvent);

}

m_bCount is just a flag I use to kill the timer and the ReadFile function is a standard Windows API call. ts is a structure that contains a pointer to the main dialog class, i.e., this.

Can anyone think of a reason this could be happening? I have tried a lot of things, and also my code does so little I cannot figure out where this unexpected behaviour is happening.

EDIT:

I am adding the COM port settings and timeouts used below :

            dcb.BaudRate = CBR_115200;
            dcb.ByteSize = 8;
            dcb.StopBits = ONESTOPBIT;
            dcb.Parity = NOPARITY;
            SetCommState(m_hComm, &dcb);

            _param->_this=this;

            COMMTIMEOUTS timeouts;
            timeouts.ReadIntervalTimeout=1;
            timeouts.ReadTotalTimeoutMultiplier = 0;
            timeouts.ReadTotalTimeoutConstant = 10;
            timeouts.WriteTotalTimeoutMultiplier = 1;
            timeouts.WriteTotalTimeoutConstant = 1;
            SetCommTimeouts(m_hComm, &timeouts);

Answer 1

You are processing one message at a time in the OnTimer() function. Since the timer interval is 1 second but the data source keeps sending message every 50 milliseconds, your application cannot process all messages in the timely manner.

You can add while loop as follow:

while(true)
{
    if(::ReadFile(m_hComm, &abBuffer, sizeof(abBuffer), &NoBytesRead, 0))
    {
        if(NoBytesRead == sizeof(abBuffer))
        {
            ...
        }
        else
        {
            ...
            break;
        }
    }
    else
    {
        ...
        break;
    }
}

But there is another problem in your code. If your software checks the message while the data source is still sending the message, NoBytesRead could be less than 45. You may want to store the data into the message buffer like CString or std::queue<unsigned char>.

If the message doesn't contain a NULL at the end of the message, passing the message to the CString object is not safe.

Also if the first byte starts at 0x80, CString will treat it as a multi-byte string. It may cause the error. If the message is not a literal text string, consider using other data format like std::vector<unsigned char>.

By the way, you don't need to call SetTimer() in the separate thread. It doesn't take time to kick a timer. Also I recommend you to call KillTimer() somewhere outside of the OnTimer() function so that the code will be more intuitive.

If the data source continuously keeps sending data, you may need to use PurgeComm() when you open/close the COMM port.