ardera / flutter_packages

My collected packages for pub.dev
MIT License
28 stars 7 forks source link

[linux_serial] unclear timeout behaviour #30

Open Markus43 opened 6 months ago

Markus43 commented 6 months ago

I have a usecase where I need to communicate with devices to identify them, and I can't rule out that sometimes a device doesn't give me replies at all. This is where the timeout behaviour of the serial port becomes important, in order to not loose time and get performant behaviour. So I tried to find out how this library behaves regarding timeouts. In makeRaw you are setting:

    ptr.ref.c_cc[VMIN] = 0;
    ptr.ref.c_cc[VTIME] = 0;

In makeRawAndSetBaudrate you are setting:

    ptr.ref.c_cc[VMIN] = 1;
    ptr.ref.c_cc[VTIME] = 100;

According to https://man7.org/linux/man-pages/man3/termios.3.html -> noncanonical mode, the first case means polling read (give back what is available) while the second case means to block until the first character was received and then to have 10 seconds (unit is tenth's of seconds) timeout between characters. The second case is used as soon as a baudrate has been configured.

I'd like to understand the reasoning behind these settings - what did you have in mind here? Would you consider adding a configurable timeout to this library, or would you at least accept a PR that implements such a feature?

ardera commented 6 months ago

In makeRawAndSetBaudrate you are setting:

    ptr.ref.c_cc[VMIN] = 1;
    ptr.ref.c_cc[VTIME] = 100;

That's a bug. It should be both 0 like in makeRaw.

I have a usecase where I need to communicate with devices to identify them, and I can't rule out that sometimes a device doesn't give me replies at all. This is where the timeout behaviour of the serial port becomes important, in order to not loose time and get performant behaviour.

That makes sense. This package is mostly designed to handle as much as possible in dart rather than in the kernel, i.e. buffering is done in dart rather than the kernel (data from the serial port is read as soon as it's available and put into dart buffers / streams), so not sure how easy it is to integrate with the current API. But other than that it's definitely useful functionality and if you'd prepare a PR, I'd approve it.

Markus43 commented 6 months ago

In makeRawAndSetBaudrate you are setting:

    ptr.ref.c_cc[VMIN] = 1;
    ptr.ref.c_cc[VTIME] = 100;

That's a bug. It should be both 0 like in makeRaw.

... if you'd prepare a PR, I'd approve it.

I want to look into this again as soon as I find the time for it, not before next year unfortunately.

Regarding "not sure how easy it is to integrate": With the current architecture, I would probably put a timeout on the streams, this function here: https://api.flutter.dev/flutter/dart-async/Stream/timeout.html. What do you think about the general idea, is it worth to proceed with it or do you have reservations about the approach?

ardera commented 6 months ago

That works yeah. The difference between that and in-kernel timeout is that the dart-side timeout is not as reliable of course, i.e. when there's a lot of work for the dart vm (hot reloading, hot restarting), could be it just takes a long time to get to handling the serial port data, and it times out before it gets there.

I'm doing a similar thing with linux_can, and after a hot restart it regularly takes like 10s till the CAN frame is seen by the main dart isolate, after it was received by the kernel. So if you really need reliable timeouts, using the in-kernel timeouts is better. But if you don't, Stream.timeout is fine