Ужасный код последовательного порта/USB (C++) - предложения по исправлению?
У меня нет большого опыта работы с последовательным вводом-выводом, но недавно мне поручили исправить код последовательного ввода-вывода с большим количеством ошибок, потому что первоначальный программист покинул компанию.
Приложение представляет собой программу для Windows, которая последовательно взаимодействует с научным прибором через виртуальный COMM-порт, работающий на USB. USB-драйверы виртуального COMM-порта предоставляются FTDI, поскольку они производят USB-микросхему, которую мы используем в приборе.
Последовательный код находится в неуправляемой C++ DLL, которая используется как нашим старым программным обеспечением C++, так и нашим новым программным обеспечением C#/.Net (WinForms).
Существуют две основные проблемы:
Сбой на многих системах XP.
При отправке первой команды на прибор нет ответа.Когда вы вводите следующую команду, вы получаете ответ от первой.
Вот типичный сценарий использования (полный исходный код вызываемых методов приведен ниже):
char szBuf [256];
CloseConnection ();
if (OpenConnection ())
{
ClearBuffer ();
// try to get a firmware version number
WriteChar ((char) 'V');
BOOL versionReadStatus1 = ReadString (szBuf, 100);
...
}
В неисправной системе вызов ReadString никогда не получит никаких последовательных данных и истекает по тайм-ауту. Но если мы введем другую, другую команду и снова вызовем ReadString, она вернет ответ из команды first, а не новый!
Но это происходит только на большом подмножестве систем Windows XP и никогда на Windows 7. К счастью, наши машины для разработки XP работали нормально, поэтому мы не заметили проблемы, пока не начали бета-тестирование. Но я также могу воспроизвести проблему, запустив виртуальную машину XP (VirtualBox) на моей машине XP dev. Кроме того, проблема возникает только при использовании DLL с новой версией C# — отлично работает со старым приложением C++.
Это казалось решенным, когда я добавил Sleep(21) к низкоуровневому методу BytesInQue перед вызовом ClearCommError, но это усугубило другую проблему — использование ЦП. Спящий режим менее 21 мс приведет к повторному появлению режима сбоя.
Высокая загрузка ЦП
При последовательном вводе-выводе ЦП используется чрезмерно — часто выше 90 %. Это происходит как с новым приложением C#, так и со старым приложением C++, но в новом приложении все гораздо хуже. Часто делает пользовательский интерфейс очень невосприимчивым, но не всегда.
Вот код нашего класса Port.cpp во всей его ужасной красе. Извините за длину, но это то, с чем я работаю. Вероятно, наиболее важными методами являются OpenConnection, ReadString, ReadChar и BytesInQue.
//
// Port.cpp: Implements the CPort class, which is
// the class that controls the serial port.
//
// Copyright (C) 1997-1998 Microsoft Corporation
// All rights reserved.
//
// This source code is only intended as a supplement to the
// Broadcast Architecture Programmer's Reference.
// For detailed information regarding Broadcast
// Architecture, see the reference.
//
#include <windows.h>
#include <stdio.h>
#include <assert.h>
#include "port.h"
// Construction code to initialize the port handle to null.
CPort::CPort()
{
m_hDevice = (HANDLE)0;
// default parameters
m_uPort = 1;
m_uBaud = 9600;
m_uDataBits = 8;
m_uParity = 0;
m_uStopBits = 0; // = 1 stop bit
m_chTerminator = '\n';
m_bCommportOpen = FALSE;
m_nTimeOut = 50;
m_nBlockSizeMax = 2048;
}
// Destruction code to close the connection if the port
// handle was valid.
CPort::~CPort()
{
if (m_hDevice)
CloseConnection();
}
// Open a serial communication port for writing short
// one-byte commands, that is, overlapped data transfer
// is not necessary.
BOOL CPort::OpenConnection()
{
char szPort[64];
m_bCommportOpen = FALSE;
// Build the COM port string as "COMx" where x is the port.
if (m_uPort > 9)
wsprintf(szPort, "\\\\.\\COM%d", m_uPort);
else
wsprintf(szPort, "COM%d", m_uPort);
// Open the serial port device.
m_hDevice = CreateFile(szPort,
GENERIC_WRITE | GENERIC_READ,
0,
NULL, // No security attributes
OPEN_EXISTING,
FILE_ATTRIBUTE_NORMAL,
NULL);
if (m_hDevice == INVALID_HANDLE_VALUE)
{
SaveLastError ();
m_hDevice = (HANDLE)0;
return FALSE;
}
return SetupConnection(); // After the port is open, set it up.
} // end of OpenConnection()
// Configure the serial port with the given settings.
// The given settings enable the port to communicate
// with the remote control.
BOOL CPort::SetupConnection(void)
{
DCB dcb; // The DCB structure differs betwwen Win16 and Win32.
dcb.DCBlength = sizeof(DCB);
// Retrieve the DCB of the serial port.
BOOL bStatus = GetCommState(m_hDevice, (LPDCB)&dcb);
if (bStatus == 0)
{
SaveLastError ();
return FALSE;
}
// Assign the values that enable the port to communicate.
dcb.BaudRate = m_uBaud; // Baud rate
dcb.ByteSize = m_uDataBits; // Data bits per byte, 4-8
dcb.Parity = m_uParity; // Parity: 0-4 = no, odd, even, mark, space
dcb.StopBits = m_uStopBits; // 0,1,2 = 1, 1.5, 2
dcb.fBinary = TRUE; // Binary mode, no EOF check : Must use binary mode in NT
dcb.fParity = dcb.Parity == 0 ? FALSE : TRUE; // Enable parity checking
dcb.fOutX = FALSE; // XON/XOFF flow control used
dcb.fInX = FALSE; // XON/XOFF flow control used
dcb.fNull = FALSE; // Disable null stripping - want nulls
dcb.fOutxCtsFlow = FALSE;
dcb.fOutxDsrFlow = FALSE;
dcb.fDsrSensitivity = FALSE;
dcb.fDtrControl = DTR_CONTROL_ENABLE;
dcb.fRtsControl = RTS_CONTROL_DISABLE ;
// Configure the serial port with the assigned settings.
// Return TRUE if the SetCommState call was not equal to zero.
bStatus = SetCommState(m_hDevice, &dcb);
if (bStatus == 0)
{
SaveLastError ();
return FALSE;
}
DWORD dwSize;
COMMPROP *commprop;
DWORD dwError;
dwSize = sizeof(COMMPROP) + sizeof(MODEMDEVCAPS) ;
commprop = (COMMPROP *)malloc(dwSize);
memset(commprop, 0, dwSize);
if (!GetCommProperties(m_hDevice, commprop))
{
dwError = GetLastError();
}
m_bCommportOpen = TRUE;
return TRUE;
}
void CPort::SaveLastError ()
{
DWORD dwLastError = GetLastError ();
LPVOID lpMsgBuf;
FormatMessage(FORMAT_MESSAGE_ALLOCATE_BUFFER |
FORMAT_MESSAGE_FROM_SYSTEM |
FORMAT_MESSAGE_IGNORE_INSERTS,
NULL,
dwLastError,
MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT), // Default language
(LPTSTR) &lpMsgBuf,
0,
NULL);
strcpy (m_szLastError,(LPTSTR)lpMsgBuf);
// Free the buffer.
LocalFree( lpMsgBuf );
}
void CPort::SetTimeOut (int nTimeOut)
{
m_nTimeOut = nTimeOut;
}
// Close the opened serial communication port.
void CPort::CloseConnection(void)
{
if (m_hDevice != NULL &&
m_hDevice != INVALID_HANDLE_VALUE)
{
FlushFileBuffers(m_hDevice);
CloseHandle(m_hDevice); ///that the port has been closed.
}
m_hDevice = (HANDLE)0;
// Set the device handle to NULL to confirm
m_bCommportOpen = FALSE;
}
int CPort::WriteChars(char * psz)
{
int nCharWritten = 0;
while (*psz)
{
nCharWritten +=WriteChar(*psz);
psz++;
}
return nCharWritten;
}
// Write a one-byte value (char) to the serial port.
int CPort::WriteChar(char c)
{
DWORD dwBytesInOutQue = BytesInOutQue ();
if (dwBytesInOutQue > m_dwLargestBytesInOutQue)
m_dwLargestBytesInOutQue = dwBytesInOutQue;
static char szBuf[2];
szBuf[0] = c;
szBuf[1] = '\0';
DWORD dwBytesWritten;
DWORD dwTimeOut = m_nTimeOut; // 500 milli seconds
DWORD start, now;
start = GetTickCount();
do
{
now = GetTickCount();
if ((now - start) > dwTimeOut )
{
strcpy (m_szLastError, "Timed Out");
return 0;
}
WriteFile(m_hDevice, szBuf, 1, &dwBytesWritten, NULL);
}
while (dwBytesWritten == 0);
OutputDebugString(TEXT(strcat(szBuf, "\r\n")));
return dwBytesWritten;
}
int CPort::WriteChars(char * psz, int n)
{
DWORD dwBytesWritten;
WriteFile(m_hDevice, psz, n, &dwBytesWritten, NULL);
return dwBytesWritten;
}
// Return number of bytes in RX queue
DWORD CPort::BytesInQue ()
{
COMSTAT ComStat ;
DWORD dwErrorFlags;
DWORD dwLength;
// check number of bytes in queue
ClearCommError(m_hDevice, &dwErrorFlags, &ComStat ) ;
dwLength = ComStat.cbInQue;
return dwLength;
}
DWORD CPort::BytesInOutQue ()
{
COMSTAT ComStat ;
DWORD dwErrorFlags;
DWORD dwLength;
// check number of bytes in queue
ClearCommError(m_hDevice, &dwErrorFlags, &ComStat );
dwLength = ComStat.cbOutQue ;
return dwLength;
}
int CPort::ReadChars (char* szBuf, int nMaxChars)
{
if (BytesInQue () == 0)
return 0;
DWORD dwBytesRead;
ReadFile(m_hDevice, szBuf, nMaxChars, &dwBytesRead, NULL);
return (dwBytesRead);
}
// Read a one-byte value (char) from the serial port.
int CPort::ReadChar (char& c)
{
static char szBuf[2];
szBuf[0] = '\0';
szBuf[1] = '\0';
if (BytesInQue () == 0)
return 0;
DWORD dwBytesRead;
ReadFile(m_hDevice, szBuf, 1, &dwBytesRead, NULL);
c = *szBuf;
if (dwBytesRead == 0)
return 0;
return dwBytesRead;
}
BOOL CPort::ReadString (char *szStrBuf , int nMaxLength)
{
char str [256];
char str2 [256];
DWORD dwTimeOut = m_nTimeOut;
DWORD start, now;
int nBytesRead;
int nTotalBytesRead = 0;
char c = ' ';
static char szCharBuf [2];
szCharBuf [0]= '\0';
szCharBuf [1]= '\0';
szStrBuf [0] = '\0';
start = GetTickCount();
while (c != m_chTerminator)
{
nBytesRead = ReadChar (c);
nTotalBytesRead += nBytesRead;
if (nBytesRead == 1 && c != '\r' && c != '\n')
{
*szCharBuf = c;
strncat (szStrBuf,szCharBuf,1);
if (strlen (szStrBuf) == nMaxLength)
return TRUE;
// restart timer for next char
start = GetTickCount();
}
// check for time out
now = GetTickCount();
if ((now - start) > dwTimeOut )
{
strcpy (m_szLastError, "Timed Out");
return FALSE;
}
}
return TRUE;
}
int CPort::WaitForQueToFill (int nBytesToWaitFor)
{
DWORD start = GetTickCount();
do
{
if (BytesInQue () >= nBytesToWaitFor)
break;
if (GetTickCount() - start > m_nTimeOut)
return 0;
} while (1);
return BytesInQue ();
}
int CPort::BlockRead (char * pcInputBuffer, int nBytesToRead)
{
int nBytesRead = 0;
int charactersRead;
while (nBytesToRead >= m_nBlockSizeMax)
{
if (WaitForQueToFill (m_nBlockSizeMax) < m_nBlockSizeMax)
return nBytesRead;
charactersRead = ReadChars (pcInputBuffer, m_nBlockSizeMax);
pcInputBuffer += charactersRead;
nBytesRead += charactersRead;
nBytesToRead -= charactersRead;
}
if (nBytesToRead > 0)
{
if (WaitForQueToFill (nBytesToRead) < nBytesToRead)
return nBytesRead;
charactersRead = ReadChars (pcInputBuffer, nBytesToRead);
nBytesRead += charactersRead;
nBytesToRead -= charactersRead;
}
return nBytesRead;
}
Основываясь на моем тестировании и прочитанном, я вижу несколько подозрительных вещей в этом коде:
COMMTIMEOUTS никогда не устанавливается. В документах MS говорится: «Если вы не установите значения тайм-аута, могут возникнуть непредсказуемые результаты». Но я пытался установить это, и это не помогло.
Многие методы (например, ReadString) зацикливаются и забивают порт повторными операциями чтения, если они не получают данные немедленно. Кажется, это объясняет высокую загрузку процессора.
Многие методы имеют собственную обработку времени ожидания с помощью GetTickCount(). Разве не для этого предназначен COMMTIMEOUTS?
В новой программе C# (WinForms) все эти последовательные подпрограммы вызываются непосредственно из основного потока из события MultiMediaTimer. Может стоит запустить в другом потоке?
Метод BytesInQue кажется узким местом. Если я перехожу к отладчику при высокой загрузке ЦП, обычно программа останавливается именно на этом. Кроме того, добавление Sleep(21) к этому методу перед вызовом ClearCommError, по-видимому, решает проблему XP, но усугубляет проблему использования ЦП.
Код кажется излишне сложным.
Мои вопросы
Кто-нибудь может объяснить, почему это работает только с программой C# на небольшом количестве систем XP?
Есть предложения, как это переписать? Приветствуются указатели на хороший пример кода.