Skip to content

Conversation

@lukehugh
Copy link

Add full duplex communications support on Windows.

When I use a separate thread to read the serial data, I notice that the write operation is blocked.

Here is a simple program to illustrate this.

#include <iostream>
#include <chrono>
#include <thread>
#include "serial/serial.h"

using namespace std;
using namespace chrono;
using namespace std::chrono_literals;
using namespace serial;

int main(int argc, char **argv) {
    Serial serial_port("COM5", 9600, Timeout::simpleTimeout(200));

    thread read_serial([&](){
        while (true) {
            uint8_t ch;
            serial_port.read(&ch, 1);
        }
    });

    while (true) {
        // Test write latency.
        auto start = high_resolution_clock::now();
        serial_port.write("Hello world!\n");
        auto end = high_resolution_clock::now();
        double dt = duration<double>(end - start).count();

        cout << "Write using " << (dt * 1000) << " ms" << endl;

        this_thread::sleep_for(500ms);
    }
}

Output:

Write using 0.0576 ms
Write using 314.211 ms
Write using 325.428 ms
Write using 299.138 ms
Write using 524.347 ms
Write using 702.12 ms
Write using 513.909 ms
...

This does not make sense because the serial port is supposed to communicate asynchronously. To fix this problem, I modified win.h and win.cc to enable asynchronous communication by adding the OVERLAPPED option. This also helps to solve the issue #150.

This is the right result:

Write using 0.0093 ms
Write using 0.034 ms
Write using 0.0564 ms
Write using 0.012 ms
Write using 0.0343 ms
Write using 0.0494 ms
Write using 0.0124 ms
Write using 0.01 ms

Copy link

@JoeyLiang-0 JoeyLiang-0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Applied your patch with several minor modifications, works great so far. Thanks,

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants