win32 xbee_ser_open broken (needs to use wchar for port name)

[pcdangio]

On windows 10 + mingw, xbee_ser_open() was failing with IO errors (123 and 5). I debugged a little bit and it turns out that a wchar array must be used when getting the hCom handle. The error can be traced to the following lines:

https://github.com/digidotcom/xbee_ansic_library/blob/64261a71fef428029b4631e53b5c22bf0d1cf1fc/ports/win32/xbee_serial_win32.c#L298-L299

Here, buffer is a regular char and not a wchar_t (aka uint16_t). This formatting issue results in a failure to obtain a valid hCom handle no matter what COM port is used.

I've implemented a working fix that uses a wchar_t buffer and swprintf(). Tried to set up a pull request but didn't have branch access so I've included the fixed xbee_ser_open() function below. The only places I changed anything were setting up the buffer at the beginning of the function, and switching to swprintf() just before the hCom handle is opened with CreateFile().

int xbee_ser_open( xbee_serial_t *serial, uint32_t baudrate)
{
    const int       buffer_size = sizeof(L"COM9999");
    wchar_t         buffer[buffer_size];
    HANDLE          hCom;
    COMMTIMEOUTS    timeouts;
    int             err;

    if (serial == NULL || serial->comport > 9999)
    {
        #ifdef XBEE_SERIAL_VERBOSE
            printf( "%s: -EINVAL (serial=%p)\n", __FUNCTION__, serial);
        #endif
        return -EINVAL;
    }

    // if XBee's existing hCom handle is not null, need to close it
    // (unless we're just changing the baud rate?)

    // Assume serial port has not changed if port is already open, and skip the
    // CreateFile step.

    if (serial->hCom)
    {
        // serial port is already open
        hCom = serial->hCom;
        serial->hCom = NULL;
        #ifdef XBEE_SERIAL_VERBOSE
            printf( "%s: port already open (hCom=%p)\n",
                __FUNCTION__, hCom);
        #endif
    }
    else
    {
        swprintf(buffer, buffer_size, L"COM%u", serial->comport);
        hCom = CreateFile(buffer, GENERIC_READ | GENERIC_WRITE,
            0, NULL, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL);
        if (hCom == INVALID_HANDLE_VALUE)
        {
            #ifdef XBEE_SERIAL_VERBOSE
                printf( "%s: error %lu opening handle to %s\n", __FUNCTION__,
                    GetLastError(), buffer);
            #endif
            return -EIO;
        }
    }

    // set up a transmit and receive buffers
    SetupComm( hCom, XBEE_SER_RX_BUFSIZE, XBEE_SER_TX_BUFSIZE);

    /*  Set the COMMTIMEOUTS structure.  Per MSDN documentation for
        ReadIntervalTimeout, "A value of MAXDWORD, combined with zero values
        for both the ReadTotalTimeoutConstant and ReadTotalTimeoutMultiplier
        members, specifies that the read operation is to return immediately
        with the bytes that have already been received, even if no bytes have
        been received."
    */
    if (!GetCommTimeouts( hCom, &timeouts))
    {
        #ifdef XBEE_SERIAL_VERBOSE
            printf( "%s: %s failed (%lu initializing COM%u)\n",
                __FUNCTION__, "GetCommTimeouts", GetLastError(), serial->comport);
        #endif
        CloseHandle( hCom);
        return -EIO;
    }
    timeouts.ReadIntervalTimeout = MAXDWORD;
    timeouts.ReadTotalTimeoutMultiplier = 0;
    timeouts.ReadTotalTimeoutConstant = 0;
    if (!SetCommTimeouts( hCom, &timeouts))
    {
        #ifdef XBEE_SERIAL_VERBOSE
            printf( "%s: %s failed (%lu initializing COM%u)\n",
                __FUNCTION__, "SetCommTimeouts", GetLastError(), serial->comport);
        #endif
        CloseHandle( hCom);
        return -EIO;
    }

    PurgeComm( hCom, PURGE_TXCLEAR | PURGE_TXABORT |
                            PURGE_RXCLEAR | PURGE_RXABORT);

    serial->hCom = hCom;

    err = xbee_ser_baudrate( serial, baudrate);

    if (err)
    {
        serial->hCom = NULL;
        return err;
    }

    #ifdef XBEE_SERIAL_VERBOSE
        printf( "%s: SUCCESS COM%u opened (hCom=%p, baud=%u)\n",
            __FUNCTION__, serial->comport, hCom, baudrate);
    #endif

    return 0;
}

[tomlogic]

It seems like a simpler fix would be to just use the CreateFileA() API, and keep the filename format of \\.\COM99 per CreateFileA documentation.

If you fork this repository to your own GitHub account, you can create a branch and then issue a PR from that branch back to this main project.

[pcdangio]

I just verified that simple CreateFileA() works on my machine. One quick thought: CreateFile() itself is a macro that, from what I can tell, ultimately decides (based on preprocessor definitions) whether to use the actual CreateFileW() or CreateFileA() functions. I did a short search but couldn't track down what may need to be defined / not defined to ultimately get CreateFile() to use CreateFileA() on Windows/MinGW. If we can track that down, this may not have to be implemented as a code change and instead just documented for users to know when to make that definition or not.

[tomlogic]

I think it's safest to just switch to CreateFileA(), which will work regardless of whether UNICODE is defined during compilation. We don't need unicode support to open the COM port, and CreateFileA() has been around since Windows XP.

Since you found this issue, I'd welcome a PR that just switches to CreateFileA(). It could be a good opportunity to learn the pull request flow on GitHub. Let me know if you aren't interested in doing that, and I'll just make a commit with the fix.

[pcdangio]

Submitted pull request: https://github.com/digidotcom/xbee_ansic_library/pull/32

[tomlogic]

Merged PR #32.