视觉可怕的串行端口/USB代码(C++)-修复建议

visual Terrible Serial Port / USB code (C++) - suggestions for fixes?

本文关键字:C++ 代码 串行端口 USB 视觉      更新时间:2023-10-16

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

该应用程序是一个Windows程序,通过USB上运行的虚拟COMM端口与科学仪器串行通信。虚拟COMM端口USB驱动程序由FTDI提供,因为它们制造了我们在仪器上使用的USB芯片。

串行代码位于非托管的C++DLL中,该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开发机器工作正常,所以我们直到开始测试才发现问题。但我也可以通过在我的XP开发机器上运行XP虚拟机(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] = ''; 

    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, "rn")));
    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] = ''; 
    szBuf[1] = ''; 
    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]= '';
    szCharBuf [1]= '';
    szStrBuf [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)程序中,所有这些串行例程都是从主线程、多媒体定时器事件直接调用的。也许应该在另一个线程中运行?

  5. BytesInQue方法似乎是一个瓶颈。如果我在CPU使用率很高的时候中断到调试器,程序通常会在那里停止。此外,在调用ClearCommError之前向该方法添加Sleep(21)似乎可以解决XP问题,但会加剧CPU使用问题。

  6. 代码似乎过于复杂。

我的问题

  1. 有人能解释为什么这只适用于少数XP系统上的C#程序吗?

  2. 有什么关于如何重写的建议吗?指向好的示例代码的指针将是最受欢迎的。

该类存在一些严重问题,而且它有Microsoft版权,这让情况变得更糟。

这个班没有什么特别的。这让我想知道为什么它除了作为创建/读取/写入文件的适配器之外还存在。如果您在.NETFramework中使用SerialPort类,那么您甚至不需要这个类。

您的CPU使用率是因为在等待设备有足够的可用数据时,代码会进入无限循环。代码可能会说while(1);如果必须坚持使用Win32和C++,则可以在调用CreateFile时查看完成端口并设置OVERLAPPED标志。通过这种方式,您可以在单独的工作线程中等待数据。

与多个COM端口通信时需要小心。我已经很久没有做C++了,但我相信Read和Write方法中的静态缓冲区szBuff对于该类的所有实例都是静态的。这意味着,如果您"同时"对两个不同的COM端口调用Read,您将得到意外的结果。

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

COMMTIMEOUTS设置正确,就可以去掉整个while循环以进行阻塞。如果Read操作有特定超时,请在执行读取之前使用SetCommTimeouts

我将ReadIntervalTimeout设置为最大超时,以确保Read不会比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方法,并在程序依赖于同步的情况下使用它们。设置高超时也要小心。通信速度很快,只需几毫秒。

这是我几年前写的一个终端程序(可能至少15年前,现在我想起来了)。我只是做了一个快速检查,在Windows7x64下,它似乎仍然运行得相当好——连接到我的GPS,读取并显示来自它的数据。

如果你看一下代码,你会发现我没有花太多时间选择comm超时值。我将它们都设置为1,打算尝试更长的超时时间,直到CPU使用率可以容忍为止。长话短说,它占用的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: %sn", 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;
}

我会添加

Sleep(2);

到CPort::WaitForQueToFill()中的while循环

这将使操作系统有机会在队列中实际放置一些字节。