Added new API s to allow fragmented text to be send (IDFGH-10198) #368
Conversation
|
|
gabsuren
force-pushed
the
feature/websocket_new_api
branch
from
720fcfc to
b7b763a
Compare
gabsuren
changed the title
gabsuren
force-pushed
the
feature/websocket_new_api
branch
3 times, most recently
from
b0ca150 to
2e73302
Compare
gabsuren
changed the title
gabsuren
changed the title
components/esp_websocket_client/examples/target/main/websocket_example.c
Outdated
Show resolved
Hide resolved
components/esp_websocket_client/examples/target/main/websocket_example.c
Outdated
Show resolved
Hide resolved
|
@gabsuren please fix the commit message, so it's readable (has the title and the body). Note that this commit message would be used to compose the release notes! |
gabsuren
force-pushed
the
feature/websocket_new_api
branch
4 times, most recently
from
f97eed0 to
11e8526
Compare
…ission
Intoduced new APIs`esp_websocket_client_send_text_partial`,
`esp_websocket_client_send_bin_partial`
`esp_websocket_client_send_cont_mgs`
`esp_websocket_client_send_fin`
`esp_websocket_client_send_with_exact_opcode`
gabsuren
force-pushed
the
feature/websocket_new_api
branch
from
11e8526 to
fae80e2
Compare
| widx += wlen; | ||
| need_write = len - widx; | ||
|
|
||
| if (esp_websocket_client_send_with_exact_opcode(client, opcode | WS_TRANSPORT_OPCODES_FIN, data, len, timeout) != true) { |
There was a problem hiding this comment.
So this is the original problem causing #460
One nitpick here (sorry, missed that) is that this API shouldn't be called with_exact_opcode() if we're changing the opcode based on the fragmentation.
So after the prefix, we should at least mention it in the comments.
I'd suggest though adding one extra boolean parameter append_fin to make it cleaner.
| @@ -1092,17 +1128,33 @@ int esp_websocket_client_send_text(esp_websocket_client_handle_t client, const c | |||
| return esp_websocket_client_send_with_opcode(client, WS_TRANSPORT_OPCODES_TEXT, (const uint8_t *)data, len, timeout); | |||
| } | |||
|
|
|||
| int esp_websocket_client_send_text_partial(esp_websocket_client_handle_t client, const char *data, int len, TickType_t timeout) | |||
| { | |||
| return esp_websocket_client_send_with_exact_opcode(client, WS_TRANSPORT_OPCODES_TEXT, (const uint8_t *)data, len, timeout); | |||
There was a problem hiding this comment.
Now, this is another potential problem, we're sending something with the "exact" opcode, but the last frame should contain FIN (which is apparently missing)
and I think it's actually missing even after #461 merged
There was a problem hiding this comment.
sorry, the problem is actually here https://github.com/espressif/esp-protocols/pull/368/files#diff-53ead0f3896f79090b1ed1e79e1cca84ac4e505faa1bb76727ab856dabebc297L1092
There was a problem hiding this comment.
In order to mark the last frame I added esp_websocket_client_send_fin API.
And documented for esp_websocket_client_send_text_partial
- To mark the end of fragmented data, you should use the 'esp_websocket_client_send_fin(...)' API. This sends a FIN frame.
https://github.com/espressif/esp-protocols/blob/f62db5cfa2e64a27828a90c42ce92c2d2cb4ae89/components/esp_websocket_client/include/esp_websocket_client.h#L284C1-L289C128
There was a problem hiding this comment.
Plus we're not locking the client and perform the usually API parameters check?
| { | ||
| return esp_websocket_client_send_with_exact_opcode(client, WS_TRANSPORT_OPCODES_CONT, (const uint8_t *)data, len, timeout); | ||
| } | ||
|
|
||
| int esp_websocket_client_send_bin(esp_websocket_client_handle_t client, const char *data, int len, TickType_t timeout) | ||
| { | ||
| return esp_websocket_client_send_with_opcode(client, WS_TRANSPORT_OPCODES_BINARY, (const uint8_t *)data, len, timeout); |
There was a problem hiding this comment.
Same issue as here https://github.com/espressif/esp-protocols/pull/368/files#r1459119170
Added new APIs
esp_websocket_client_send_text_partial,esp_websocket_client_send_bin_partialesp_websocket_client_send_cont_dataesp_websocket_client_send_finesp_websocket_client_send_with_exact_opcode