Attention is currently required from: jolly, laforge.

fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/30763?usp=email )

Change subject: core: Add software UART implementation ......................................................................

Patch Set 3:

(5 comments)

Patchset:

PS3:

I think something like a soft-uart is an ideal candidate for some unit tests for decoding valid char […]

See https://gerrit.osmocom.org/c/libosmocore/+/35046. So far I am testing both Rx/Tx and parity errors, but not under-/over-flows yet - this is TBD.

File src/core/soft_uart.c:

https://gerrit.osmocom.org/c/libosmocore/+/30763/comment/6085dfec_34753526 PS3, Line 132: TODO: verify parity

not critical to get it merged: But do you still have on your agenda to implement this?

Implemented in https://gerrit.osmocom.org/c/libosmocore/+/35023.

https://gerrit.osmocom.org/c/libosmocore/+/30763/comment/ffb3c23a_3b79f155 PS3, Line 138: fprintf(stderr, "framing e

this was ok for a hack in my private branch, but we shouldn't merge code to master that prints direc […]

This one is going be removed by a follow-up patch (see https://gerrit.osmocom.org/c/libosmocore/+/35022). We may actually want to keep logging, but use LOGP instead?

https://gerrit.osmocom.org/c/libosmocore/+/30763/comment/5d84d58b_3296868c PS3, Line 181: if (!suart->tx.running)

the transmit is not implemented at all? Please squash your additions/fixes into this patch so we don […]

The transmitter is implemented in a follow up patch https://gerrit.osmocom.org/c/libosmocore/+/35044. As was discussed in the IRC, I will for now focus on addressing code review and improving the overall architecture. Once we have settled everything down, we can think of squashing/merging.

https://gerrit.osmocom.org/c/libosmocore/+/30763/comment/1b473b54_ff783cb2 PS3, Line 203: /* FIXME: Tx */

likewise here. the function is a NOP. Please squash your implementation into the patch, thanks.

Ah, I actually wanted to clarify the idea behind this function first. My guess is that this is needed for implementing the "hardware" flow control? But then, wouldn't calling `osmo_soft_uart_set_{rx,tx}()` be enough?