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

pkg/micropython: model in Kconfig #19672

Merged
merged 2 commits into from May 31, 2023

Conversation

aabadie
Copy link
Contributor

@aabadie aabadie commented May 26, 2023

Contribution description

For some reason, I had to add a dependency to periph_spi. Otherwise, the generated firmware size was bigger with TEST_KCONFIG=1 on native. Not sure why.

In general, I think there's room for improvement with the build system integration of micropython, for example to select which feature (uart, spi, timer, etc) to add in the firmware, if that is possible.

Testing procedure

  • Green CI

Issues/PRs references

Tick one item in #16875

@aabadie aabadie added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR CI: no fast fail don't abort PR build after first error labels May 26, 2023
@github-actions github-actions bot added Area: examples Area: Example Applications Area: Kconfig Area: Kconfig integration Area: pkg Area: External package ports labels May 26, 2023
@riot-ci
Copy link

riot-ci commented May 26, 2023

Murdock results

✔️ PASSED

c7f1226 examples/micropython: add Kconfig config

Success Failures Total Runtime
6933 0 6933 15m:06s

Artifacts

@aabadie aabadie force-pushed the pr/pkg/micropython_kconfig branch from 7845cee to 3ea6117 Compare May 26, 2023 09:22
@MrKevinWeiss
Copy link
Contributor

I can look into that. It seems the package wasn't updated for quite some time. We should also probably upstream the ztimer update...

pkg/micropython/Kconfig Outdated Show resolved Hide resolved
@aabadie aabadie force-pushed the pr/pkg/micropython_kconfig branch from 3ea6117 to dfcc054 Compare May 30, 2023 09:00
Copy link
Contributor

@MrKevinWeiss MrKevinWeiss left a comment

Choose a reason for hiding this comment

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

I think the standard is a bit closer to the following diff:

diff --git a/examples/micropython/Kconfig b/examples/micropython/Kconfig
index 6e1b6464e8..32980b2927 100644
--- a/examples/micropython/Kconfig
+++ b/examples/micropython/Kconfig
@@ -5,8 +5,9 @@
 # directory for more details.
 #
 
-config APPLICATION_EXAMPLES_MICROPYTHON
-    bool "Micropython example application"
+config APPLICATION
+    bool
+    default y
     depends on TEST_KCONFIG
 
     select MODULE_PERIPH_ADC if HAS_PERIPH_ADC
diff --git a/examples/micropython/Makefile b/examples/micropython/Makefile
index 247540cfbe..e5e7829693 100644
--- a/examples/micropython/Makefile
+++ b/examples/micropython/Makefile
@@ -34,4 +34,7 @@ TESTRUNNER_RESET_AFTER_TERM ?= 1
 # failing on native with floating point exception (#15870)
 TEST_ON_CI_BLACKLIST = native
 
+# avoid running Kconfig by default
+SHOULD_RUN_KCONFIG ?=
+
 include $(RIOTBASE)/Makefile.include
diff --git a/examples/micropython/app.config.test b/examples/micropython/app.config.test
index f03643a0c9..d2c7fc42c9 100644
--- a/examples/micropython/app.config.test
+++ b/examples/micropython/app.config.test
@@ -1,2 +1 @@
 CONFIG_PACKAGE_MICROPYTHON=y
-CONFIG_APPLICATION_EXAMPLES_MICROPYTHON=y

@aabadie aabadie force-pushed the pr/pkg/micropython_kconfig branch from dfcc054 to c7f1226 Compare May 31, 2023 07:44
Copy link
Contributor

@MrKevinWeiss MrKevinWeiss left a comment

Choose a reason for hiding this comment

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

ACK!

@MrKevinWeiss
Copy link
Contributor

bors merge

@bors
Copy link
Contributor

bors bot commented May 31, 2023

🕐 Waiting for PR status (GitHub check) to be set, probably by CI. Bors will automatically try to run when all required PR statuses are set.

@aabadie
Copy link
Contributor Author

aabadie commented May 31, 2023

bors merge

bors bot added a commit that referenced this pull request May 31, 2023
19672: pkg/micropython: model in Kconfig r=aabadie a=aabadie



Co-authored-by: Alexandre Abadie <alexandre.abadie@inria.fr>
@aabadie
Copy link
Contributor Author

aabadie commented May 31, 2023

bors cancel
bors merge

@bors
Copy link
Contributor

bors bot commented May 31, 2023

Canceled.

@bors
Copy link
Contributor

bors bot commented May 31, 2023

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

@bors bors bot merged commit 784692e into RIOT-OS:master May 31, 2023
26 checks passed
@aabadie aabadie deleted the pr/pkg/micropython_kconfig branch June 1, 2023 12:26
@benpicco benpicco added this to the Release 2023.07 milestone Aug 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: examples Area: Example Applications Area: Kconfig Area: Kconfig integration Area: pkg Area: External package ports CI: no fast fail don't abort PR build after first error CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants