Experimental support for CH340, FT232RL & CP2101 devices #460
Conversation
|
Looks good to me! |
|
Does the C header need updating? |
|
I fixed an issue with CH340 clones and I'm adding CH341 today. I'll also update the header file. |
|
Is it good to merge? |
Experimental support for CH340, FT232RL & CP2101 devices. Tested ESP8266 D1 mini clone, ESP-WROOM-32 board and FTDI232 boards Currently baud rates are table driven so baud rates are limited to: 9600, 19200, 38400, 57600 & 115200 but can be easily expanded.
Updated baud settings table to fix CH340 (clones should now work)
|
rebased and launched CI just to check |
| ring_buf_ctrl_t rx_buf; | ||
| ring_buf_ctrl_t tx_buf; | ||
| srl_error_t err; | ||
| uint8_t reserved[16]; |
There was a problem hiding this comment.
I'm a little worried about this change - does this break out backwards compatibility with user allocated structures? @commandblockguy @tr1p1ea
The issue is someone using a program compiled for an old version of the lib, and using a newer lib.
There was a problem hiding this comment.
Could new fields be added at the end instead, and the reserved array shortened?
There was a problem hiding this comment.
This is why the structure should be big enough to support forwards compatibility and the addition of fields - also moving of fields breaks backwards compatibility for all programs too. See how fatdrvce handles this for reference.
There was a problem hiding this comment.
@adriweb no, because the structure is user-allocated. so using a newer library would corrupt memory.
There was a problem hiding this comment.
Memory corruption could only happen if the total size of the struct changes, right?
In hindsight, yeah, it probably wasn't a good idea to make the internal structures visible - I mostly only did that to ease debugging, and didn't get around to removing them before actually releasing it.
Aside from the device field, I don't imagine that people have used the struct fields - it would probably be a good idea to deprecate them.
|
@tr1p1ea what is the status of this PR? Also @commandblockguy, which struct fields can we deprecate to make everything reserved? |
|
We can probably deprecate everything except for the USB device handle. |
|
Great :) |
|
After testing I've come to the conclusion that there is some further work to do to improve compatibility with some devices. Should I recall this PR until that's complete? Also did you want the struct optimisation to be part of this too? |
mateoconlechuga
force-pushed
the
master
branch
from
4d41a4a to
483e052
Compare
|
@tr1p1ea yes to the struct optimization and any fun new status updates for us :) It's fine to just keep it in this PR |
mateoconlechuga
force-pushed
the
master
branch
2 times, most recently
from
88cf7d0 to
c6152ff
Compare
Experimental support for CH340, FT232RL & CP2102 devices.
Tested with ESP8266 D1 mini clone, ESP-WROOM-32 board and FTDI232 boards.
Currently baud rates are table driven so baud rates are limited to: 9600, 19200, 38400, 57600 & 115200 but can be easily expanded.