-
Notifications
You must be signed in to change notification settings - Fork 135
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Added new API s to allow fragmented text to be send (IDFGH-10198) #368
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,5 @@ | ||
/* | ||
* SPDX-FileCopyrightText: 2015-2022 Espressif Systems (Shanghai) CO LTD | ||
* SPDX-FileCopyrightText: 2015-2023 Espressif Systems (Shanghai) CO LTD | ||
* | ||
* SPDX-License-Identifier: Apache-2.0 | ||
*/ | ||
|
@@ -545,6 +545,42 @@ static esp_err_t esp_websocket_client_create_transport(esp_websocket_client_hand | |
return ESP_OK; | ||
} | ||
|
||
static bool esp_websocket_client_send_with_exact_opcode(esp_websocket_client_handle_t client, ws_transport_opcodes_t opcode, const uint8_t *data, int len, TickType_t timeout) | ||
{ | ||
int ret = -1; | ||
int need_write = len; | ||
int wlen = 0, widx = 0; | ||
|
||
while (widx < len || opcode) { // allow for sending "current_opcode" only message with len==0 | ||
if (need_write > client->buffer_size) { | ||
need_write = client->buffer_size; | ||
} | ||
memcpy(client->tx_buffer, data + widx, need_write); | ||
// send with ws specific way and specific opcode | ||
wlen = esp_transport_ws_send_raw(client->transport, opcode, (char *)client->tx_buffer, need_write, | ||
(timeout == portMAX_DELAY) ? -1 : timeout * portTICK_PERIOD_MS); | ||
if (wlen < 0 || (wlen == 0 && need_write != 0)) { | ||
ret = wlen; | ||
esp_websocket_free_buf(client, true); | ||
esp_tls_error_handle_t error_handle = esp_transport_get_error_handle(client->transport); | ||
if (error_handle) { | ||
esp_websocket_client_error(client, "esp_transport_write() returned %d, transport_error=%s, tls_error_code=%i, tls_flags=%i, errno=%d", | ||
ret, esp_err_to_name(error_handle->last_error), error_handle->esp_tls_error_code, | ||
error_handle->esp_tls_flags, errno); | ||
} else { | ||
esp_websocket_client_error(client, "esp_transport_write() returned %d, errno=%d", ret, errno); | ||
} | ||
esp_websocket_client_abort_connection(client, WEBSOCKET_ERROR_TYPE_TCP_TRANSPORT); | ||
return false; | ||
} | ||
opcode = 0; | ||
widx += wlen; | ||
need_write = len - widx; | ||
} | ||
esp_websocket_free_buf(client, true); | ||
return true; | ||
} | ||
|
||
esp_websocket_client_handle_t esp_websocket_client_init(const esp_websocket_client_config_t *config) | ||
{ | ||
esp_websocket_client_handle_t client = calloc(1, sizeof(struct esp_websocket_client)); | ||
|
@@ -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); | ||
} | ||
|
||
int esp_websocket_client_send_cont_msg(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_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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same issue as here https://github.com/espressif/esp-protocols/pull/368/files#r1459119170 |
||
} | ||
|
||
int esp_websocket_client_send_with_opcode(esp_websocket_client_handle_t client, ws_transport_opcodes_t opcode, const uint8_t *data, int len, TickType_t timeout) | ||
int esp_websocket_client_send_bin_partial(esp_websocket_client_handle_t client, const char *data, int len, TickType_t timeout) | ||
{ | ||
int need_write = len; | ||
int wlen = 0, widx = 0; | ||
int ret = ESP_FAIL; | ||
return esp_websocket_client_send_with_exact_opcode(client, WS_TRANSPORT_OPCODES_BINARY, (const uint8_t *)data, len, timeout); | ||
} | ||
|
||
int esp_websocket_client_send_fin(esp_websocket_client_handle_t client, TickType_t timeout) | ||
{ | ||
return esp_websocket_client_send_with_exact_opcode(client, WS_TRANSPORT_OPCODES_FIN, NULL, 0, timeout); | ||
} | ||
|
||
int esp_websocket_client_send_with_opcode(esp_websocket_client_handle_t client, ws_transport_opcodes_t opcode, const uint8_t *data, int len, TickType_t timeout) | ||
{ | ||
if (client == NULL || len < 0 || (data == NULL && len > 0)) { | ||
ESP_LOGE(TAG, "Invalid arguments"); | ||
return ESP_FAIL; | ||
|
@@ -1126,41 +1178,13 @@ int esp_websocket_client_send_with_opcode(esp_websocket_client_handle_t client, | |
ESP_LOGE(TAG, "Failed to setup tx buffer"); | ||
goto unlock_and_return; | ||
} | ||
uint32_t current_opcode = opcode; | ||
while (widx < len || current_opcode) { // allow for sending "current_opcode" only message with len==0 | ||
if (need_write > client->buffer_size) { | ||
need_write = client->buffer_size; | ||
} else { | ||
current_opcode |= WS_TRANSPORT_OPCODES_FIN; | ||
} | ||
memcpy(client->tx_buffer, data + widx, need_write); | ||
// send with ws specific way and specific opcode | ||
wlen = esp_transport_ws_send_raw(client->transport, current_opcode, (char *)client->tx_buffer, need_write, | ||
(timeout == portMAX_DELAY) ? -1 : timeout * portTICK_PERIOD_MS); | ||
if (wlen < 0 || (wlen == 0 && need_write != 0)) { | ||
ret = wlen; | ||
esp_websocket_free_buf(client, true); | ||
esp_tls_error_handle_t error_handle = esp_transport_get_error_handle(client->transport); | ||
if (error_handle) { | ||
esp_websocket_client_error(client, "esp_transport_write() returned %d, transport_error=%s, tls_error_code=%i, tls_flags=%i, errno=%d", | ||
ret, esp_err_to_name(error_handle->last_error), error_handle->esp_tls_error_code, | ||
error_handle->esp_tls_flags, errno); | ||
} else { | ||
esp_websocket_client_error(client, "esp_transport_write() returned %d, errno=%d", ret, errno); | ||
} | ||
esp_websocket_client_abort_connection(client, WEBSOCKET_ERROR_TYPE_TCP_TRANSPORT); | ||
goto unlock_and_return; | ||
} | ||
current_opcode = 0; | ||
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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So this is the original problem causing #460 One nitpick here (sorry, missed that) is that this API shouldn't be called |
||
ESP_LOGE(TAG, "Failed to send the buffer"); | ||
goto unlock_and_return; | ||
} | ||
ret = widx; | ||
esp_websocket_free_buf(client, true); | ||
unlock_and_return: | ||
xSemaphoreGiveRecursive(client->lock); | ||
return ret; | ||
return ESP_FAIL; | ||
} | ||
|
||
bool esp_websocket_client_is_connected(esp_websocket_client_handle_t client) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sorry, the problem is actually here https://github.com/espressif/esp-protocols/pull/368/files#diff-53ead0f3896f79090b1ed1e79e1cca84ac4e505faa1bb76727ab856dabebc297L1092
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Plus we're not locking the client and perform the usually API parameters check?