From ec2f08ace68aadd56517389db1c20f1af6abf149 Mon Sep 17 00:00:00 2001 From: Darian Leung Date: Fri, 23 Feb 2024 04:25:10 +0800 Subject: [PATCH] refactor(usb): Update USBH event callback arguments This commit does the following: - Updates the USBH event callback arguments to now pass a usbh_event_data_t which can contain different data for each event - Updated event names --- components/usb/private_include/usbh.h | 30 +++++++++++++++++++++------ components/usb/usb_host.c | 21 +++++++------------ components/usb/usbh.c | 28 +++++++++++++++++++------ 3 files changed, 54 insertions(+), 25 deletions(-) diff --git a/components/usb/private_include/usbh.h b/components/usb/private_include/usbh.h index 0fafea2974..c14eee8658 100644 --- a/components/usb/private_include/usbh.h +++ b/components/usb/private_include/usbh.h @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2015-2022 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2015-2024 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -29,12 +29,31 @@ typedef struct usbh_ep_handle_s *usbh_ep_handle_t; // ----------------------- Events -------------------------- +/** + * @brief Enumerator for various USBH events + */ typedef enum { - USBH_EVENT_DEV_NEW, /**< A new device has been enumerated and added to the device pool */ + USBH_EVENT_NEW_DEV, /**< A new device has been enumerated and added to the device pool */ USBH_EVENT_DEV_GONE, /**< A device is gone. Clients should close the device */ - USBH_EVENT_DEV_ALL_FREE, /**< All devices have been freed */ + USBH_EVENT_ALL_FREE, /**< All devices have been freed */ } usbh_event_t; +/** + * @brief Event data object for USBH events + */ +typedef struct { + usbh_event_t event; + union { + struct { + uint8_t dev_addr; + } new_dev_data; + struct { + uint8_t dev_addr; + usb_device_handle_t dev_hdl; + } dev_gone_data; + }; +} usbh_event_data_t; + /** * @brief Endpoint events * @@ -109,9 +128,8 @@ typedef void (*usbh_ctrl_xfer_cb_t)(usb_device_handle_t dev_hdl, urb_t *urb, voi * @brief Callback used to indicate that the USBH has an event * * @note This callback is called from within usbh_process() - * @note On a USBH_EVENT_DEV_ALL_FREE event, the dev_hdl argument is set to NULL */ -typedef void (*usbh_event_cb_t)(usb_device_handle_t dev_hdl, usbh_event_t usbh_event, void *arg); +typedef void (*usbh_event_cb_t)(usbh_event_data_t *event_data, void *arg); /** * @brief Callback used by the USBH to request actions from the Hub driver @@ -497,7 +515,7 @@ esp_err_t usbh_hub_enum_fill_str_desc(usb_device_handle_t dev_hdl, const usb_str /** * @brief Indicate the device enumeration is completed * - * This will all the device to be opened by clients, and also trigger a USBH_EVENT_DEV_NEW event. + * This will allow the device to be opened by clients, and also trigger a USBH_EVENT_NEW_DEV event. * * @note Hub Driver only * @note Must call in sequence diff --git a/components/usb/usb_host.c b/components/usb/usb_host.c index 8328927952..080fd8a36d 100644 --- a/components/usb/usb_host.c +++ b/components/usb/usb_host.c @@ -275,33 +275,28 @@ static void ctrl_xfer_callback(usb_device_handle_t dev_hdl, urb_t *urb, void *ar HOST_EXIT_CRITICAL(); } -static void dev_event_callback(usb_device_handle_t dev_hdl, usbh_event_t usbh_event, void *arg) +static void usbh_event_callback(usbh_event_data_t *event_data, void *arg) { - // Check usbh_event. The data type of event_arg depends on the type of event - switch (usbh_event) { - case USBH_EVENT_DEV_NEW: { + switch (event_data->event) { + case USBH_EVENT_NEW_DEV: { // Prepare a NEW_DEV client event message, the send it to all clients - uint8_t dev_addr; - ESP_ERROR_CHECK(usbh_dev_get_addr(dev_hdl, &dev_addr)); usb_host_client_event_msg_t event_msg = { .event = USB_HOST_CLIENT_EVENT_NEW_DEV, - .new_dev.address = dev_addr, + .new_dev.address = event_data->new_dev_data.dev_addr, }; send_event_msg_to_clients(&event_msg, true, 0); break; } case USBH_EVENT_DEV_GONE: { // Prepare event msg, send only to clients that have opened the device - uint8_t dev_addr; - ESP_ERROR_CHECK(usbh_dev_get_addr(dev_hdl, &dev_addr)); usb_host_client_event_msg_t event_msg = { .event = USB_HOST_CLIENT_EVENT_DEV_GONE, - .dev_gone.dev_hdl = dev_hdl, + .dev_gone.dev_hdl = event_data->dev_gone_data.dev_hdl, }; - send_event_msg_to_clients(&event_msg, false, dev_addr); + send_event_msg_to_clients(&event_msg, false, event_data->dev_gone_data.dev_addr); break; } - case USBH_EVENT_DEV_ALL_FREE: { + case USBH_EVENT_ALL_FREE: { // Notify the lib handler that all devices are free HOST_ENTER_CRITICAL(); p_host_lib_obj->dynamic.lib_event_flags |= USB_HOST_LIB_EVENT_FLAGS_ALL_FREE; @@ -401,7 +396,7 @@ esp_err_t usb_host_install(const usb_host_config_t *config) .proc_req_cb_arg = NULL, .ctrl_xfer_cb = ctrl_xfer_callback, .ctrl_xfer_cb_arg = NULL, - .event_cb = dev_event_callback, + .event_cb = usbh_event_callback, .event_cb_arg = NULL, }; ret = usbh_install(&usbh_config); diff --git a/components/usb/usbh.c b/components/usb/usbh.c index faceb904bd..b4297bbd88 100644 --- a/components/usb/usbh.c +++ b/components/usb/usbh.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2015-2023 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2015-2024 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -35,7 +35,7 @@ typedef enum { DEV_ACTION_FREE_AND_RECOVER = (1 << 5), // Free the device object, but send a USBH_HUB_REQ_PORT_RECOVER request afterwards. DEV_ACTION_FREE = (1 << 6), // Free the device object DEV_ACTION_PORT_DISABLE = (1 << 7), // Request the hub driver to disable the port of the device - DEV_ACTION_PROP_NEW = (1 << 8), // Propagate a USBH_EVENT_DEV_NEW event + DEV_ACTION_PROP_NEW = (1 << 8), // Propagate a USBH_EVENT_NEW_DEV event } dev_action_t; typedef struct device_s device_t; @@ -500,7 +500,14 @@ static inline void handle_prop_gone_evt(device_t *dev_obj) { // Flush EP0's pipe. Then propagate a USBH_EVENT_DEV_GONE event ESP_LOGE(USBH_TAG, "Device %d gone", dev_obj->constant.address); - p_usbh_obj->constant.event_cb((usb_device_handle_t)dev_obj, USBH_EVENT_DEV_GONE, p_usbh_obj->constant.event_cb_arg); + usbh_event_data_t event_data = { + .event = USBH_EVENT_DEV_GONE, + .dev_gone_data = { + .dev_addr = dev_obj->constant.address, + .dev_hdl = (usb_device_handle_t)dev_obj, + }, + }; + p_usbh_obj->constant.event_cb(&event_data, p_usbh_obj->constant.event_cb_arg); } static void handle_free_and_recover(device_t *dev_obj, bool recover_port) @@ -526,10 +533,13 @@ static void handle_free_and_recover(device_t *dev_obj, bool recover_port) xSemaphoreGive(p_usbh_obj->constant.mux_lock); device_free(dev_obj); - // If all devices have been freed, propagate a USBH_EVENT_DEV_ALL_FREE event + // If all devices have been freed, propagate a USBH_EVENT_ALL_FREE event if (all_free) { ESP_LOGD(USBH_TAG, "Device all free"); - p_usbh_obj->constant.event_cb((usb_device_handle_t)NULL, USBH_EVENT_DEV_ALL_FREE, p_usbh_obj->constant.event_cb_arg); + usbh_event_data_t event_data = { + .event = USBH_EVENT_ALL_FREE, + }; + p_usbh_obj->constant.event_cb(&event_data, p_usbh_obj->constant.event_cb_arg); } // Check if we need to recover the device's port if (recover_port) { @@ -547,7 +557,13 @@ static inline void handle_port_disable(device_t *dev_obj) static inline void handle_prop_new_evt(device_t *dev_obj) { ESP_LOGD(USBH_TAG, "New device %d", dev_obj->constant.address); - p_usbh_obj->constant.event_cb((usb_device_handle_t)dev_obj, USBH_EVENT_DEV_NEW, p_usbh_obj->constant.event_cb_arg); + usbh_event_data_t event_data = { + .event = USBH_EVENT_NEW_DEV, + .new_dev_data = { + .dev_addr = dev_obj->constant.address, + }, + }; + p_usbh_obj->constant.event_cb(&event_data, p_usbh_obj->constant.event_cb_arg); } // ------------------------------------------------- USBH Functions ----------------------------------------------------