movement doesn't buffer serial input correctly

[wryun]

movement.c uses read to grab something from the input, then processes it immediately, regardless of whether there's a newline in the buffer.

This means that if you use a normal terminal emulator (i.e. picocom), which sends characters immediately as they're typed - as opposed to Arduino's one, which sends a bunch of characters at once - every filesystem_process_command is called with a single character. And even using Arduino's, you can only send about 50 chars or so before the line is effectively broken at that point due to how fast the characters are transmitted (or some buffer size somewhere).

I believe the easiest fix is to implement an fgets-ish thing in the loop. Unfortunately, we can't use normal fgets at the moment (AFAIK), as it will block up the loop. Moreover, even if we were prepared to be a bit hacky by calling fgets only once there was input to read, AFAIK because select/poll isn't implemented there's no way to detect if there's something is there without actually calling read(), and one is not supposed to mix read() with the normal stdio functions.

Note that from messing around with serial I/O I suspect there are also underlying issues with the serial implementation in TinyUSB, but this is the issue I'm 99% certain exists and is easily fixable.

[wryun]

Just looked at the code in watch_private.c. There may be a few other issues:

• _read doesn't look like it can read NULL bytes, making binary transfer impossible (the output of tud_cdc_n_read isn't used)
• cdc_task always puts bytes at the start of the buffer, rather than appending to the buffer (what happens if no-one has called 'read' before the next TC0_Handler occurs?). Maybe the timings always work out ok... EDIT: I guess the expectation is that we call read exactly once each time through the loop, and buffer inside the loop if no '\n'.
• the buffer seems small to me and could easily overflow (but possibly relates to above... it would be nice at least to document what the timing constraints are here, I guess)

[bademux]

test script

import serial

ser = serial.Serial("/dev/ttyACM1",baudrate = 19200, timeout=1)

if ser.isOpen():

    ser.write("echo otpauth://totp/ACME%20Co:[email protected]?secret=HXDMVJECJJWSRB3HWIZR4IFUGFTMXBOZ&issuer=ACME%20Co&algorithm=SHA1&digits=6&period=30 > totp_uris.txt".encode())
    print(ser.read(256).decode("ascii"))

    ser.write("ls".encode())
    print(ser.read(256).decode("ascii"))

    ser.write("cat totp_uris.txt".encode())
    print(ser.read(256).decode("ascii"))
ser.close()

and output is (two empty lines after echo) and command timeouts.

$ python totp_test.py
$ echo otpauth://totp/ACME%20Co:[email protected]?secret=HXDMVJ

after replug ls returns totp_uris.tx without txt without T

lsdir     0 bytes .
dir     0 bytes ..
file   45 bytes totp_uris.tx

and cat

Traceback (most recent call last):
  File "/home/deck/workspace/Sensor-Watch/movement/make/../../../totp.py", line 11, in <module>
    print(ser.read(256).decode("ascii"))
UnicodeDecodeError: 'ascii' codec can't decode byte 0xba in position 64: ordinal not in range(128)

[wryun]

👍 nice test script.

[bademux]

@josecastillo sorry disturbing you is it possible for you to take a look into the issue?

It will be real quality of life improvement, it really will help us simplify firmware upgrade proces and keep sensitive data out of generic code.

[wryun]

See #344

[madhogs]

I believe this is now fixed following the merge of #344 and can be closed.