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

Introduce cdc_shell_write_string() function for zero-terminated strings #18

Merged
merged 5 commits into from
Dec 2, 2020

Conversation

vnodeng
Copy link
Contributor

@vnodeng vnodeng commented Dec 2, 2020

This makes programming less error prone and saves 140 bytes of flash.
Also, replace some const pointers to const strings with just const strings.

This makes programming less error prone and saves 140 bytes of flash.
Also, replace some const pointers to const strings with just const strings.
@r2axz
Copy link
Owner

r2axz commented Dec 2, 2020

@borisxm Looks good, thank you for this. I will test your pull-request and let you know the results.

@r2axz
Copy link
Owner

r2axz commented Dec 2, 2020

Tested with pvs-studio, looks OK.

@r2axz
Copy link
Owner

r2axz commented Dec 2, 2020

I confirm code size improvement.

@r2axz
Copy link
Owner

r2axz commented Dec 2, 2020

Tested, looks good. @borisxm if you don't mind, I'm going to merge your PR and tag to v2.2.1. Thoughts?

@vnodeng
Copy link
Contributor Author

vnodeng commented Dec 2, 2020

Sounds fine.

@r2axz
Copy link
Owner

r2axz commented Dec 2, 2020

@borisxm Thank you!

@r2axz
Copy link
Owner

r2axz commented Dec 2, 2020

@borisxm One more thing, almost overlooked it, but I believe cdc_shell_write_string should live in cdc_shell.c. cdc_shell_write is a generic write callback that knows about usb_cdc internals. On the other hand, all its wrappers should live outside usb_cdc.c because they have nothing to do with usb_cdc itself. Can you fix that?

This makes programming less error prone and saves 140 bytes of flash.
Also, replace some const pointers to const strings with just const strings.
@vnodeng
Copy link
Contributor Author

vnodeng commented Dec 2, 2020

Sure, slightly reworked the patch, now it saves even more space.

@r2axz
Copy link
Owner

r2axz commented Dec 2, 2020

@borisxm I will have a look later today and let you know! Thank you!

@r2axz
Copy link
Owner

r2axz commented Dec 2, 2020

@borisxm Looks good now, I'm going to merge the PR. Thank you for your contribution.

@r2axz r2axz merged commit 48c0293 into r2axz:main Dec 2, 2020
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