Documentation: REPL: suggestions for UART example

[ewenmcneill]

(I started writing this as a comment on #312 but it got long, so I've made it its own issue)

Re https://glasgow-embedded.org/latest/use/repl-script.html#uart which i had a look at because Attie mentioned it in today's meeting, experimenting with it, it appears that the default iface.read() will read "whatever is available, possibly nothing": see

glasgow/software/glasgow/access/direct/demultiplexer.py

Lines 227 to 241 in 05c44bd

	async def read(self, length=None, *, flush=True):
	if flush and len(self._out_buffer) > 0:
	# Flush the buffer, so that everything written before the read reaches the device.
	await self.flush(wait=False)
	if length is None and len(self._in_buffer) > 0:
	# Just return whatever is in the buffer.
	length = len(self._in_buffer)
	elif length is None:
	# Return whatever is received in the next transfer, even if it's nothing.
	# (Gateware doesn't normally submit zero-length packets, so, unless that changes
	# or customized gateware is used, we'll always get some data here.)
	self._in_stalls += 1
	await self._in_tasks.wait_one()
	length = len(self._in_buffer)

with the default length=None. Which means as presented the example is a bit racey (I started looking into this as on my second write/read cycle in a run I got a partial return, even with "types by hand" delays, and while experimenting I got an empty byte string even after writing bytes).

It might be worth adding a note to the UART REPL example in the documentation to the effect that iface.read() will return "whatever is immediately available, which might not be the whole message as shown in the example". Or perhaps explicitly calling iface.read(length=6) to force reading all bytes expected to be returned for more user-reproducibility. (FTR, I wasn't that surprised by the short read, given it's a byte oriented serial line; but I was a bit surprised by having an iface.read() finish returning a zero length byte string when I had written something.)

Relatedly, help(iface) doesn't include any description of how the length parameter is interpreted on read(), which might be worth adding. Eg, "if length is omitted or None returns data immediately available; otherwise waits until required number of bytes have been received". Which I think would be a doc string on glasglow.access.direct.demultiplexer.DirectDemultiplexerInterface.read() since AFAICT that's what the REPL user is encouraged to use for interactive help:

ewen@parthenon:~$ glasgow repl uart -V 3.3
I: g.device.hardware: device already has bitstream ID d45fa78180875415078a05fcec3dd124
I: g.cli: running handler for applet 'uart'
I: g.applet.interface.uart: port(s) A, B voltage set to 3.3 V
I: g.applet.interface.uart: dropping to REPL; use 'help(iface)' to see available APIs
>>> type(iface)
<class 'glasgow.access.direct.demultiplexer.DirectDemultiplexerInterface'>
>>> 

ETA: Example of the "zero bytes read" read (in this case on the same command line, to make it faster to reproduce; but I did also get this with "up arrow, run again" one command per command line earlier):

UART REPL example showing some racey behaviour

>>> await iface.write(b'hello!')
>>> await iface.read()
<memory at 0x7fc93eac31c0>
>>> bytes(_)
b'hello!'
>>> await iface.write(b'hello!')
>>> await iface.read()
<memory at 0x7fc93eac3400>
>>> bytes(_)
b'hello!'
>>> await iface.write(b'hello!'); await iface.read(); bytes(_)
<memory at 0x7fc93eb5cd00>
b''
>>> await iface.write(b'hello!'); await iface.read(); bytes(_)
<memory at 0x7fc93eac3c40>
b'hello!'
>>> await iface.write(b'hello!'); await iface.read(); bytes(_)
<memory at 0x7fc93ea9f100>
b'hello!'
>>> await iface.read()
<memory at 0x7fc93ea9f4c0>
>>> bytes(_)
b'hello!'
>>> 

(Note also the final iface.read() which caught up on the outstanding message.)

Example of partial message reads, including parts of two messages

>>> await iface.write(b'Hello World!')
>>> await iface.read()
<memory at 0x7fc93eac7400>
>>> bytes(_)
b'Hello World!'
>>> await iface.write(b'Hello World!')
>>> await iface.read()
<memory at 0x7fc93ea9fc40>
>>> bytes(_)
b'He'
>>> await iface.write(b'Hello World!')
>>> await iface.read()
<memory at 0x7fc93eacba00>
>>> bytes(_)
b'llo World!Hello Wo'
>>> await iface.read()
<memory at 0x7fc93eac71c0>
>>> bytes(_)
b'rld!'
>>> 

(FTR, all of this is expected, but IMHO should be documented as expected to avoid user surprise.)

[whitequark]

The UART should simply not return a DirectDemultiplexerInterface directly. In our chat I have called that out as incorrect and we should fix this instead of messing with documentation.