Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

UVC driver release 1.0.4 (IEC-104) #31

Merged

Conversation

lijunru-hub
Copy link
Contributor

@lijunru-hub lijunru-hub commented Apr 23, 2024

  1. support print format frame based descriptor
  2. support print frame frame based descriptor

@lijunru-hub
Copy link
Contributor Author

related: espressif/libuvc#1

@github-actions github-actions bot changed the title feat: support printf frame based feat: support printf frame based (IEC-104) Apr 23, 2024
@leeebo
Copy link
Collaborator

leeebo commented Apr 23, 2024

@lijunru-hub LGTM! but please add more detailed PR descriptions (eg. the purpose, and specs you referred).

and please add the scope information in your commit message

eg. feat(usb_host_uvc): support printf frame based descriptors

@leeebo
Copy link
Collaborator

leeebo commented Apr 23, 2024

"USB Video Payload Frame Based v1.5" https://www.usb.org/document-library/video-class-v15-document-set

@tore-espressif
Copy link
Collaborator

@lijunru-hub lijunru-hub force-pushed the feat/support_printf_frame_based branch from e1d112c to c0f4575 Compare April 23, 2024 12:14
Copy link
Collaborator

@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.

@lijunru-hub Thanks for the request!

This suppose to be a release for verison 1.0.4 of uvc host class driver.

Could you please update the PR description accordingly? (you can refer to #24 or #11)

host/class/uvc/usb_host_uvc/src/descriptor.c Show resolved Hide resolved
@lijunru-hub lijunru-hub reopened this Apr 24, 2024
@lijunru-hub lijunru-hub changed the title feat: support printf frame based (IEC-104) UVC driver release 1.0.4 (IEC-104) Apr 24, 2024
@roma-jam roma-jam self-requested a review April 26, 2024 08:44
Copy link
Collaborator

@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.

Thanks,
Changes LGTM.

@lijunru-hub
Copy link
Contributor Author

@tore-espressif PTAL

@lijunru-hub
Copy link
Contributor Author

@tore-espressif @roma-jam
Is there still an issue with this PR? If there are no issues, should it be merged?

@tore-espressif tore-espressif force-pushed the feat/support_printf_frame_based branch from c0f4575 to a2b5ac4 Compare May 9, 2024 07:43
Copy link
Collaborator

@tore-espressif tore-espressif left a comment

Choose a reason for hiding this comment

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

LGTM! I rebased the PR and we can merge after the CI succeeds

// Copy to local buffer due to potential misalignment issues.
uint32_t raw_desc[25];
uint32_t desc_size = ((const vs_frame_desc_t *)buff)->bLength;
memcpy(raw_desc, buff, desc_size);

Check warning

Code scanning / clang-tidy

Call to function 'memcpy' is insecure as it does not provide security checks introduced in the C11 standard. Replace with analogous functions that support length arguments or provides boundary checks such as 'memcpy_s' in case of C11 [clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling] Warning

Call to function 'memcpy' is insecure as it does not provide security checks introduced in the C11 standard. Replace with analogous functions that support length arguments or provides boundary checks such as 'memcpy_s' in case of C11 [clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling]
@tore-espressif tore-espressif merged commit 8dfe41c into espressif:master May 9, 2024
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants