Skip to content

Conversation

@peter-marcisovsky
Copy link
Collaborator

@peter-marcisovsky peter-marcisovsky commented Jun 6, 2025

Description

This MR adds Suspend/Resume events for HID class driver as a follow-up for the Global/Suspend resume MR in esp-idf

Changes

Added Suspend and Resume HID Host interface events

// For backward compatibility with IDF versions which do not have suspend/resume api
#ifdef USB_HOST_LIB_EVENT_FLAGS_AUTO_SUSPEND
#define HID_HOST_SUSPEND_RESUME_API_SUPPORTED
#endif

As the global Suspend issued by the USB Host Halts and flushes all EPs of all connected devices. The HID Host driver must submit the in transfer upon Resuming the root port (similar to hid_host_device_start() when a new device is connected)

Related

  • Closes Suspend/Resume support for HID Class driver

Testing

  • Added target tests to test HID Host reaction to suspend/resume events

Checklist

Before submitting a Pull Request, please ensure the following:

  • 🚨 This PR does not introduce breaking changes.
  • All CI checks (GH Actions) pass.
  • Documentation is updated as needed.
  • Tests are updated or added as necessary.
  • Code is well-commented, especially in complex areas.
  • Git history is clean — commits are squashed to the minimum necessary.

Note

Add global suspend/resume handling and events to HID Host, with state management updates and comprehensive tests.

  • HID Host core (driver):
    • Add HID_INTERFACE_STATE_SUSPENDED and last_state to track suspend/resume.
    • Implement suspend/resume helpers: hid_host_suspend_interface(), hid_host_resume_interface(), plus device-level iterators to broadcast events per interface.
    • Extend client event callback to handle USB_HOST_CLIENT_EVENT_DEV_SUSPENDED/DEV_RESUMED and log others via switch.
    • Update disable/close paths to work when interface is suspended (EP clear logic, state transitions).
  • API/Header (include/usb/hid_host.h):
    • Add conditional HID_HOST_SUSPEND_RESUME_API_SUPPORTED (gated by USB_HOST_LIB_EVENT_FLAGS_AUTO_SUSPEND).
    • Add new interface events: HID_HOST_INTERFACE_EVENT_SUSPENDED and HID_HOST_INTERFACE_EVENT_RESUMED.
  • Tests (test_app):
    • Refactor test event queue to carry both driver and interface events; initialize/cleanup queue defensively.
    • Add PM tests: suspend_resume_basic, auto_suspend_timer, resume_by_transfer_submit, sudden_disconnect_suspended_device.
    • Add error-handling tests for opening/starting devices while suspended; minor logging improvements.
    • USB lib task handles USB_HOST_LIB_EVENT_FLAGS_AUTO_SUSPEND by suspending root port.
  • Docs:
    • Update CHANGELOG.md under [unreleased] with global suspend/resume support.

Written by Cursor Bugbot for commit 2aad240. This will update automatically on new commits. Configure here.

@peter-marcisovsky peter-marcisovsky self-assigned this Jun 6, 2025
@peter-marcisovsky peter-marcisovsky added the Status: In Progress Issue is being worked on label Jun 6, 2025
@peter-marcisovsky peter-marcisovsky force-pushed the feat/usb_host_hid_suspend_resume_evts branch from 3394856 to f1525db Compare July 7, 2025 11:05
@peter-marcisovsky peter-marcisovsky force-pushed the feat/usb_host_hid_suspend_resume_evts branch 4 times, most recently from 0a327e0 to fc3c43b Compare August 8, 2025 08:00
@peter-marcisovsky peter-marcisovsky force-pushed the feat/usb_host_hid_suspend_resume_evts branch from fc3c43b to 494766b Compare August 28, 2025 08:38
@peter-marcisovsky peter-marcisovsky added stall PR is stall due to low priority, or blocked by other PR and removed Status: In Progress Issue is being worked on labels Sep 26, 2025
@peter-marcisovsky peter-marcisovsky force-pushed the feat/usb_host_hid_suspend_resume_evts branch from 494766b to 618176b Compare October 30, 2025 07:54
@peter-marcisovsky peter-marcisovsky force-pushed the feat/usb_host_hid_suspend_resume_evts branch 3 times, most recently from 034e930 to 2aad240 Compare November 19, 2025 14:06
@peter-marcisovsky peter-marcisovsky marked this pull request as ready for review November 19, 2025 14:08
@peter-marcisovsky peter-marcisovsky added Status: Reviewing Issue is being reviewed Component: usb_host Issue affects usb_host component and removed stall PR is stall due to low priority, or blocked by other PR labels Nov 19, 2025
Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the final PR Bugbot will review for you during this billing cycle

Your free Bugbot reviews will reset on November 20

Details

Your team is on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle for each member of your team.

To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.


// start data transfer
HID_RETURN_ON_ERROR( usb_host_transfer_submit(iface->in_xfer), "Unable to start data transfer");
return ESP_OK;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Transfer resubmitted for interfaces not in ACTIVE state

The hid_host_resume_interface function unconditionally resubmits the transfer if in_xfer is not NULL, regardless of the interface's previous state. This causes transfers to be submitted for interfaces that were in HID_INTERFACE_STATE_READY state before suspension, violating the state machine where transfers should only be active when the interface is in HID_INTERFACE_STATE_ACTIVE state. The transfer should only be resubmitted when last_state equals HID_INTERFACE_STATE_ACTIVE.

Fix in Cursor Fix in Web


if (hid_iface_curr->parent && (hid_iface_curr->parent->dev_addr == hid_device->dev_addr)) {
hid_host_suspend_interface(hid_iface_curr, false);
hid_host_user_interface_callback(hid_iface_curr, HID_HOST_INTERFACE_EVENT_SUSPENDED);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Unchecked return values in suspend/resume device functions

The hid_host_suspend_interface return value is ignored in hid_host_device_suspended, allowing the suspended event callback to be delivered even if the suspend operation failed. This creates inconsistency between the actual interface state and the events delivered to the user. The same issue exists in hid_host_device_resumed at lines 691-692 where hid_host_resume_interface errors are ignored.

Fix in Cursor Fix in Web

    - usb_host lib supports global suspend and resume
    - backward compatibility with older IDF releases
    - HID Host target tests
@peter-marcisovsky peter-marcisovsky force-pushed the feat/usb_host_hid_suspend_resume_evts branch from 2aad240 to cfb9e08 Compare November 25, 2025 12:40
Copy link
Contributor

@roma-jam roma-jam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One more question from my side for now:

@roma-jam roma-jam self-requested a review November 26, 2025 17:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Component: usb_host Issue affects usb_host component Status: Reviewing Issue is being reviewed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants