Conversation
…self-test-refactor
WantClue
left a comment
WantClue
left a comment
There was a problem hiding this comment.
lgtm, i'll do some testing and report back before merging
| if (GLOBAL_STATE->DEVICE_CONFIG.DS4432U && DS4432U_test() != ESP_OK) { | ||
| ESP_LOGE(TAG, "DS4432 test failed!"); | ||
| self_test_show_message(GLOBAL_STATE, "DS4432U:FAIL"); | ||
| tests_done(GLOBAL_STATE, false); |
There was a problem hiding this comment.
you're missing a return here
| snprintf(error_buf, 20, "VCORE:PWR FAULT"); | ||
| display_msg(error_buf, GLOBAL_STATE); | ||
| self_test_show_message(GLOBAL_STATE, "VCORE:PWR FAULT"); | ||
| tests_done(GLOBAL_STATE, false); |
There was a problem hiding this comment.
missing return here as well
| snprintf(error_buf, 20, "ASIC:FAIL %d CHIPS", chips_detected); | ||
| display_msg(error_buf, GLOBAL_STATE); | ||
| self_test_show_message(GLOBAL_STATE, "VOLTAGE:FAIL"); | ||
| tests_done(GLOBAL_STATE, false); |
There was a problem hiding this comment.
missing return here as well
| } | ||
|
|
||
| static void display_msg(char * msg, GlobalState * GLOBAL_STATE) | ||
| void self_test_show_message(void * pvParameters, char * msg) |
There was a problem hiding this comment.
This function stores msg directly in GLOBAL_STATE->SELF_TEST_MODULE.message. Throughout the code, this is called with stack-allocated buffers like logString (lines 316, 327, 348) and error_buf (line 382). Once the buffer is reused or goes out of scope, the stored pointer becomes invalid.
+static char self_test_message_buf[64];
+
void self_test_show_message(void * pvParameters, char * msg)
{
GlobalState * GLOBAL_STATE = (GlobalState *) pvParameters;
if (GLOBAL_STATE->SELF_TEST_MODULE.is_active) {
- GLOBAL_STATE->SELF_TEST_MODULE.message = msg;
+ strncpy(self_test_message_buf, msg, sizeof(self_test_message_buf) - 1);
+ self_test_message_buf[sizeof(self_test_message_buf) - 1] = '\0';
+ GLOBAL_STATE->SELF_TEST_MODULE.message = self_test_message_buf;
vTaskDelay(10 / portTICK_PERIOD_MS);
}
}
main/self_test/self_test.c
Outdated
| { | ||
| ESP_LOGI(TAG, "Long press detected..."); | ||
| // Give the semaphore back | ||
| xSemaphoreGive(longPressSemaphore); |
There was a problem hiding this comment.
add a NULL check or initialize the semaphore earlier in self_test_init
| { | ||
| GlobalState * GLOBAL_STATE = (GlobalState *) pvParameters; | ||
|
|
||
| esp_vfs_spiffs_conf_t conf = {.base_path = "", .partition_label = NULL, .max_files = 5, .format_if_mount_failed = false}; |
There was a problem hiding this comment.
afaik ESP-IDF does require a non empty path starting with "/" e.g. "/spiffs"
There was a problem hiding this comment.
This code is just moved out of http_server.c, unchanged?
…self-test-refactor
* Fix recovery mode and enable web if system init fails Follow-up of #1615 * Fix main.c * Add countdown on display
2 participants
This refactors self-test to use the normal startup code of the firmware, minus Wi-Fi, fan controller and stratum task.