search

monome / druid

terminal interface for crow
GNU General Public License v3.0
35 stars 16 forks source link

ensure crlf line endings to prevent upload crashes #48

Closed csboling closed 5 years ago

csboling commented 5 years ago

Since crow expects CRLF line endings (I am not exactly sure what it is causes problems with LF-only line endings), line endings need to be adjusted when uploading file contents. Sending line endings as-is on Linux and Mac would typically only send \n, causing upload failures / hangs / other odd states. This patch corrects line endings by using rstrip to remove trailing whitespace and then adding \r\n to each line sent during upload.

This explains why the patch https://github.com/monome/crow/pull/232 improves upload stability a lot on Windows, but uploads were still failing for me on Mac - on Windows the line endings are already CRLF. In combination with https://github.com/monome/crow/pull/232 I believe this patch resolves a lot of upload issues, possibly including https://github.com/monome/druid/issues/44 and https://github.com/monome/druid/issues/47.

simonvanderveldt commented 5 years ago

Nice find! Shouldn't we fix crow though to either not care about this or just work when there's only a \n?

@trentgill do you know why crow expects a \r\n?

tehn commented 5 years ago

expert find.

this is huge, thank you!

tehn commented 5 years ago

i merged this as a short-term fix ahead of any firmware updates.

csboling commented 5 years ago

It also seems possible that this is getting buffered on the PC side, in pyserial or elsewhere, not sending until it decides it has a "whole line" rather than this being an issue on the crow side.