OTA over HTTPS (MEGH-6086)#339
Conversation
… report functions for default OTA callback
- for checking if OTA validation is pending - for erasing OTA rollback flag from NVS
|
|
| ESP_ERROR_CHECK(wifi_init_connect()); | ||
|
|
||
| // Needs to be done after WiFi is connected. | ||
| esp_rmaker_ota_https_enable(NULL); |
There was a problem hiding this comment.
Why does https_enable needs to be done after Wi-Fi is connected?
There was a problem hiding this comment.
It requires internet connection for validating the OTA firmware. (see here)
|
|
||
| /* Actual url will be 59 bytes. */ | ||
| char ota_report_url[64]; | ||
| snprintf(ota_report_url, 64, "%s/%s", NODE_API_ENDPOINT_BASE, NODE_API_ENDPOINT_SUFFIX_REPORT); |
There was a problem hiding this comment.
You pre-calculate the required size for ota_report_url.
There was a problem hiding this comment.
Required size remains the same everytime(as URL does not change) - 59 characters.
There was a problem hiding this comment.
I see. Please add a define for this.
| /** OTA Metadata. Applicable only for OTA using Topics. Will be received (if applicable) from the backend, along with the OTA URL */ | ||
| char *metadata; | ||
| /** The Function to be called for reporting OTA status. This can be used if needed to override transport of OTA*/ | ||
| esp_rmaker_ota_report_fn_t report_fn; |
There was a problem hiding this comment.
This is a breaking change if I'm not mistaken. Please, increase the component version and update in the dependencies accordingly.
Please add a line in the CHANGES.md as well
There was a problem hiding this comment.
Updated the changelog.
I'm not familiar with versioning scheme for esp-rainmaker, can you suggest version to increment the component to?
There was a problem hiding this comment.
Do increase the minor version in idf_component.yml
There was a problem hiding this comment.
Hi @shreyash-b regarding #339 (comment) please increase the esp-rainmaker's compont minor version.
|
I'm incorporating suggestions as additional commits, will squash/rebase once everything looks good. |
vikramdattu
left a comment
vikramdattu
left a comment
There was a problem hiding this comment.
Few other minor comments. LGTM.
|
Now it should be good |
3 participants
Description
Checklist
Before submitting a Pull Request, please ensure the following: