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

Recommend renaming NUM_ENDPOINTS to LAST_ENDPOINT #640

Open
liudr opened this issue Mar 26, 2022 · 2 comments
Open

Recommend renaming NUM_ENDPOINTS to LAST_ENDPOINT #640

liudr opened this issue Mar 26, 2022 · 2 comments

Comments

@liudr
Copy link

liudr commented Mar 26, 2022

Here is the relevant line that uses the #define:

endpoint_t endpoint_queue_head[(NUM_ENDPOINTS+1)*2] __attribute__ ((used, aligned(4096), section(".endpoint_queue") ));

The most common problem of NUM_ENDPOINTS is that one has to guess what EPs to include in the count. Should I include EP0 (yes), should I include EP1 (no)? An even more serious problem is if the user decides to skip some EPs and say only use EP3 (IN) and EP4 (OUT). They think they have 2 EPs and based on other definitions they set NUM_ENDPOINTS to 3 to include EP0. But they should have set it to 4 because the skipped EP2 although isn't initialized, MUST be allocated endpoint_t queue heads because how the rest of the code is written: using EP2 or EP2+1 to address the queue heads as if all EPs before the EP were present.

Three files use this #define, usb_desc.c, usb_desc_h, and usb.c

@PaulStoffregen
Copy link
Owner

While this could be clearer, as a general rule I usually do not change the names of defines or global scope items after the software has been widely published. Sometimes these changes cause a lot of unexpected pain for people who have modified copies of the code. To make this sort of change requires a compelling need.

@ssilverman
Copy link
Contributor

@liudr What about instead adding a comment line to where NUM_ENDPOINTS is defined? This would add some info, while at the same time meeting Paul’s requirements.

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

No branches or pull requests

3 participants