Data processing from COM port (C + win api)

Question

The program's goal is to endlessly wait for data from the com port. When it receives specific data - it does specific action.

The problem is that the program processes the incoming data 2 times and I can't figure out why.

It was assumed that WaitCommEvent(hCom, &state, NULL); will wait for some data if (state == EV_RXCHAR) to arrive, then we read (ReadFile(hCom, dst, 20, &read, NULL)) and check the data.

With these timeouts I thought, that we will read data until the delay between two bytes is not greater than 20ms or the total reading time is not greater than 20*20+100 ms. After that we processing the data.

comTimeOuts.ReadIntervalTimeout = 20;
comTimeOuts.ReadTotalTimeoutMultiplier = 20;
comTimeOuts.ReadTotalTimeoutConstant = 100;
comTimeOuts.WriteTotalTimeoutMultiplier = 0;

Can you tell please, I was wrong in understanding of the timeout's logic?

If I try to catch data without endless loop - the program processes the data 1 time, not 2.

How can I avoid double processing with infinite loop? Thank you in advance!

Full code:

    #include <windows.h>
    #include <stdio.h>
    #include <conio.h>
    #include <signal.h>
    //#include <stdlib.h>
    
    HANDLE hCom;
    HANDLE hEvent;
    
    void diediedie(int sig) {
        //puts("Bye, brutal world!");
        if (hEvent)
            CloseHandle(hEvent);
        if (hCom)
            CloseHandle(hCom);
        exit(1);
    }
    
    void comError(HANDLE hCom, char* s) {
        printf("%s\n", s);
        CloseHandle(hCom);
        printf("Press any key to exit...");
        _getch();
        exit(1);
    }
    
    int main()
    {
        signal(SIGINT, diediedie);
    
        unsigned long int data = 0;
    
        /*******************    Port opening     *********************/
        hCom = CreateFileA("\\\\.\\COM3",    // port name
            GENERIC_READ,                   // Read/Write
            0,                              // No Sharing
            NULL,                           // No Security
            OPEN_EXISTING,                  // Open existing port only
            0,           // Non Overlapped I/O 
            NULL);                          // Null for Comm Devices
    
        if (hCom == INVALID_HANDLE_VALUE)
            comError(hCom, "Error in opening serial port");
        else
            printf("\n   Port \\\\.\\COM3 Opened\n");
    
        /*******************    Setting DCB structure    *********************/
        DCB comDCB;
    
        memset(&comDCB, 0, sizeof(comDCB));
        comDCB.DCBlength = sizeof(DCB);
        comDCB.BaudRate = CBR_9600;
        comDCB.ByteSize = 8;
        comDCB.Parity = NOPARITY;
        comDCB.StopBits = ONESTOPBIT;
        comDCB.fBinary = TRUE;
        comDCB.fParity = FALSE;
        comDCB.fAbortOnError = TRUE;
        comDCB.fDtrControl = DTR_CONTROL_DISABLE;
        comDCB.fRtsControl = RTS_CONTROL_DISABLE;
        comDCB.fInX = FALSE;
        comDCB.fOutX = FALSE;
        comDCB.XonChar = 0x11;
        comDCB.XoffChar = 0x13;
        comDCB.fErrorChar = FALSE;
        comDCB.fNull = FALSE;
        comDCB.fOutxCtsFlow = FALSE;
        comDCB.fOutxDsrFlow = FALSE;
        comDCB.XonLim = 128;
        comDCB.XoffLim = 128;
    
        if (!SetCommState(hCom, &comDCB))
            comError(hCom, "Error in SetCommState");
        else
            printf("\n   Setting DCB Structure Successfull\n");
    
        GetCommState(hCom, &comDCB);
        printf("\n\tBaudrate = %d\n\tByteSize = %d\n\tStopBits = %d\n\tParity   = %d\n", comDCB.BaudRate, comDCB.ByteSize, comDCB.StopBits, comDCB.Parity);
    
        /*******************    Setting Timeouts     *********************/
        COMMTIMEOUTS comTimeOuts;
        comTimeOuts.ReadIntervalTimeout = 20;
        comTimeOuts.ReadTotalTimeoutMultiplier = 20;
        comTimeOuts.ReadTotalTimeoutConstant = 100;
        comTimeOuts.WriteTotalTimeoutMultiplier = 0;
        comTimeOuts.WriteTotalTimeoutConstant = 0;
        if (!SetCommTimeouts(hCom, &comTimeOuts))
            comError(hCom, "Error in setting timeouts");
        else
            printf("\n   Setting Serial Port Timeouts Successfull\n");
    
        /*******************    Setting Buffer   *********************/
        SetupComm(hCom, 20, 0);
        if (!SetupComm(hCom, 20, 0))
            comError(hCom, "Error in setting buffer");
    
        unsigned long wait = 0, read = 0, state = 0;
        unsigned char dst[20] = { 0 };
        hEvent = CreateEvent(NULL, FALSE, FALSE, NULL);
        /* Setting event mask */
        if (!SetCommMask(hCom, EV_RXCHAR))
            comError(hCom, "Error in setting SetCommMask");
        else {
            while (1) {
                WaitCommEvent(hCom, &state, NULL);
                if (state == EV_RXCHAR) {
                    if (ReadFile(hCom, dst, 20, &read, NULL)) {
                        printf("%s", dst);
                        data = atoi(dst);
                        switch (data) {
                        case 16724175:
                            printf("yes\n");
                            break;
                        }
                    }
                }
            }
        }
        CloseHandle(hEvent);
        CloseHandle(hCom);//Closing the Serial Port
        return 0;
    }

Output:

   Port \\.\COM3 Opened

   Setting DCB Structure Successfull

        Baudrate = 9600
        ByteSize = 8
        StopBits = 0
        Parity   = 0

   Setting Serial Port Timeouts Successfull

16718055
16718055

You haven't used read the number of bytes actually read, but assumed there will be a nul-terminated string available. — Weather Vane
– Weather Vane, Commented Jul 10, 2022 at 12:29
@WeatherVane Do you mean, I need additionally to check the read variable? ... if (!read) { ... }. Yes, it works, thanks! But why did the previous code check the data 2 times (not 1 or not 3 times)? Can't understand the logic. — kancler
– kancler, Commented Jul 10, 2022 at 12:55
Yes, always check the result of any input operation, if only to protect against fools and glitches. It is not a boolean. It is the number of characters read. Also, don't guess what to expect. Process the actual number of characters that arrive. If you are going to pass the data to a string handling function, make sure you nul-terminate it, and with a buffer large enough to do so. And never use a buffer that is only just big enough. It costs nothing to make sure that you don't get buffer overruns. — Weather Vane
– Weather Vane, Commented Jul 10, 2022 at 13:00
@WeatherVane data is sent with /r/n at the end. if I have such loop: while (1) { WaitCommEvent(hCom, &state, NULL); if (state == EV_RXCHAR) if (ReadFile(hCom, dst, 20, &read, NULL)) { printf("read: %d\n", read); printf("data: %s", dst); ... output is: read: 10 data: 16718055 read: 0 data: 16718055 As you can see the loop runs 2 times with each sending and the second time read = 0, Isn't it a crutch using if (read>0) condition? — kancler
– kancler, Commented Jul 10, 2022 at 13:14
No, it is necessary. Never ignore the count of input. Process the actual number of characters, otherwise you'll process the same data that was previously read into the buffer. The second iteration returned 0 yet you handled the previously read data twice. It was placed in the buffer at the first iteration, not the second. If you are going to pass the data as a string, then you must do dst[read] = '\0'; and make the buffer big enough. — Weather Vane
– Weather Vane, Commented Jul 10, 2022 at 13:19

Weather Vane · Accepted Answer · 2022-07-10 13:31:51Z

You must always check the return value from input functions. In this case you have ignored it, and processed the input from the first iteration twice. It wasn't sent twice, but on the second iteration was already in the buffer.

Also, you should nul-terminate any data that you are going to pass to a string-handling function. And to do that, the buffer must be big enough. It's usually worth allocating a buffer a little larger than you actually need.

So the modified code is

unsigned char dst[32];              // large enough buffer
//...
while (1) {
    WaitCommEvent(hCom, &state, NULL);
    if (state == EV_RXCHAR) {
        if (ReadFile(hCom, dst, 20, &read, NULL)) {
            dst[read] = '\0';       // nul terminate
            printf("%s", dst);
            data = atoi(dst);
            switch (data) {
            case 16724175:
                printf("yes\n");
                break;
            }
        }
//...

Collectives™ on Stack Overflow

Data processing from COM port (C + win api)

1 Answer 1

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

Comments

Your Answer

Sign up or log in

Post as a guest

Related