Add fast binary file transfer to SD card option

[p3p]

Description

Adds support for transferring arbitrary files over the serial connection from host software.

I implemented a simple custom protocol, any input into improvements are welcome, I have written similar things in the past but this wasn't exactly thoroughly designed (there may have been a doodle), its a very simple packet structure and keeps the host side of the communication as ASCII (2 byte command tokens) apart from a 16bit max packet size value transferred as part of the stream initialisation.

The host side of the protocol is implemented in the python example attached.
(superseded)

Benefits

Ability to transfer binaries over the serial connection on devices that can be flashed from sd card and
due to less overhead and constant maximum packet size it improves transfer rate of gcode files.

Until this has host support it will be of very limited use, and we would need to report its availability.

[TheSFReader]

I'd suggest to separate the BinaryStream into its own files, to limit the queue.cpp fle size and overall complexity. (And Good Job p3p !)

[p3p]

yea sorting out the feature configuration and tidying up was on the agenda for after initial review, the feature is pretty much completely encapsulated in its class so that's not a problem.

@thinkyhead It would probably be better just to conditional compile the whole thing on whether the sdcard is enabled rather than pick out the individual parts unless you have plans for alternate steam types.

[p3p]

seems that there's a bug where sometimes the filename is wrong in the transfer completed message, which would mean on error it would delete the wrong file, so I may have misunderstood how and when this variable is initialised.

[thinkyhead]

If you use B without a value, then B will be assumed to be the start of the string_arg. It would be better to let M28 take the whole line as string_arg and parse it afterward, as done in GcodeSuite::M118.

[p3p]

Using a standard gcode argument B followed by the filename is an artefact of an earlier design where you had to send the number of blocks that would be transferred, I left it in as I preferred the simplicity of design, and made it easy not to break backwards compatibility.

What format would you prefer?, M28 B <filename>? we would be relying on the white-space then to differentiate command versions, rather than a 0, which is a bit clearer. The number could be used to specify protocol version if any major change is ever needed.

[thinkyhead]

Use M118 as a guide for how to deal with a string argument following code-word parameters.

[marcio-ao]

Just curious, does this do error checking and recovery? Serial comms are full of errors. If not, I wonder if it makes sense to reuse a historical protocol, such as ZModem that already solves a lot of those problems.

[p3p]

It would be pretty useless without error checking ;) considering I'm using it for transmitting firmware, each packet has a 16bit checksum, a size that can be varied depending on transmission errors, and there is a resend packet request mechanism. In order to keep buffer space to a minimum the protocol does not allow out of order sending so each packet must be acked before the next is sent.

The main benefit of ZMODEM is the multiple packets in transit simultaneously which is not applicable here, although adding the packet id to the ack would allow it given enough memory it over complicates matters.

[marcio-ao]

It would be pretty useless without error checking ;)

@p3p: Ah. Okay. Glad you already have that covered :)

The main benefit of ZMODEM is the multiple packets in transit simultaneously which is not applicable here, although adding the packet id to the ack would allow it given enough memory it over complicates matters.

Sounds like you already looked into the pros and cons of using it. I don't know much about ZModem other than I remembered using it back in the stone ages :)

[MagoKimbra]

But how do you use it?
I put the first command M28 B1 test.gco at the beginning of gcode.
Launched the gcode but gives me error:

15: 15: 47.029: Writing to file: test.gco
15: 15: 47.547: echo: Datastream packet out of orderecho: Resend request
15: 15: 47.547: rs
15: 15: 48.051: echo: Datastream timeout
15: 15: 48.051: echo: Resend request
15: 15: 48.051: rs
15: 15: 48.563: echo: Datastream timeout
15: 15: 48.566: echo: Resend request
15: 15: 48.566: rs
15: 15: 49.066: echo: Datastream timeout
15: 15: 49.075: File deleted: TEST.GCO
15: 15: 49.075: echo: File transfer failed
15: 15: 49.075: sf

[TheSFReader]

As far as I understand, you need to use the transfer.py sample linked in first message to manage the host/sender side, or an other compatible implementation.

[MagoKimbra]

Ok thank's i try now...

[p3p]

@MagoKimbra Indeed as @TheSFReader said it will require host support, the python script in the PR description is an example of the host side of the protocol.

This was merged faster than expected and was only submitted as a "concept", I expected a little more feedback from users before it was merged. It hasn't really been tested much on normal UART or USB to UART bridges, I needed a way to transfer my firmware over the serial connection and as my MCU has native USB CDC that's what I tested with, as such I don't really know what performance you will get, on CDC I get 80 - 100KiB/s.

Any feedback is welcome.

[MagoKimbra]

Excuse me, I did not understand that the host's support was needed, I thought it was enough to add B1 to the M28 command to use it ...

[thinkyhead]

Merging things is a great way to get feedback. Otherwise very few ever notice most PRs.

[Dark-Guan]

@p3p Hi, Friend, I want to use the FAST_FILE_TRANSFER function. I use the file transfer.py to test the process. Actully there are something werid thing.

Transfering packet(0) 6B
<<<  b'0 0 0 0 6 0 23 198 Header Buffer 0\r\n'
<<<  b'52 18 160 50 1 0 Data\r\n'
<<<  b'Buffer 0\r\n'
<<<  b'echo: Datastream initialized (78496Bytes expected)\r\n'
<<<  b'so`\x00\n'
max supported block size: 96
Send Block b'( Input file was xinshijifuyinzans.mid )\r\nG21 (Metric FTW)\r\nG90 (Absolute posiitioning)\r\nG92 X0 '

Transfering packet(1) 96B
<<<  b'1 0 0 0 96 0 242 88 Header Buffer 1\r\n'
<<<  b'40 32 73 110 112 117 116 32 102 105 108 101 32 119 97 115 32 120 105 110 115 104 105 106 105 102 117 121 105 110 122 97 110 115 46 109 105 100 32 41 13 10 71 50 49 32 40 77 101 116 114 105 99 32 70 84 87 41 13 10 71 57 48 32 40 65 98 115 111 108 117 116 101 32 112 111 115 105 105 116 105 111 110 105 110 103 41 13 10 71 57 50 32 88 48 32 Data\r\n'
<<<  b'Buffer 0\r\n'                                                                                                    
<<<  b'ok\r\n'
Send Block b'Y0 Z0 (set origin to current position)\r\nG0 X0 Y0 Z0 F2000.0 (Pointless move to origin to reset f'

Transfering packet(2) 96B                                                                                               
<<<  b'116 105 111 110 41 13 10 71 Header Buffer 55\r\n'                                                               
 <<<  b'echo: Datastream packet out of orderecho: Resend request 1\r\n'
<<<  b'rs\r\n'
Transfering packet(2) 96B
<<<  b'2 0 0 0 96 0 167 50 Header Buffer 0\r\n'
<<<  b'89 48 32 90 48 32 40 115 101 116 32 111 114 105 103 105 110 32 116 111 32 99 117 114 114 101 110 116 32 112 111 115 105 116 105 111 110 41 13 10 71 48 32 88 48 32 89 48 32 90 48 32 70 50 48 48 48 46 48 32 40 80 111 105 110 116 108 101 115 115 32 109 111 118 101 32 116 111 32 111 114 105 103 105 110 32 116 111 32 114 101 115 101 116 32 102 Data\r\n'
<<<  b'Buffer 0\r\n'
<<<  b'ok\r\n'                                                                                                          
Send Block b'eed rate to a sane value)\r\nG01 X2.4916695588 Y1.8666430666 Z1.4815555838 F217.2157799874\r\nG01 X5'                                                                                                                              Transfering packet(3) 96B                                                                                               
<<<  b'3 0 0 0 96 0 154 94 Header Buffer 0\r\n'
<<<  b'101 101 100 32 114 97 116 101 32 116 111 32 97 32 115 97 110 101 32 118 97 108 117 101 41 13 10 71 48 49 32 88 50 46 52 57 49 54 54 57 53 53 56 56 32 89 49 46 56 54 54 54 52 51 48 54 54 54 32 90 49 46 52 56 49 53 53 53 53 56 51 56 32 70 50 49 55 46 50 49 53 55 55 57 57 56 55 52 13 10 71 48 49 32 88 53 Data\r\n'                                         
<<<  b'Buffer 0\r\n'
<<<  b'ok\r\n'

I have adde some more debug message.The packet(2) was resend, Actually there are 40 bytes lost, this situation is always happened, And the Speed is limited be this error;

I think There may be some Problem;

[p3p]

I have some updates to the protocol to push upstream that help with stream recovery and reliability but I'm not sure it will help you as you seem to be dropping data at the hardware level?

What type of serial connection are you using?

[rafaljot]

It looks like there is a little bug in transfer.py line 82
max_block_size = int.from_bytes(value, byteorder='little', signed=False)

should be

max_block_size = int(value)

and line 100
return res[0:2], res[2:4]
should be
return res[0:2], res[2:].strip()

transfer.py.zip

[rafaljot]

4.3MB in 45s it starts to be usable :)
with SKR-PRO (stm32) @ 2000000

<<<  b'echo: preparing to receive: test4MB.g\r\n'
<<<  b'echo:Now fresh file: test4MB.g\n'
<<<  b'Writing to file: test4MB.g\n'
<<<  b'echo:Datastream initialized (4321898 bytes expected)\n'
<<<  b'so256\n'
max supported block size: 256
................................................................................
................................................................................
........<<<  b'echo:TEST4MB.G transfer completed @ 91KiB/s\n'
<<<  b'sc\n'
File Transfered in 45.75 s
Process finished with exit code 0

[p3p]

@rafaljot Sorry about that was sure I'd updated the host example when I changed the connect response to ascii.

(mumbles something about finishing the updates mentioned in my previous post .. march isn't that long ago right?)

[rafaljot]

@luc-github @p3p
4.3MB upload time with different MAX_CMD_SIZE
96 - 62.89 s
256 - 45.75 s
512 - 35.27 s
1024 - 34.96
2048 - 34.96 s

[p3p]

I have a PR that improves stream recovery ready (superseded) but I may wait until I finish the changes I'm working on to generalise the transport layer, hopefully finalising the protocol before anyone uses it in a host if they ever do ^^.

[p3p]

@rafaljot I put a PR (#14817) in for the generalisation update if your interested