Poll for incoming UART data

[kovmir]

How do I poll for new data with this library?

[akaeba]

Hi Kovmir, please check out this function:

int simple_uart_has_data(struct simple_uart *uart);

In addtion the project USBISS shows also how to deal with simple_uart:
https://github.com/andkae/usbiss/blob/main/usbiss.c#L145-L161

[kovmir]

I want to poll continuously for incoming data, this approach saturates the CPU core:

#include <stdio.h>
#include <unistd.h>

#include "simple_uart.h"

#define UART_DEVICE "/dev/ttyUSB0"
#define BUF_SIZE 256

int
main(void)
{
	struct simple_uart *uart;
	ssize_t n_bytes;
	char buf[BUF_SIZE];

	uart = simple_uart_open(UART_DEVICE, 115200, "8N1");
	if (uart == NULL) {
		fprintf(stderr, "failed to open %s\n", UART_DEVICE);
		return 1;
	}

	for (;;) {
		if (simple_uart_has_data(uart)) {
			n_bytes = simple_uart_read(uart, buf, BUF_SIZE);
			printf("%ld bytes read\n", n_bytes);
		}
	}
}

[andkae]

Hi Kovmir,

let's start with some basic math, we have 115200bps. Per char we need 10bit - start, char, stop. That means we receive every ~90us a new char. So what about to add an usleep in your for loop to say the scheduler it can send this task to sleep? Or polling this function by an 1ms timer interrupt?

Hops that helps.

BR,
Andreas

[kovmir]

Delay? Ok.

[AndreRenaud]

At the moment, we open the uart in a non-blocking way, so yes, it results in CPU churn when polling. This is convenient for most of the applications I use this for, even if it's not that efficient.

However easy enough to add a flag to the mode (8N1B) or something, to make it open in blocking mode if that's what works for your application.

[AndreRenaud]

@kovmir I've added simple_uart_read_timeout which will block for a given number of milliseconds. Does that solve the problem?

#45

[akaeba]

Hi André,

to the latest changes on simple_uart_read_timeout i have some concernes, since simple_uart_read maps also to it.

In code sequence:

simple_uart/simple_uart.c

Lines 129 to 135 in 5e8b6fe

	// Have a timeout of 50ms to avoid just thrashing the CPU
	FD_ZERO(&readfds);
	FD_SET(sc->fd, &readfds);
	FD_ZERO(&exceptfds);
	FD_SET(sc->fd, &exceptfds);
	t.tv_sec = 0;
	t.tv_usec = 50 * 1000;

is always a force wait time implemented, even if it set timeout_ms to zero. Is that really the behaviour what the API user expects if is set the timeout_ms to zero? In addition behaves now the windows implementation different from the linux implementation.

Another thing to the redirect from simple_uart_read:

simple_uart/simple_uart.c

Line 88 in 5e8b6fe

if (!sc || !buffer || max_len == 0)

simple_uart/simple_uart.c

Line 95 in 5e8b6fe

if (!sc || !buffer || max_len == 0)

If max_len is set to zero, i had expect to return 0, we have no memory the function will complete succesfully with zero read bytes

Thanks for sharing your thoughts and the great maintianing of this project.

BR,
Andreas

[AndreRenaud]

Hi Andreas,
Good point about a read of 0 bytes - that should just return immediately I think (with a result of 0).

For the new timeout read, I left in the previous behaviour of doing a delay of 50ms to avoid CPU churn. However I think it would be better to just adopt the new behaviour by default, and you can always use simple_uart_read_time(sc, buf, len, 50) if you want the old behaviour.

Any thoughts on #50 ?

[akaeba]

Hi André,

Any thoughts on #50 ?

No, looks perfect. Thanks for changing :)

BR,
Andreas

[AndreRenaud]

Thanks for checking. I'll close this as we now have a both async & sync options, so up to the user to decide how to use it.

[kovmir]

@kovmir I've added simple_uart_read_timeout which will block for a given number of milliseconds. Does that solve the problem?

#45

I does solve the problem. You got it right, I wanted a blocking function.

@AndreRenaud, should you declare simple_uart_read(...) inline?

[AndreRenaud]

It could be, but I generally prefer not to do that for libraries like this, as it clutters the header with implementation details that might change in the future. If you're using LTO builds, then this would be resolved by that anyway most likely.