Closed devyte closed 4 years ago
Can yougive me a hint on that? Where can I obtain the size of that buffer? Is the "lwip buffer configuration" what is being defined by the IDE as "-DTCP_MSS=536" ? When I tested my code, I noticed that when reading from a WiFiClient, WiFiClient::available() returns
[FTP] Transfer 536 bytes net->FS
[FTP] Transfer 2048 bytes net->FS
[FTP] Transfer 1704 bytes net->FS
[FTP] Transfer 536 bytes net->FS
[FTP] Transfer 1608 bytes net->FS
[FTP] Transfer 2048 bytes net->FS
[FTP] Transfer 1704 bytes net->FS
[FTP] Transfer 536 bytes net->FS
[FTP] Transfer 536 bytes net->FS
[FTP] Transfer 2048 bytes net->FS
[FTP] Transfer 632 bytes net->FS
[FTP] Transfer 1072 bytes net->FS
[FTP] Transfer 1072 bytes net->FS
[FTP] Transfer 2048 bytes net->FS
[FTP] Transfer 1704 bytes net->FS
[FTP] Transfer 1072 bytes net->FS
quite a range of available bytes to read.
Is the "lwip buffer configuration" what is being defined by the IDE as "-DTCP_MSS=536"
Correct. You can see in the above numbers that they tend to be a multiple of 536, which is the lwip MSS. The cases where it's not a multiple aren't common. The case of 536 is meant to optimize for low mem. If a user wants speed, he should build with the high bandwidth lwip variant, which increases the lwip internal buffer to the max possible. This is why your own buffer should be tied to this.
Ok, than using a buffer of exactly TCP_MSS bytes seems the most straight forward thing to do. Changed my code accordingly
You implemented 4xMSS. Foe high bandwidth that gives about 6KB. It looks correct to me, you can close this.
I did some speed comparisons and 4xMSS really does not make the difference (<10%) so I believe setting it to 1MSS seems rational, given the user has the choice to opt for a performance option with larger buffers. Changed it to 1MSS.
I see one buffer of size 2048 and one buffer of 32KB. Neither makes much sense. The buffers should be of size 1x or 2x the size of the lwip buffer configuration (low mem vs. high speed). Support for lwip 1.x can be ignored, it will be pulled put in the near future. Requirement of lwip 2.x can be stated in the Readme. About that 32KB buffer, that one really doesn't make sense, especially if support for encryption is to be implemented (requires a lot of heap).