Skip to content

Allow mpconfigboard.mk to set board extensions #6618

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

Closed
wants to merge 1 commit into from

Conversation

jepler
Copy link

@jepler jepler commented Jul 19, 2022

Looking at the PR to add Beetle esp32 c3 (#6615) I noticed that any microcontroller of this type would require modifying build_board_info, which in turn causes a build of all boards. The same would be true of esp32 boards, of which we hope a large number will be submitted soon.

Instead, create two different ways for an mpconfigboard.mk file to override the default setting by port: For espressif, use the existing IDF_TARGET variable. For the general case, allow the EXTENSIONS variable to be specified.

Note that the value cannot come from mpconfigport.mk and cannot be conditional. This condition could be lifted in the future if it is useful. Additionally, if IDF_TARGET and EXTENSIONS both exist, the one that appears FIRST in the mpconfigboard.mk file is used, which is a bit tricky.

I ran "DEBUG=x RELEASE_TAG=x build_board_info.py > foo.json" before and after to generate a copy of the release info. When diffing them, there is no difference (as expected).

I also used a new quick debug facility just for checking extensions, "build_board_info.py espressif:adafruit_feather_esp32s2", to spot check that the code was working as expected, since running the full "build_board_info" process is time consuming.

The remaining special cases in "extension_by_board" should be moved to the correct mpconfigport.mk files and the whole facility of "extension by board" should be removed from build_board_info.py.

Looking at the PR to add Beetle esp32 c3 (adafruit#6615) I noticed that any
microcontroller of this type would require modifying build_board_info,
which in turn causes a build of all boards. The same would be true of
esp32 boards, of which we hope a large number will be submitted soon.

Instead, create two different ways for an mpconfigboard.mk file to
override the default setting by port: For espressif, use the existing
IDF_TARGET variable. For the general case, allow the EXTENSIONS variable
to be specified.

Note that the value cannot come from mpconfigport.mk and cannot be
conditional. This condition could be lifted in the future if it is
useful. Additionally, if IDF_TARGET and EXTENSIONS both exist, the one
that appears FIRST in the mpconfigboard.mk file is used, which is
a bit tricky.

I ran "DEBUG=x RELEASE_TAG=x build_board_info.py > foo.json" before and
after to generate a copy of the release info. When diffing them, there
is no difference (as expected).

I also used a new quick debug facility just for checking extensions,
"build_board_info.py espressif:adafruit_feather_esp32s2", to spot check
that the code was working as expected, since running the full
"build_board_info" process is time consuming.

The remaining special cases in "extension_by_board" should be moved to
the correct mpconfigport.mk files and the whole facility of "extension by
board" should be removed from build_board_info.py.
@jepler jepler requested a review from dhalbert July 19, 2022 00:45
@jepler jepler marked this pull request as draft July 19, 2022 01:50
@jepler
Copy link
Author

jepler commented Jul 19, 2022

Setting this to draft as Neradoc mentioned on Discord having a work in progress solution to this which is probably better (cleans up this script and the support matrix script to work better together)

@askpatrickw
Copy link

This PR would fix this issue, no?
#6269

@jepler
Copy link
Author

jepler commented Jul 20, 2022

Yes, I updated the initial comment to link this PR to that issue. Thanks!

@jepler
Copy link
Author

jepler commented Jul 25, 2022

Closing because this is better done in #6629.

@jepler jepler closed this Jul 25, 2022
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.

2 participants