Add XMODEM support compatible w/ HDMI2USB #3
Conversation
flterm.c
Outdated
| } else { | ||
| } else if(c != 'C') { | ||
| /* If XMODEM start not detected, continue looking | ||
| for SFL start. */ |
There was a problem hiding this comment.
Could you please elaborate on what's wrong with the indentation?
https://github.com/cr1901/flterm/blob/master/flterm.c#L747-L769
There was a problem hiding this comment.
See https://github.com/cr1901/flterm/blob/9e11d2c7e32cc196b5f7aa6109c9f817d43c91cf/flterm.c#L467-L470 for example.
This style of comment should be wrapped like this;
/* If XMODEM start not detected, continue looking
* for SFL start.
*/| @@ -433,6 +576,8 @@ static void do_terminal( | |||
| const char *initrd_image, unsigned int initrd_address, | |||
| char *log_path) | |||
| { | |||
| int first_xstart = 1; /* XMODEM should only run once, and only if kernel | |||
| @@ -36,6 +36,7 @@ | |||
| #include <unistd.h> | |||
There was a problem hiding this comment.
You seem to have lots of extra newlines in a couple of places?
There was a problem hiding this comment.
Can you also add doxygen style comments to your new functions?
There was a problem hiding this comment.
I feel like the --help needs some type of update too?
There was a problem hiding this comment.
Two things I can think of saying:
"If a kernel_image is passed, flterm will listen for either an XMODEM start number (if --xmodem is passed) or SFL magic number, whichever comes first."
(I haven't implemented the --xmodem option yet)
"To avoid spurious transfers due to the XMODEM start character being within the printable ASCII set, an XMODEM session is only attempted once after invocation."
flterm.c
Outdated
| @@ -206,6 +232,103 @@ static int upload_fd(int serialfd, const char *name, int firmwarefd, unsigned in | |||
| return length; | |||
| } | |||
|
|
|||
| static int upload_xmodem(int serialfd, const char *name, int firmwarefd) | |||
There was a problem hiding this comment.
Can you add a doxygen style comment about this function?
There is something about only supporting 1k packets or something?
flterm.c
Outdated
| gettimeofday(&t0, NULL); | ||
|
|
||
| // Abbreviated XMODEM transmitter- only sends 1024 byte packets. Expects | ||
| // receiver to |
|
Everything except "--help/--xmodem" should be fixed to your satisfaction (see comment). Could we please discuss :)? |
flterm.c
Outdated
| @@ -107,6 +108,31 @@ static unsigned short crc16(const void *_buffer, int len) | |||
| return crc; | |||
| } | |||
|
|
|||
| unsigned short xmodem_crc16(unsigned char * data, size_t size) | |||
There was a problem hiding this comment.
This function is missing a comment :-P
flterm.c
Outdated
| \param [in] serialfd File descriptor for serial connection. | ||
| \param [in] kernel_image File name of file to open an send. | ||
| */ | ||
| static void answer_xmodem(int serialfd, const char *kernel_image) { |
There was a problem hiding this comment.
Brace wrapping doesn't match the rest of the file
This pull request adds XMODEM support (only sends 1k packets). It is compatible with the firmware development I'm currently doing for serial upload support. Closes #2.