diff --git a/usb/usb_host_msc/CHANGELOG.md b/usb/usb_host_msc/CHANGELOG.md index e89d3049a1..9d1e866187 100644 --- a/usb/usb_host_msc/CHANGELOG.md +++ b/usb/usb_host_msc/CHANGELOG.md @@ -2,6 +2,7 @@ - Fix `msc_host_get_device_info` for devices without Serial Number string descriptor https://github.com/espressif/esp-idf/issues/12163 - Fix regression from version 1.1.0 that files could not be opened in PSRAM https://github.com/espressif/idf-extra-components/issues/202 +- Fix MSC driver event handling without background task ## 1.1.0 - yanked diff --git a/usb/usb_host_msc/include/usb/msc_host.h b/usb/usb_host_msc/include/usb/msc_host.h index e3017893e4..6562fd0291 100644 --- a/usb/usb_host_msc/include/usb/msc_host.h +++ b/usb/usb_host_msc/include/usb/msc_host.h @@ -142,8 +142,15 @@ __attribute__((deprecated("use API from esp_private/msc_scsi_bot.h"))); /** * @brief Handle MSC HOST events. * - * @param[in] timeout_ms Timeout in miliseconds - * @return esp_err_t + * If MSC Host install was made with create_background_task=false configuration, + * application needs to handle USB Host events itself. + * Do not use if MSC host install was made with create_background_task=true configuration + * + * @param[in] timeout Timeout in FreeRTOS tick + * @return + * - ESP_OK: All events handled + * - ESP_ERR_TIMEOUT: No events handled within the timeout + * - ESP_FAIL: Event handling finished, driver uninstalled. You do not have to call this function further */ esp_err_t msc_host_handle_events(uint32_t timeout_ms); @@ -171,8 +178,8 @@ esp_err_t msc_host_print_descriptors(msc_host_device_handle_t device); * * @param[in] device Handle of MSC device * @return - * - ESP_OK: The device was recovered from reset - * - ESP_FAI: Recovery unsuccessful, might indicate broken device + * - ESP_OK: The device was recovered from reset + * - ESP_FAIL: Recovery unsuccessful, might indicate broken device */ esp_err_t msc_host_reset_recovery(msc_host_device_handle_t device); diff --git a/usb/usb_host_msc/src/msc_host.c b/usb/usb_host_msc/src/msc_host.c index 04cab199e8..adeaffd216 100644 --- a/usb/usb_host_msc/src/msc_host.c +++ b/usb/usb_host_msc/src/msc_host.c @@ -94,6 +94,7 @@ typedef struct { void *user_arg; SemaphoreHandle_t all_events_handled; volatile bool end_client_event_handling; + bool event_handling_started; STAILQ_HEAD(devices, msc_host_device) devices_tailq; } msc_driver_t; @@ -321,17 +322,34 @@ static bool is_mass_storage_device(uint8_t dev_addr) return is_msc_device; } -static void event_handler_task(void *arg) + + +esp_err_t msc_host_handle_events(uint32_t timeout) { - while (1) { - usb_host_client_handle_events(s_msc_driver->client_handle, portMAX_DELAY); + MSC_RETURN_ON_FALSE(s_msc_driver != NULL, ESP_ERR_INVALID_STATE); - if (s_msc_driver->end_client_event_handling) { - break; - } + ESP_LOGV(TAG, "USB MSC handling"); + s_msc_driver->event_handling_started = true; + esp_err_t ret = usb_host_client_handle_events(s_msc_driver->client_handle, timeout); + if (s_msc_driver->end_client_event_handling) { + xSemaphoreGive(s_msc_driver->all_events_handled); + return ESP_FAIL; } - ESP_ERROR_CHECK( usb_host_client_deregister(s_msc_driver->client_handle) ); - xSemaphoreGive(s_msc_driver->all_events_handled); + return ret; +} +/** + * @brief USB Client Event handler + * + * Handle all USB client events such as USB transfers and connections/disconnections + * + * @param[in] arg Argument, not used + */ +static void event_handler_task(void *arg) +{ + ESP_LOGD(TAG, "USB HID handling start"); + while (msc_host_handle_events(portMAX_DELAY) == ESP_OK) { + } + ESP_LOGD(TAG, "USB HID handling stop"); vTaskDelete(NULL); } @@ -445,9 +463,13 @@ esp_err_t msc_host_uninstall(void) s_msc_driver->end_client_event_handling = true; MSC_EXIT_CRITICAL(); - ESP_ERROR_CHECK( usb_host_client_unblock(s_msc_driver->client_handle) ); - xSemaphoreTake(s_msc_driver->all_events_handled, portMAX_DELAY); + if (s_msc_driver->event_handling_started) { + ESP_ERROR_CHECK( usb_host_client_unblock(s_msc_driver->client_handle) ); + // In case the event handling started, we must wait until it finishes + xSemaphoreTake(s_msc_driver->all_events_handled, portMAX_DELAY); + } vSemaphoreDelete(s_msc_driver->all_events_handled); + ESP_ERROR_CHECK( usb_host_client_deregister(s_msc_driver->client_handle) ); free(s_msc_driver); s_msc_driver = NULL; return ESP_OK; @@ -515,13 +537,6 @@ esp_err_t msc_host_write_sector(msc_host_device_handle_t device, size_t sector, return scsi_cmd_write10(dev, data, sector, 1, dev->disk.block_size); } -esp_err_t msc_host_handle_events(uint32_t timeout_ms) -{ - MSC_RETURN_ON_FALSE(s_msc_driver != NULL, ESP_ERR_INVALID_STATE); - - return usb_host_client_handle_events(s_msc_driver->client_handle, pdMS_TO_TICKS(timeout_ms)); -} - static void copy_string_desc(wchar_t *dest, const usb_str_desc_t *src) { if (dest == NULL) { diff --git a/usb/usb_host_msc/test/test_msc.c b/usb/usb_host_msc/test/test_msc.c index 3d9fdccefd..260eb7849f 100644 --- a/usb/usb_host_msc/test/test_msc.c +++ b/usb/usb_host_msc/test/test_msc.c @@ -122,6 +122,22 @@ static void handle_usb_events(void *args) vTaskDelete(NULL); } +/** + * @brief MSC driver handling task + * + * This task is only used if the MSC driver was installed with no background task + * + * @param[in] args Not used + */ +static void msc_task(void *args) +{ + ESP_LOGI(TAG, "USB MSC handling start"); + while (msc_host_handle_events(portMAX_DELAY) == ESP_OK) { + } + ESP_LOGI(TAG, "USB MSC handling stop"); + vTaskDelete(NULL); +} + #if ESP_IDF_VERSION < ESP_IDF_VERSION_VAL(5, 0, 0) static void check_file_content(const char *file_path, const char *expected) { @@ -166,7 +182,7 @@ static void check_sudden_disconnect(void) fclose(file); } -static void msc_setup(void) +static void msc_test_init(void) { BaseType_t task_created; @@ -190,15 +206,10 @@ static void msc_setup(void) task_created = xTaskCreatePinnedToCore(handle_usb_events, "usb_events", 2 * 2048, NULL, 2, NULL, 0); TEST_ASSERT(task_created); +} - const msc_host_driver_config_t msc_config = { - .create_backround_task = true, - .callback = msc_event_cb, - .stack_size = 4096, - .task_priority = 5, - }; - ESP_OK_ASSERT( msc_host_install(&msc_config) ); - +static void msc_test_wait_and_install_device(void) +{ ESP_LOGI(TAG, "Waiting for USB stick to be connected"); msc_host_event_t app_event; xQueueReceive(app_queue, &app_event, portMAX_DELAY); @@ -209,16 +220,32 @@ static void msc_setup(void) ESP_OK_ASSERT( msc_host_vfs_register(device, "/usb", &mount_config, &vfs_handle) ); } -static void msc_teardown(void) +static void msc_setup(void) { - vTaskDelay(10); // Wait to finish any ongoing USB operations + msc_test_init(); + const msc_host_driver_config_t msc_config = { + .create_backround_task = true, + .callback = msc_event_cb, + .stack_size = 4096, + .task_priority = 5, + }; + ESP_OK_ASSERT( msc_host_install(&msc_config) ); + msc_test_wait_and_install_device(); +} +static void msc_test_uninstall_device(void) +{ ESP_OK_ASSERT( msc_host_vfs_unregister(vfs_handle) ); ESP_OK_ASSERT( msc_host_uninstall_device(device) ); +} + +static void msc_test_deinit(void) +{ ESP_OK_ASSERT( msc_host_uninstall() ); xSemaphoreTake(ready_to_deinit_usb, portMAX_DELAY); vSemaphoreDelete(ready_to_deinit_usb); + vTaskDelay(10); // Wait to finish any ongoing USB operations ESP_OK_ASSERT( usb_host_uninstall() ); //Tear down USB PHY ESP_OK_ASSERT(usb_del_phy(phy_hdl)); @@ -228,6 +255,12 @@ static void msc_teardown(void) vTaskDelay(10); // Wait for FreeRTOS to clean up deleted tasks } +static void msc_teardown(void) +{ + msc_test_uninstall_device(); + msc_test_deinit(); +} + static void write_read_sectors(void) { uint8_t write_data[DISK_BLOCK_SIZE]; @@ -453,6 +486,29 @@ TEST_CASE("device_info", "[usb_msc]") print_device_info(&info); } +/** + * @brief USB MSC driver with no background task + * + * Install the driver without background task + * and make sure that everything works + */ +TEST_CASE("no_background_task", "[usb_msc]") +{ + msc_test_init(); + const msc_host_driver_config_t msc_config = { + .create_backround_task = false, + .callback = msc_event_cb, + .stack_size = 4096, + .task_priority = 5, + }; + ESP_OK_ASSERT( msc_host_install(&msc_config) ); + BaseType_t task_created = xTaskCreatePinnedToCore(msc_task, "msc_events", 2 * 2048, NULL, 2, NULL, 0); + TEST_ASSERT(task_created); + msc_test_wait_and_install_device(); + write_read_sectors(); // Do some dummy operations + msc_teardown(); +} + /** * @brief USB MSC Device Mock *