5

私はシリアル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;
}

私のテストと読み取りに基づいて、このコードにはいくつかの疑わしいことがあります。

  1. COMMTIMEOUTSが設定されることはありません。MSのドキュメントには、「タイムアウト値の設定に失敗すると、予期しない結果が発生する可能性がある」と記載されています。しかし、これを設定してみましたが、役に立ちませんでした。

  2. 多くのメソッド(ReadStringなど)は、データをすぐに取得しない場合、タイトなループに入り、繰り返しの読み取りでポートをハンマーで叩きます。これは、CPU使用率が高いことを説明しているようです。

  3. 多くのメソッドには、GetTickCount()を使用した独自のタイムアウト処理があります。それがCOMMTIMEOUTSの目的ではありませんか?

  4. 新しいC#(WinForms)プログラムでは、これらのシリアルルーチンはすべて、MultiMediaTimerイベントからメインスレッドから直接呼び出されます。たぶん、別のスレッドで実行する必要がありますか?

  5. BytesInQueメソッドがボトルネックのようです。CPU使用率が高いときにデバッガーにアクセスすると、通常、プログラムが停止します。また、ClearCommErrorを呼び出す前にこのメソッドにSleep(21)を追加すると、XPの問題は解決するようですが、CPU使用率の問題が悪化します。

  6. コードは不必要に複雑に見えます。

私の質問

  1. これが少数のXPシステムのC#プログラムでのみ機能する理由を誰かが説明できますか?

  2. これを書き直す方法について何か提案はありますか?良いサンプルコードへのポインタは大歓迎です。

4

3 に答える 3

7

そのクラスにはいくつかの深刻な問題があり、Microsoftの著作権があることで事態はさらに悪化します。

このクラスには特別なことは何もありません。そして、Create / Read / WriteFile上のアダプタとして以外に、なぜそれが存在するのか不思議に思います。.NET FrameworkでSerialPortクラスを使用した場合は、このクラスも必要ありません。

CPU使用率は、デバイスが十分なデータを利用できるようになるのを待っている間、コードが無限ループに入るためです。while(1);Win32とC++を使い続ける必要がある場合は、CreateFileを呼び出すときに、完了ポートを調べてOVERLAPPEDフラグを設定することができます。このようにして、別のワーカースレッドでデータを待つことができます。

複数のCOMポートと通信する場合は注意が必要です。C ++を実行してから長い時間が経ちましたが、ReadメソッドとWriteメソッドの静的バッファーszBuffは、そのクラスのすべてのインスタンスに対して静的であると思います。これは、2つの異なるCOMポートに対して「同時に」Readを呼び出すと、予期しない結果が生じることを意味します。

一部のXPマシンの問題については、読み取り/書き込みのたびにGetLastErrorをチェックして結果をログに記録すると、問題を確実に把握できます。とにかくGetLastErrorをチェックする必要があります。これは、常に「エラー」であるとは限らず、必要な結果を得るためにサブシステムから何か他のことを行うように要求する場合があるためです。

COMMTIMEOUTS正しく設定すれば、ブロックのためにwhileループ全体を取り除くことができます。読み取りを実行する前に、Read操作の使用に特定のタイムアウトがある場合。SetCommTimeouts

ReadIntervalTimeout読み取りがm_nTimeOutより速く戻らないように、最大​​タイムアウトに設定しました。この値により、任意の2バイトの間に時間が経過すると、読み取りが返されます。2ミリ秒に設定され、最初のバイトがtに到着し、2番目がt + 1に到着し、3番目がt + 4に到着した場合、バイト間の間隔を超えたため、ReadFileは最初の2バイトのみを返します。 。ReadTotalTimeoutConstantは、何があってもm_nTimeOutより長く待機しないことを保証します。

maxWait = BytesToRead * ReadTotalTimeoutMultiplier + ReadTotalTimeoutConstant。したがって(BytesToRead * 0) + m_nTimeout = m_nTimeout

BOOL CPort::SetupConnection(void) 
{
      // Snip...
      COMMTIMEOUTS comTimeOut;                   
      comTimeOut.ReadIntervalTimeout = m_nTimeOut; // Ensure's we wait the max timeout
      comTimeOut.ReadTotalTimeoutMultiplier = 0;
      comTimeOut.ReadTotalTimeoutConstant = m_nTimeOut;
      comTimeOut.WriteTotalTimeoutMultiplier = 0;
      comTimeOut.WriteTotalTimeoutConstant = m_nTimeOut;
      SetCommTimeouts(m_hDevice,&comTimeOut);
}

// If return value != nBytesToRead check check GetLastError()
// Most likely Read timed out.
int CPort::BlockRead (char * pcInputBuffer, int nBytesToRead)
{
      DWORD dwBytesRead; 
      if (FALSE == ReadFile(
             m_hDevice, 
             pcInputBuffer, 
             nBytesToRead, 
             &dwBytesRead, 
             NULL))
      {
           // Check GetLastError
           return dwBytesRead;
      }
      return dwBytesRead;
}

これが完全に正しいかどうかはわかりませんが、それはあなたにアイデアを与えるはずです。ReadCharメソッドとReadStringメソッドを削除し、プログラムが同期に依存している場合はこれを使用します。高いタイムアウトの設定にも注意してください。通信はミリ秒単位で高速です。

于 2012-04-04T23:32:09.857 に答える
1

これが私が何年も前に書いたターミナルプログラムです(おそらく少なくとも15年前、今私はそれについて考えています)。簡単なチェックを行ったところ、Windows 7 x64でも十分に機能しているようです。つまり、GPSに接続し、そこからのデータを読み取って表示します。

コードを見ると、通信タイムアウト値の選択にあまり時間をかけていなかったことがわかります。CPU使用率が許容できるようになるまで、より長いタイムアウトを試すことを意図して、それらをすべて1に設定しました。簡単に言うと、これまで気にしたことのないCPU時間をほとんど使用していません。たとえば、タスクマネージャーのCPU使用率グラフでは、実行中と実行中の違いはわかりません。一度に数時間GPSからデータを収集して実行したままにしましたが、タスクマネージャーはまだCPU使用率の合計が0:00:00であると言います。

結論:もっと効率的になると確信していますが、十分な場合もあります。私がこれ以上あまり使用しないこと、そしてファイル転送プロトコルのようなものを追加する可能性を考えると、それをより効率的にすることはおそらくやるべきことの山の頂点に達することはないでしょう。

#include <stdio.h>
#include <conio.h>
#include <string.h>

#define STRICT
#define WIN32_LEAN_AND_MEAN
#include <windows.h>

void system_error(char *name) {
// Retrieve, format, and print out a message from the last error.  The 
// `name' that's passed should be in the form of a present tense noun 
// (phrase) such as "opening file".
//
    char *ptr = NULL;
    FormatMessage(
        FORMAT_MESSAGE_ALLOCATE_BUFFER |
        FORMAT_MESSAGE_FROM_SYSTEM,
        0,
        GetLastError(),
        0,
        (char *)&ptr,
        1024,
        NULL);

    fprintf(stderr, "\nError %s: %s\n", name, ptr);
    LocalFree(ptr);
}

int main(int argc, char **argv) {

    int ch;
    char buffer[64];
    HANDLE file;
    COMMTIMEOUTS timeouts;
    DWORD read, written;
    DCB port;
    HANDLE keyboard = GetStdHandle(STD_INPUT_HANDLE);
    HANDLE screen = GetStdHandle(STD_OUTPUT_HANDLE);
    DWORD mode;
    char port_name[128] = "\\\\.\\COM3";
    char init[] = "";

    if ( argc > 2 )
        sprintf(port_name, "\\\\.\\COM%s", argv[1]);

    // open the comm port.
    file = CreateFile(port_name,
        GENERIC_READ | GENERIC_WRITE,
        0, 
        NULL, 
        OPEN_EXISTING,
        0,
        NULL);

    if ( INVALID_HANDLE_VALUE == file) {
        system_error("opening file");
        return 1;
    }

    // get the current DCB, and adjust a few bits to our liking.
    memset(&port, 0, sizeof(port));
    port.DCBlength = sizeof(port);
    if (!GetCommState(file, &port))
        system_error("getting comm state");
    if (!BuildCommDCB("baud=19200 parity=n data=8 stop=1", &port))
        system_error("building comm DCB");
    if (!SetCommState(file, &port))
        system_error("adjusting port settings");

    // set short timeouts on the comm port.
    timeouts.ReadIntervalTimeout = 1;
    timeouts.ReadTotalTimeoutMultiplier = 1;
    timeouts.ReadTotalTimeoutConstant = 1;
    timeouts.WriteTotalTimeoutMultiplier = 1;
    timeouts.WriteTotalTimeoutConstant = 1;
    if (!SetCommTimeouts(file, &timeouts))
        system_error("setting port time-outs.");

    // set keyboard to raw reading.
    if (!GetConsoleMode(keyboard, &mode))
        system_error("getting keyboard mode");
    mode &= ~ ENABLE_PROCESSED_INPUT;
    if (!SetConsoleMode(keyboard, mode))
        system_error("setting keyboard mode");

    if (!EscapeCommFunction(file, CLRDTR))
        system_error("clearing DTR");
    Sleep(200);
    if (!EscapeCommFunction(file, SETDTR))
        system_error("setting DTR");

    if (!WriteFile(file, init, sizeof(init), &written, NULL))
        system_error("writing data to port");

    if (written != sizeof(init))
        system_error("not all data written to port");

    // basic terminal loop:
    do {
        // check for data on port and display it on screen.
        ReadFile(file, buffer, sizeof(buffer), &read, NULL);
        if (read)
            WriteFile(screen, buffer, read, &written, NULL);

        // check for keypress, and write any out the port.
        if ( kbhit() ) {
            ch = getch();
            WriteFile(file, &ch, 1, &written, NULL);
        }
    // until user hits ctrl-backspace.
    } while ( ch != 127);

    // close up and go home.
    CloseHandle(keyboard);
    CloseHandle(file);
    return 0;
}
于 2012-04-05T01:27:11.363 に答える
0

追加します

Sleep(2);

CPort :: WaitForQueToFill()のwhileループへ

これにより、OSは実際にいくつかのバイトをキューに入れることができます。

于 2012-04-05T00:32:52.887 に答える