esp_http_client: add head method support #2093
Conversation
|
|
|
I think it would be more helpful to add some description on this PR. |
| @@ -690,6 +691,11 @@ static int esp_http_client_get_data(esp_http_client_handle_t client) | |||
| if (client->state < HTTP_STATE_RES_COMPLETE_HEADER) { | |||
| return ESP_FAIL; | |||
| } | |||
|
|
|||
| if (client->connection_info.method == HTTP_METHOD_HEAD) { | |||
| return 0; | |||
There was a problem hiding this comment.
0 means ESP_OK? It would be better to use more meaningful definition instead of 0.
There was a problem hiding this comment.
No it means 0 bytes read. The return of the function is the number of bytes read. The above ESP_FAIL return seems to rely on the fact that currently ESP_FAIL is #define'd to -1. I wavered in putting the HEAD check below in esp_http_client_perform and just skipping looking at the body at all, but then the state check above would need to move as well, and the correct data length for a HEAD request is 0 since the RFC states that it "must not return a message-body in the response", so the esp_http_client_get_data seemed more appropriate.
There was a problem hiding this comment.
Thanks for explaining your intention. This function's return type is int. If a function's return type is esp_err_t then we should consider using ESP_OK.
| esp_http_client_get_content_length(client)); | ||
| } else { | ||
| ESP_LOGE(TAG, "HTTP HEAD request failed: %s", esp_err_to_name(err)); | ||
| } |
|
Thanks for the useful method, LGTM |
No description provided.