feature(usb_host_hid): USB Host HID (Human Interface Device) Driver v.2.0.0 #214

roma-jam · 2023-07-20T11:29:05Z

Checklist

Component contains License
Component contains README.md
Component contains idf_component.yml file with url field defined
Component was added to upload job
Component was added to build job
Optional: Component contains unit tests
CI passing

Change description

Changed the logic of user notification procedure (merge two callbacks in one, extend with new events, add esp_event_loop to simplify application events handling procedure and exclude external task to handle HID Driver events).
Optimized memory and pipes (in USB Host library) usage. Allocation memory and pipe for the HID Interface are done only by application layer request. Otherwise, no resources is used.
Simplified the internal logic of opening and closing procedure for HID Device.
Added API to work with OUT EP in case it is present in HID Interface.
Changed the way of device list handling. Now the device list is available only when HID device is opened by a client. That means that is there is no one device has been opened the driver can be uninstall easily, even if the device is still connected to the port.

roma-jam · 2023-07-20T11:40:31Z

I have prepared a document with architectural changes for the next version of USB Host HID Driver.
I would like to ask you both to have a look at the document and share your opinion.

USB_HID_Host_Driver_migration_guide_v0.0.1_draft.pdf

Things, that I want to ask you:

Check the architectural changes, make your point (agree/disagree/other). I do not want you to spend more than 10-15 minutes on that, so if you are not able to understand the main idea from the document quickly - please tell me about that. I will update the document and try to do it more concise. Every feedback I would appreciate.
Verify the advantages/disadvantages paragraphs for both versions. I would like to know your opinion.

Do not refer to the code changes, only to the document. Code is not ready (only the part with esp_event_loop + device attaching and removing).

If there is anything you want to share/add/discuss - I would be happy to do it. Please tell me about that.
Sorry for my concise tone (probably), I really don't want to spend your time a lot and I want the document to be short and clear.

Thanks.

roma-jam · 2023-10-13T10:09:51Z

After removing the OPEN event and made hid_host_device_open() synchronous new version and new scheme are required.
They can be found in the next draft description file:

USB_HID_Host_Driver_migration_guide_v0.0.2_draft.pdf

@tore-espressif I have changed the hid_host_open_device() logic and now you can verify the document again.
If there is anything else you want to change or make better, feel free to share.

I also updated all the tests and update the concurrent test and now it launch on every interface connected to the ESP32-S2/3 Host.

I am going to investigate how esp_hid works with BLE an BT HID devices and based on that implement a Keyboard/Mouse Driver (which would provide the possibility to automatically, based on the report descriptor data, handle the reports and send an event of keys pressed). Also, I think about making it possible to tie the VID/PID of the device to the specific Keyboard/Mouse driver (or just to use a common one) to be able to implement different HID device driver, even proprietary one.

tore-espressif

@roma-jam The API LGTM overall. Could you provide some proof-of-concept integration with esp_hid before we proceed with the review?

tore-espressif · 2023-12-01T03:53:59Z

usb/usb_host_hid/include/usb/hid_host.h

@@ -103,18 +120,10 @@ typedef struct {
    size_t task_priority;                   /**< Task priority of created background task */
    size_t stack_size;                      /**< Stack size of created background task */
    BaseType_t core_id;                     /**< Select core on which background task will run or tskNO_AFFINITY  */
-    hid_host_driver_event_cb_t callback;    /**< Callback invoked when HID driver event occurs. Must not be NULL. */
+    esp_event_handler_t callback;           /**< Callback invoked when HID driver event occurs. Must not be NULL. */


Please correct me if I'm wrong, but isn't the main idea of esp_event that we do not have callbacks from the driver?

We should only expose the esp_loop and its events and the the user can register handlers (callbacks) with esp_event API

@tore-espressif ,
callback here is the same as is used in the esp_hid_host example, esp_hid_host_main.c, line 32
With prototype: void callback(void *handler_args, esp_event_base_t base, int32_t id, void *event_data).

The events are the same, except the ESP_HIDH_OPEN_EVENT (about what we agreed before).

But probably, it can be eliminated during integration to the esp_hid....because the esp_hid already has the implementation.
Anyway, it can be easily done.

Do I need to add these changes to this MR also? (possibility to smoothly integration to the esp_hid)

Do I need to add these changes to this MR also?

If it makes sense to you, then yes. I always thought that the v2 is supposed to be ready to be integrated with esp_hid.

If you could provide an example of the integration with esp_hid (in this MR, or in different repo), it would help the reviewers with better understanding of the new API design, as well as with easier testing with real HID devices

@tore-espressif
Sure it make sense to me, in case I think that it is always easier to verify and review everything when it is ready.

I suppose that example with integration with esp_hid should be in esp-idf repo....
Or I can actually think about make it as a part of unit testing of the component.

But just to be sure - if I am going to add these changes in this PR, it will be even bigger then it is right now.
Yes, it is ready and I have covered all the same test cases (so it should not be worse than prev version) but anyway.

~~Could you please rebase this PR on top of current HID 1.0.1~1?~~

EDIT: sorry, I forgot to do git fetch

Anyway, it is a good idea to make a rebase 👍

…ible device

… disconnect)

…disconnect events

…vice functions

…string descriptors from the USB HID Device

…ctor control EP interaction

…ct test back

roma-jam added the Component: usb label Jul 20, 2023

roma-jam self-assigned this Jul 20, 2023

roma-jam requested review from Dazza0 and tore-espressif July 20, 2023 11:30

roma-jam mentioned this pull request Sep 15, 2023

fix(usb/hid): HID Host Driver v.1.0.1 #204

Merged

7 tasks

roma-jam force-pushed the feature/usb_host_hid branch from 592e0fa to 78243a7 Compare October 10, 2023 20:32

tore-espressif reviewed Dec 1, 2023

View reviewed changes

roma-jam marked this pull request as ready for review December 1, 2023 07:53

This was referenced Dec 1, 2023

Update HID Mouse example with scroll wheel status and middle mouse button click (IDFGH-11578) espressif/esp-idf#12698

Open

The ESP crashes when I merge two projects. (IDFGH-11598) espressif/esp-idf#12716

Closed

roma-jam added 18 commits December 14, 2023 21:12

Xusb: HID Driver component, change logic to event_loop for HID compat…

4c2d117

…ible device

usb: HID Driver component, added callbacks for events (connect, open,…

03e36f7

… disconnect)

usb: HID Driver component, provide logic description for connect and …

2ed97a9

…disconnect events

usb: HID Driver component, partially added logic of open and close de…

e4f50f4

…vice functions

usb_host_hid: add hid_host_get_device_info() call to get VID/PID and …

7635beb

…string descriptors from the USB HID Device

usb_host_hid: add client_event_handling_started flag

c92b3f7

usb_host_hid: add open/close device api

532d7e4

usb_host_hid: start to implement class specific requests

4ff2cc8

usb_host_hid: class specific requests, add OUT EP send data api, refa…

f9d9279

…ctor control EP interaction

usb_host_hid: verify EP OUT data xfer, fixed

4c19f20

usb_host_hid: fix event data creation with zero initializing

cd74457

usb_host_hid: add get report descriptor api

5a17fd7

usb_host_hid: proceed to implement test cases

a8f8e87

usb_host_hid: remove variable initializaion from switch-case

b00ef89

usb_host_hid: temporary ignore sudden disconnect test

7e62692

usb_host_hid: add error handling tests

f7a0fa9

usb_host_hid: remove open event, open device is now syncronious

1683c5f

usb_host_hid: add hid_dev_handle verification, return sudden disconne…

0154503

…ct test back

roma-jam force-pushed the feature/usb_host_hid branch from 844edc4 to 0154503 Compare December 14, 2023 20:13

roma-jam mentioned this pull request Mar 22, 2024

feat: add usb host uac driver and examples (IEC-99) espressif/esp-usb#27

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feature(usb_host_hid): USB Host HID (Human Interface Device) Driver v.2.0.0 #214

feature(usb_host_hid): USB Host HID (Human Interface Device) Driver v.2.0.0 #214

roma-jam commented Jul 20, 2023 •

edited

roma-jam commented Jul 20, 2023

roma-jam commented Oct 13, 2023

tore-espressif left a comment

tore-espressif Dec 1, 2023

roma-jam Dec 1, 2023 •

edited

tore-espressif Dec 1, 2023

roma-jam Dec 1, 2023

tore-espressif Dec 14, 2023 •

edited

roma-jam Dec 14, 2023

feature(usb_host_hid): USB Host HID (Human Interface Device) Driver v.2.0.0 #214

Are you sure you want to change the base?

feature(usb_host_hid): USB Host HID (Human Interface Device) Driver v.2.0.0 #214

Conversation

roma-jam commented Jul 20, 2023 • edited

Checklist

Change description

roma-jam commented Jul 20, 2023

roma-jam commented Oct 13, 2023

tore-espressif left a comment

Choose a reason for hiding this comment

tore-espressif Dec 1, 2023

Choose a reason for hiding this comment

roma-jam Dec 1, 2023 • edited

Choose a reason for hiding this comment

tore-espressif Dec 1, 2023

Choose a reason for hiding this comment

roma-jam Dec 1, 2023

Choose a reason for hiding this comment

tore-espressif Dec 14, 2023 • edited

Choose a reason for hiding this comment

roma-jam Dec 14, 2023

Choose a reason for hiding this comment

roma-jam commented Jul 20, 2023 •

edited

roma-jam Dec 1, 2023 •

edited

tore-espressif Dec 14, 2023 •

edited