Open-Bionics / Beetroot

BETA - Release of the Open Bionics Brunel robotic hand control firmware for the Chestnut board
42 stars 28 forks source link

UART TX buffer overflow and hang #1

Closed OllyMcBrideOB closed 7 years ago

OllyMcBrideOB commented 7 years ago

When the hand is in 'CSV/Fast Mode', the hand sends a string of the current position of each finger.

If this data is not 'read' then the hand will hang.

I believe this may be due to the TX UART buffer on the Brunel hand filling up and the program waiting for it to be empty enough to allow another position string.

MartinYoungOB commented 7 years ago

Reproduced by opening the console, putting the hand into test mode (A0) then turning on CSV output (A4) and noting that the test continues. If the console is then disconnected (the terminal emulator disconnects from the COM: port) then the test mode halts when it would ideally continue.

This was indeed down to a back-up in the serial comms.

The obvious solution is to use Serial.availableForWrite() but for the instantiation of this class being used here, this function always returns 63. Every time. It does this because USB serial comms claim to fully flush on every every right so the full on-chip buffer is available all the time.

Happily the Serial_ class provided access to the pseudo-DTS signal which does go low when the PC end isn't listening.

So we check DTS before each write (at the level of the MYSERIAL_WRITE macro et al) and only do it if DTS is set.

Checked by following the reproduction instructions and noting that the demo mode now continues through multiple connection and disconnection cycles.

MartinYoungOB commented 7 years ago

Unfortunately these changes, whilst they avoid blocking when no console is attached or when it is detached mid-run, do not fix the case where there is a console attached but it doesn't read the data.

In this case writes will eventually block but there is no indication at the Arduino level that this will happen: availableForWrite() still returns 63, dts() does not go low. It's possible that the USB could detect this situation but the information is not made available to higher-up structures.

The Right Thing would probably be to note this restriction in the streaming CSV mode and add an extra command to return a single instance of the current hand position. Provided the client always reads the response prior to issuing another query then the buffer will not fill up and writes will not block.

OllyMcBrideOB commented 7 years ago

The finger positions will no longer be streamed when in CSV mode.

The serial command 'A6' is used to request a string of the finger positions.