DeqingSun / ch55xduino

An Arduino-like programming API for the CH55X
GNU Lesser General Public License v2.1
449 stars 87 forks source link

Allow inclusion of Hardware Serial by menu item #115

Open serisman opened 1 year ago

serisman commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

@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 commented 5 months ago

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.