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

Allow inclusion of Hardware Serial by menu item #115

Open
wants to merge 3 commits into
base: ch55xduino
Choose a base branch
from

Conversation

serisman
Copy link

This PR adds menu items to be able to select which hardware serial items (if any) to be included in the build.
This is a potential solution for issue #113

Here are the results when compiling an empty sketch with CH552 (Default CDC @ 24 MHz):
No UART: Sketch uses 3584 (25%) of 14336 bytes. Global variables use 23 (2%) of 876 bytes.
UART0: Sketch uses 3919 (27%) of 14336 bytes. Global variables use 59 (6%) of 876 bytes.
UART1: Sketch uses 3919 (27%) of 14336 bytes. Global variables use 59 (6%) of 876 bytes.
UART0 & UART1: Sketch uses 4123 (28%) of 14336 bytes. Global variables use 95 (10%) of 876 bytes.

So, not a dramatic difference, but sometimes every little bit helps (particularly on RAM usage)

Savings for CH551 is a bit more dramatic...
No UART: Sketch uses 3568 (34%) of 10240 bytes. Global variables use 23 (6%) of 364 bytes.
UART0: Sketch uses 3903 (38%) of 10240 bytes. Global variables use 59 (16%) of 364 bytes.
Original (i.e. UART0 & UART1): Sketch uses 4107 (40%) of 10240 bytes. Global variables use 95 (26%) of 364 bytes.

@DeqingSun
Copy link
Owner

The default setting is "No UART", and it is extremely dangerous to users. A lot of issues on GitHub was caused by following some outdated random guides on the internet and not reading the Readme. We can not expect users to choose the right option before they get frustrated.

@serisman
Copy link
Author

serisman commented Feb 16, 2023

Ok, I'm curious... Why would the "No UART" option be 'extremely dangerous' to users?

Worst case I can think of is that code that tries to use Serial just doesn't compile (with an error message), right?

I suppose we could make the default be "UART0" if you think that is safer for some reason. Not sure how many projects actually use hardware serial though. Most probably only really need USB Serial.

@DeqingSun
Copy link
Owner

Because when the sketch can not compile, the issues will pop up and I will need to deal with them. This repo is not a business and no dedicated people working on that.

Or in another case, if you are using the repo for a workshop. An improper default setting may take 10% or more of the total time for troubleshooting.

So we'd better leave the option for experts like you to save the flash space, and leave those users with blinking light with a easier configuration.

@serisman
Copy link
Author

Ok, understood. Your choice of words 'extremely dangerous' just surprised me, and I just wanted to make sure I wasn't missing something.

I just committed a change that moves 'No UART' to the end of the list, making UART0 the default. That seems pretty safe to me, since that is all that was even supported until recently. Using UART1 or No UART can be considered advanced options for those that want to override the defaults. Does that work for you?

@DeqingSun
Copy link
Owner

Thanks, actually can you put "UART0 & UART1" as default? CH552E does not have UART0.

Also can you reverse the logic of the flags? Changing "UART0" to "NO_UART0" and get code included when the flag is absent.

@serisman
Copy link
Author

Yeah, I can take a look at that further tomorrow or Friday (getting to be too late here right now). Might be a bit complicated since CH551 doesn't have UART1, but I'm sure we can come up with something.

@serisman
Copy link
Author

Ok, as requested, I reversed the UARTx logic to be NO_UARTx instead, and made UART0 & UART1 to be the default (except for CH551 which doesn't have UART1).

@Saatvik-Aggarwal
Copy link

@DeqingSun This seems to be very useful since sdcc does not appear to auto exclude unused functions. 500-600 extra bytes would be very nice to have. Are there any updates on merging this before the next release?

@koendv
Copy link

koendv commented Jun 13, 2024

This is an interesting subject.
I would not add menu items; that would give too much support.
But perhaps this is something for a tool like "menuconfig"?
If somewhere in the CH55xDuino variants directory you could type "menuconfig", select which features you do not need, and a header file with ifdefs gets created.

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