私はシリアルI/Oの経験はあまりありませんが、元のプログラマーが会社を辞めたため、最近、いくつかの非常に欠陥のあるシリアルコードの修正を任されました。
このアプリケーションは、USB上で実行されている仮想COMMポートを介して科学機器とシリアルに通信するWindowsプログラムです。仮想COMMポートのUSBドライバーは、機器で使用するUSBチップを製造しているため、FTDIによって提供されます。
シリアルコードは、管理されていないC ++ DLLに含まれています。これは、古いC ++ソフトウェアと新しいC#/ .Net(WinForms)ソフトウェアの両方で共有されています。
2つの主な問題があります:
多くの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を再度呼び出すと、新しいコマンドではなく、最初のコマンドからの応答が返されます。
ただし、これはWindows XPシステムの大規模なサブセットでのみ発生します。Windows7では発生しません。運が良ければ、XP開発マシンは正常に動作したため、ベータテストを開始するまで問題は発生しませんでした。ただし、XP開発マシンでXP VM(VirtualBox)を実行することで、問題を再現することもできます。また、この問題は、DLLを新しいC#バージョンで使用する場合にのみ発生します-古いC++アプリで正常に動作します。
これは、ClearCommErrorを呼び出す前に低レベルのBytesInQueメソッドにSleep(21)を追加したときに解決されたようですが、これは他の問題であるCPU使用率を悪化させました。21ミリ秒未満のスリープでは、障害モードが再び表示されます。
高いCPU使用率
シリアルI/Oを実行する場合、CPUの使用は過剰になります(多くの場合、90%を超えます)。これは、新しいC#アプリと古いC ++アプリの両方で発生しますが、新しいアプリではさらに悪化します。多くの場合、UIが非常に応答しなくなりますが、常にそうとは限りません。
これが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など)は、データをすぐに取得しない場合、タイトなループに入り、繰り返しの読み取りでポートをハンマーで叩きます。これは、CPU使用率が高いことを説明しているようです。
多くのメソッドには、GetTickCount()を使用した独自のタイムアウト処理があります。それがCOMMTIMEOUTSの目的ではありませんか?
新しいC#(WinForms)プログラムでは、これらのシリアルルーチンはすべて、MultiMediaTimerイベントからメインスレッドから直接呼び出されます。たぶん、別のスレッドで実行する必要がありますか?
BytesInQueメソッドがボトルネックのようです。CPU使用率が高いときにデバッガーにアクセスすると、通常、プログラムが停止します。また、ClearCommErrorを呼び出す前にこのメソッドにSleep(21)を追加すると、XPの問題は解決するようですが、CPU使用率の問題が悪化します。
コードは不必要に複雑に見えます。
私の質問
これが少数のXPシステムのC#プログラムでのみ機能する理由を誰かが説明できますか?
これを書き直す方法について何か提案はありますか?良いサンプルコードへのポインタは大歓迎です。