Make sure pretty_hex works under Python 2 and Python 3.

[mithro]

• Run doctests under tox.
• Add pretty_hex doctests for string, bytes and unicode

Fixes #48.

[mithro]

Will merge if CI confirms the doctests are run and it goes green.

[ewenmcneill]

@mithro,

FYI, we never did decide that we'd release 1.0.24 as a production release. The various requests for testing (eg, https://discourse.tinyfpga.com/t/tinyprog-1-0-24-testing/886 and #25 (comment)) didn't really get much response. AFAICR the only 1.0.24 issue found was someone who had trouble with the 1.0.24devNN version string containing letters, but it's a bit inconclusive (eg, probably only litex-buildenv really does testing of 1.0.24 in practice).

Given the various issues people have had, possibly we should consider what would be required to be happy to make a 1.0.24 actual (non-dev) release, so more people get to use that version?

Also FWIW, my guess is that a bunch of these errors people are seeing -- #48, #18, #42, #43 -- are actually the result of shorter-than-expected reads, due to race conditions in the serial IO code, which seems to assume that a read of N bytes will either all happen or none of it will happen, but that's definitely not true on any unix/linux; unix/linux will happily supply 1-N bytes if it has some of them now. I suspect most of the issues reported are in the bootloader update simply because if it crashes out at the wrong point (stage 2 seems the worst), then it bricks the board to the point of needing a SPI programmer -- see https://discourse.tinyfpga.com/t/error-while-updating-bootloader-invalid-literal-for-int-with-base-10/484/3. But I still seem them periodically even with uploading regular code. (I don't think 1.0.24devNN is any worse for this than 1.0.23, perhaps better for it, hence this comment; but I do suspect that Python 3 might be somewhat worse for short reads than Python 2, possibly just due to being faster.)

Ewen

[ewenmcneill]

@mithro Re shorter than expected reads, see also #42 (comment), where someone modified their own version of tinyprog to handle short reads. They just retry the command, but in practice I suspect the low level read() ought to be tried again until the right number of bytes have been received, as is usual C practice for using read().

Ewen

PS: Feel free to, eg, move these comments to a new issue if you'd prefer.

[ewenmcneill]

@mithro Also, in Python 3 bytearray and "" are pretty different things, but in Python 2 they're the equivalent. So

TinyFPGA-Bootloader/programmer/tinyprog/__init__.py

Lines 159 to 165 in 99f87a5

	def read(self, length):
	try:
	if length > 0:
	data = self.IN.read(length)
	return bytearray(data)
	else:
	return ""

is probably non-portable. I wonder if that's the trigger for a bunch of these issues on Python 3? Maybe the length = 0 case should explicitly return an empty bytearray?

Ewen

[ewenmcneill]

@mithro pyserial read() is explicitly documented as "Read size bytes from the serial port. If a timeout is set it may return less characters as requested. With no timeout it will block until the requested number of bytes is read.". And the programming tool explicitly opens the port with a 1 second timeout, which means short reads quite possible (writes will raise an exception instead).

But AFAICT the code at

TinyFPGA-Bootloader/programmer/tinyprog/__init__.py

Lines 339 to 350 in 99f87a5

	def read(self, addr, length, disable_progress=True, max_length=255):
	data = b''
	with tqdm(desc=" Reading", unit="B", unit_scale=True, total=length,
	disable=disable_progress) as pbar:
	while length > 0:
	read_length = min(max_length, length)
	data += self.cmd(0x0b, addr, b'\x00', read_len=read_length)
	self.progress(read_length)
	addr += read_length
	length -= read_length
	pbar.update(read_length)
	return data

which calls

TinyFPGA-Bootloader/programmer/tinyprog/__init__.py

Lines 297 to 304 in 99f87a5

	def cmd(self, opcode, addr=None, data=b'', read_len=0):
	addr = b'' if addr is None else struct.pack('>I', addr)[1:]
	write_string = bytearray([opcode]) + addr + data
	cmd_write_string = b'\x01' + struct.pack(
	'<HH', len(write_string), read_len) + write_string
	self.ser.write(bytearray(cmd_write_string))
	self.ser.flush()
	return self.ser.read(read_len)

just assumes that it read everything if it didn't get an exception, and doesn't check for/retry short reads. Which probably leaves the serial stream out of sync, causing things to get worse from there (including these pretty print issues).

Ewen

PS: Sorry this all ended up in a merged pull request :-) It was how I found the references, and then started looking at the code and at the beginning it was just going to be a quick comment in passing....