-
Notifications
You must be signed in to change notification settings - Fork 369
scripts: module: helper script to create a new module from template #10211
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
Changes from 7 commits
dfcad0b
05de128
f05314d
7b45a09
d2c4193
d6b2aa1
ae3c142
c47b550
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -398,6 +398,8 @@ def parse_args(): | |||||||||||||
| help="Prints version of this script.") | ||||||||||||||
| parser.add_argument("-m", "--menuconfig", required=False, action="store_true", | ||||||||||||||
| help="Build menuconfig for target") | ||||||||||||||
| parser.add_argument("-z", "--zephyrsdk", required=False, action="store_true", | ||||||||||||||
| help="Force Build using Zephyr SDK for target") | ||||||||||||||
|
|
||||||||||||||
| args = parser.parse_args() | ||||||||||||||
|
|
||||||||||||||
|
|
@@ -836,7 +838,12 @@ def build_platforms(): | |||||||||||||
| _dict = dataclasses.asdict(platform_configs[platform]) | ||||||||||||||
| platform_dict = { k:v for (k,v) in _dict.items() if _dict[k] is not None } | ||||||||||||||
|
|
||||||||||||||
| xtensa_tools_root_dir = os.getenv("XTENSA_TOOLS_ROOT") | ||||||||||||||
| if args.zephyrsdk: | ||||||||||||||
| print("Using Zephyr SDK for building") | ||||||||||||||
| xtensa_tools_root_dir = None | ||||||||||||||
|
Comment on lines
+841
to
+843
|
||||||||||||||
| if args.zephyrsdk: | |
| print("Using Zephyr SDK for building") | |
| xtensa_tools_root_dir = None | |
| xtensa_tools_root_dir = None | |
| if args.zephyrsdk: | |
| print("Using Zephyr SDK for building") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
False positive. the else: case catches this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just run this script with XTENSA_TOOLS_ROOT unset to get the same effect, but I guess this we can have an option as well.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,20 +17,18 @@ if(NOT CONFIG_COMP_MODULE_SHARED_LIBRARY_BUILD) | |
| add_subdirectory(pcm_converter) | ||
| add_subdirectory(pipeline) | ||
|
|
||
| if(CONFIG_COMP_BASEFW_IPC4 AND NOT CONFIG_LIBRARY) | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How is this before aria?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So the alpha sorting logic is only sorting on and inserting on a |
||
| add_local_sources(sof base_fw.c) | ||
| endif() | ||
| add_local_sources_ifdef(CONFIG_IPC4_BASE_FW_INTEL sof base_fw_intel.c) | ||
|
|
||
| # directories and files included conditionally (alphabetical order) | ||
| if(CONFIG_COMP_ARIA) | ||
| add_subdirectory(aria) | ||
| endif() | ||
| if(CONFIG_COMP_ASRC) | ||
| add_subdirectory(asrc) | ||
| endif() | ||
| if(CONFIG_COMP_BASEFW_IPC4 AND NOT CONFIG_LIBRARY) | ||
| add_local_sources(sof base_fw.c) | ||
| endif() | ||
| add_local_sources_ifdef(CONFIG_IPC4_BASE_FW_INTEL sof base_fw_intel.c) | ||
| if(CONFIG_COMP_CHAIN_DMA) | ||
| add_local_sources(sof chain_dma.c) | ||
| endif() | ||
| if(CONFIG_COMP_COPIER) | ||
| add_subdirectory(copier) | ||
| endif() | ||
|
|
@@ -52,19 +50,17 @@ if(NOT CONFIG_COMP_MODULE_SHARED_LIBRARY_BUILD) | |
| if(CONFIG_COMP_IIR) | ||
| add_subdirectory(eq_iir) | ||
| endif() | ||
| if(CONFIG_COMP_KPB AND NOT CONFIG_LIBRARY_STATIC) | ||
| add_local_sources(sof | ||
| kpb.c | ||
| ) | ||
| if(CONFIG_COMP_LEVEL_MULTIPLIER) | ||
| add_subdirectory(level_multiplier) | ||
| endif() | ||
| if(CONFIG_COMP_MFCC) | ||
| add_subdirectory(mfcc) | ||
| endif() | ||
| if(CONFIG_COMP_MIXER) | ||
| add_subdirectory(mixer) | ||
| add_subdirectory(mixer) | ||
| endif() | ||
| if(CONFIG_COMP_MIXIN_MIXOUT) | ||
| add_subdirectory(mixin_mixout) | ||
| add_subdirectory(mixin_mixout) | ||
| endif() | ||
| if(CONFIG_COMP_MODULE_ADAPTER) | ||
| add_subdirectory(module_adapter) | ||
|
|
@@ -90,42 +86,49 @@ if(NOT CONFIG_COMP_MODULE_SHARED_LIBRARY_BUILD) | |
| if(CONFIG_COMP_TDFB) | ||
| add_subdirectory(tdfb) | ||
| endif() | ||
| if(CONFIG_COMP_TONE) | ||
| add_local_sources(sof | ||
| tone.c | ||
| ) | ||
| if(CONFIG_COMP_TEMPLATE) | ||
| add_subdirectory(template) | ||
| endif() | ||
| if(CONFIG_COMP_TENSORFLOW) | ||
| add_subdirectory(tensorflow) | ||
| endif() | ||
| if(CONFIG_COMP_UP_DOWN_MIXER) | ||
| add_subdirectory(up_down_mixer) | ||
| endif() | ||
| if(CONFIG_COMP_VOLUME) | ||
| add_subdirectory(volume) | ||
| endif() | ||
| if(CONFIG_COMP_TENSORFLOW) | ||
| add_subdirectory(tensorflow) | ||
| endif() | ||
| if(CONFIG_DTS_CODEC) | ||
| add_subdirectory(codec) | ||
| endif() | ||
| # end of directories and files included conditionally (alphabetical order) | ||
|
|
||
| add_subdirectory(google) | ||
| add_subdirectory(nxp) | ||
|
|
||
| if(CONFIG_COMP_CHAIN_DMA) | ||
| add_local_sources(sof chain_dma.c) | ||
| endif() | ||
| if(CONFIG_COMP_KPB AND NOT CONFIG_LIBRARY_STATIC) | ||
| add_local_sources(sof | ||
| kpb.c | ||
| ) | ||
| endif() | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. And why these are here, shouldn't these be in above alphabetical listing?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ditto - and these should really be in their own directories. |
||
| if(CONFIG_INTEL_ADSP_MIC_PRIVACY) | ||
| add_subdirectory(mic_privacy_manager) | ||
| endif() | ||
| if(CONFIG_COMP_TONE) | ||
| add_local_sources(sof | ||
| tone.c | ||
| ) | ||
| endif() | ||
| if(CONFIG_ZEPHYR_NATIVE_DRIVERS) | ||
| list(APPEND base_files host-zephyr.c) | ||
| sof_list_append_ifdef(CONFIG_COMP_DAI base_files dai-zephyr.c) | ||
| else() | ||
| list(APPEND base_files host-legacy.c) | ||
| sof_list_append_ifdef(CONFIG_COMP_DAI base_files dai-legacy.c) | ||
| endif() | ||
| if(CONFIG_COMP_TEMPLATE_COMP) | ||
| add_subdirectory(template_comp) | ||
| endif() | ||
| if(CONFIG_COMP_LEVEL_MULTIPLIER) | ||
| add_subdirectory(level_multiplier) | ||
| endif() | ||
| endif() | ||
|
|
||
| ### Common files (also used in shared library build) | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Debug print statements should not be left in production code. This print statement should be removed or wrapped in a debug flag check.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Intentional to show module UUID is being compiled.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This print statement adds unnecessary noise to the logs. If someone needs to inspect the UUIDs, they can easily find them in the
uuid-registry.txtfile, so logging each one during generation isn't really needed.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The logging makes it more obvious to the module developer (most of whom will not know about UUIDs) which UUIDs are being added to the build. i.e. 3P developer can see their new module UUID is either in the build or not as they will see volume, mixin etc UUIDs and realize more quickly I need to add my UUID.