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

[Silabs] Fix enable_openthread_cli/uart build configuration #25835

Merged

Conversation

jmartinez-silabs
Copy link
Member

@jmartinez-silabs jmartinez-silabs commented Mar 25, 2023

This pr fix an issue where enable_openthread_cli default value was defining some values and adding multiple uart files to the build when it was not needed for wifi ncp builds.

tested by building lighting app for thread and all 3 wifi ncp board supported and validating outputs

…piled when if not needed mostly on the wifi builds
@github-actions
Copy link

PR #25835: Size comparison from ec2ad41 to 4fa4413

Increases (1 build for cc32xx)
platform target config section ec2ad41 4fa4413 change % change
cc32xx lock CC3235SF_LAUNCHXL .debug_info 20309905 20309906 1 0.0
Full report (1 build for cc32xx)
platform target config section ec2ad41 4fa4413 change % change
cc32xx lock CC3235SF_LAUNCHXL 0 0 0 0.0
(read only) 645745 645745 0 0.0
(read/write) 203848 203848 0 0.0
.ARM.attributes 44 44 0 0.0
.ARM.exidx 8 8 0 0.0
.bss 197248 197248 0 0.0
.comment 194 194 0 0.0
.data 1480 1480 0 0.0
.debug_abbrev 933102 933102 0 0.0
.debug_aranges 87704 87704 0 0.0
.debug_frame 301604 301604 0 0.0
.debug_info 20309905 20309906 1 0.0
.debug_line 2680953 2680953 0 0.0
.debug_loc 2827596 2827596 0 0.0
.debug_ranges 286376 286376 0 0.0
.debug_str 3041114 3041114 0 0.0
.ramVecs 780 780 0 0.0
.resetVecs 64 64 0 0.0
.rodata 105953 105953 0 0.0
.shstrtab 232 232 0 0.0
.stab 204 204 0 0.0
.stabstr 441 441 0 0.0
.stack 2048 2048 0 0.0
.strtab 380555 380555 0 0.0
.symtab 257456 257456 0 0.0
.text 537672 537672 0 0.0

Copy link
Contributor

@andy31415 andy31415 left a comment

Choose a reason for hiding this comment

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

Need to replace the -1 magic value with something else. Either enum that is user-friendly or a bool with a default (I believe bool with default is clearer as "default/auto" in an enum seems to be ambigous)

@jmartinez-silabs
Copy link
Member Author

jmartinez-silabs commented Mar 27, 2023

@andy31415 Replying to your require changes comment.

I'll explain the situation here.

The default args depend on other possible configurations. True for a standard thread build, False for a build with wifi-ncp.
Since it depends on other build args that 1. are decided by the user, 2. need to be defined in another gni file elsewhere I cannot do the conditional default value in the declare_args since passed arguments value would not have replaced the default value yet et the gn flow.

I also want the user the have the final say in his build arguments. I need to know that the user did pass this argument in his build command and to not override what he passed with my conditional value. Hence the undefined value.

This doesn't apply, user error on my part!

@jmartinez-silabs jmartinez-silabs enabled auto-merge (squash) March 27, 2023 18:09
@jmartinez-silabs jmartinez-silabs merged commit 8712142 into project-chip:master Mar 27, 2023
@jmartinez-silabs jmartinez-silabs deleted the fix/uart_conditions branch March 27, 2023 18:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants