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

f4-discovery usb_cdcacm cleanup #119

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

svofski
Copy link

@svofski svofski commented Jan 25, 2016

Some samples are slightly rough around the corners. I think they can be made a little bit more self-descriptive. Here's clean up of f4-discovery usb_cdcacm sample. Things modified:

  • Named endpoints
  • Named result codes
  • Data buffer made static instead of allocated on stack
  • Used sizeof-based values where possible

Clean up of f4-discovery usb_cdcacm sample.
- Named endpoints
- Named result codes
- Data buffer made static instead of allocated on stack
- Used sizeof-based values where possible
static uint8_t usbd_control_buffer[128];

/* Data I/O buffer */
static char data_buf[64];
Copy link
Member

Choose a reason for hiding this comment

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

mixing char and uint8_t?

Copy link
Author

Choose a reason for hiding this comment

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

Right, no reason to use char here.

@karlp
Copy link
Member

karlp commented Jan 26, 2016

The general problem with something like this is that it results in the examples being different. They really need to be more common, not diverging with this cdcacm example being completely different from the others.

@svofski
Copy link
Author

svofski commented Jan 26, 2016

@karlp I cannot really update all examples because I do not own every possible supported board/arch. Or rather, I can - why not, I just cannot verify the changes in that case.

Examples are already different enough. Example: not sure what this line does here and I'm pretty sure it's wrong:
https://github.com/libopencm3/libopencm3-examples/blob/master/examples/stm32/f3/stm32f3-discovery/usb_cdcacm/cdcacm.c#L215

@tormodvolden tormodvolden mentioned this pull request Dec 1, 2021
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.

None yet

2 participants