c++ - 糟糕的串行端口/USB 代码 (C++) - 修复建议？

Question

我在串行 I/O 方面没有太多经验，但最近接到任务修复一些有严重缺陷的串行代码，因为原来的程序员已经离开公司。

该应用程序是一个 Windows 程序，可通过 USB 上运行的虚拟 COMM 端口与科学仪器串行通信。虚拟 COMM 端口 USB 驱动程序由 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，它将返回来自第一个命令的响应，而不是新命令!

但这只发生在大部分 Windows XP 系统上，而从未发生在 Windows 7 上。幸运的是，我们的 XP 开发机器运行正常，所以在开始 Beta 测试之前我们没有发现问题。但我也可以通过在我的 XP 开发机器上运行 XP VM (VirtualBox) 来重现该问题。此外，仅当将 DLL 与新的 C# 版本一起使用时才会出现此问题 - 与旧的 C++ 应用程序一起使用时效果很好。

当我在调用 ClearCommError 之前将 Sleep(21) 添加到低级 BytesInQue 方法时，这似乎得到了解决，但这加剧了另一个问题 - 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. 关于如何重写它有什么建议吗？非常欢迎指向好的示例代码的指针。

Answer 1

那个类有一些严重的问题，而且它有微软的版权，这让事情变得更糟。

这个类没有什么特别的。这让我想知道为什么它除了作为 Create/Read/WriteFile 的适配器之外还存在。如果您使用 SerialPort，您甚至不需要此类.NET Framework 中的类。

您的 CPU 使用率是因为代码在等待设备有足够的可用数据时进入无限循环。代码也可能是 while(1); 如果您必须坚持使用 Win32 和 C++，您可以查看 Completion Ports 并在调用 CreateFile 时设置 OVERLAPPED 标志。这样您就可以在单独的工作线程中等待数据。

与多个 COM 端口通信时需要小心。自从我完成 C++ 以来已经有很长时间了，但我相信 Read 和 Write 方法中的静态缓冲区 szBuff 对于该类的所有实例都是静态的。这意味着如果您“同时”对两个不同的 COM 端口调用 Read，您将得到意想不到的结果。

至于某些 XP 机器上的问题，如果您在每次读/写后检查 GetLastError 并记录结果，您肯定会找出问题所在。它无论如何都应该检查 GetLastError，因为它有时并不总是一个“错误”，而是来自子系统的请求，要求执行其他操作以获得您想要的结果。

如果正确设置 COMMTIMEOUTS，您可以摆脱整个 while 循环的阻塞。如果 Read 操作有特定超时，请在执行读取之前使用 SetCommTimeouts。

我将 ReadIntervalTimeout 设置为最大超时以确保读取不会比 m_nTimeOut 更快返回。如果时间在任意两个字节之间过去，此值将导致 Read 返回。如果将其设置为 2 毫秒并且第一个字节在 t 时进入，第二个字节在 t+1 时进入，第三个字节在 t+4 时进入，ReadFile 将只返回前两个字节，因为字节之间的间隔被超过. 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 方法，如果您的程序依赖于同步事物，则使用它。也要小心设置高超时。通信速度很快，以毫秒为单位。