Skip to content

feat: improve Makefile #7

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.

Sign up for GitHub

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

Jump to bottom
Open
LZhaoM wants to merge 1 commit into
base: main
Choose a base branch
from
Open

feat: improve Makefile #7

LZhaoM wants to merge 1 commit into from

Conversation

@LZhaoM
Copy link
Collaborator

@LZhaoM LZhaoM commented Sep 5, 2025 •
edited
Loading 

    feat: improve Makefile
    
    Requiring setting the variables before building can avoid the situation
    that, in the first time building, the default configurations are not what
    users want.
    
    Some paths are hardcoded with c200/*. Now this repo supports both Airbox
    Orin and C200, we should use the PRODUCT variable to replace the
    hardcoded part of the paths.
    
    For consistency, README is updated to introduce the possible values of
    PRODUCT and BOARDSKU.
    
    Signed-off-by: Zhaoming Luo <[email protected]>

@LZhaoM LZhaoM requested a review from RadxaYuntian September 5, 2025 03:59
RadxaYuntian
RadxaYuntian reviewed Sep 5, 2025
View reviewed changes
Copy link
Member

@RadxaYuntian RadxaYuntian left a comment

Choose a reason for hiding this comment

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

用?=就是为了可以在调用make的时候传入自己定义的变量值啊，你现在就必须去改文件了

@LZhaoM
Copy link
Collaborator Author

LZhaoM commented Sep 5, 2025 •
edited
Loading

主要是感觉这样Simple and Stupid一些，README里也没提到这些配置，加到README里我们又需要保持Makefile里和README的一致性。干脆直接这样子更直观一些。用户需要的话自己加个?=就行。

@LZhaoM LZhaoM requested a review from RadxaYuntian September 5, 2025 05:58
@RadxaYuntian
Copy link
Member

RadxaYuntian commented Sep 5, 2025

# Other products can be specified with PRODUCT variable的意思我指的就是调用时传入不同的PRODUCT，不然说明就是让用户去修改文件了

@RadxaYuntian
Copy link
Member

RadxaYuntian commented Sep 5, 2025

原本的readme也都是给c200准备的，你要改的话就把readme还有比如默认路径都改了

@LZhaoM
Copy link
Collaborator Author

LZhaoM commented Sep 5, 2025

原本的readme也都是给c200准备的，你要改的话就把readme还有比如默认路径都改了

README删掉# Other products can be specified with PRODUCT variable就行了是吗。

默认路径指的是什么？

@RadxaYuntian
Copy link
Member

RadxaYuntian commented Sep 5, 2025

默认路径指的是makefile里面很多写死的c200
另外你直接把你现在的= error改成?= error不就好了么？

@RadxaYuntian
Copy link
Member

RadxaYuntian commented Sep 5, 2025

另外现在既然没有默认值可用，readme还要准备一个表格，用于告诉用户哪个产品哪个核心板用什么参数配置

@LZhaoM LZhaoM force-pushed the improve-makefile branch 2 times, most recently from 9d77618 to 3493d1e Compare September 5, 2025 07:17
@LZhaoM
feat: improve Makefile
52f177e 
Requiring setting the variables before building can avoid the situation
that, in the first time building, the default configurations are not what
users want.

Some paths are hardcoded with c200/*. Now this repo supports both Airbox
Orin and C200, we should use the PRODUCT variable to replace the
hardcoded part of the paths.

For consistency, README is updated to introduce the possible values of
PRODUCT and BOARDSKU.

Signed-off-by: Zhaoming Luo <[email protected]>
@LZhaoM LZhaoM requested review from RadxaYuntian and removed request for RadxaYuntian September 5, 2025 07:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@RadxaYuntian RadxaYuntian Awaiting requested review from RadxaYuntian

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@LZhaoM @RadxaYuntian