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

buildsystem: fix kconfig mismatches #19813

Merged
merged 3 commits into from
Jul 9, 2023
Merged

buildsystem: fix kconfig mismatches #19813

merged 3 commits into from
Jul 9, 2023

Conversation

maribu
Copy link
Member

@maribu maribu commented Jul 9, 2023

Contribution description

This adds three commits each addressing a deviation between dependency resolutions done with and without Kconfig. Hopefully, these in combination should fix the nightly builds.

Testing procedure

For each $failing_board and $failing_app pair in the latest nightly build, do:

make BOARD=$failing_board -C $failing_app TEST_KCONFIG=0 info-modules > ~/without-kconfig
make BOARD=$failing_board -C $failing_app TEST_KCONFIG=1 info-modules > ~/with-kconfig
diff -rupN ~/without-kconfig ~/with-kconfig

should yield an empty diff other than the === [ATTENTION] Testing Kconfig dependency modelling ===.

Issues/PRs references

None

This matches the behavior when KConfig is used.
Without KConfig presence of the TFT is assumed in anycase, with KConfig
it is optional. This turns the opt-in into an opt-out to match behavior
with make by default, fixing the nightly breakage.
@maribu maribu added Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) Area: build system Area: Build system CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Jul 9, 2023
@github-actions github-actions bot added Area: tests Area: tests and testing framework Area: drivers Area: Device drivers Area: boards Area: Board ports Area: Kconfig Area: Kconfig integration and removed Area: build system Area: Build system labels Jul 9, 2023
@riot-ci
Copy link

riot-ci commented Jul 9, 2023

Murdock results

✔️ PASSED

ca07f77 drivers/st7735: fix inconsistency in dependency modeling

Success Failures Total Runtime
6936 0 6936 10m:06s

Artifacts

Copy link
Contributor

@gschorcht gschorcht left a comment

Choose a reason for hiding this comment

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

Thanks for figuring out these problems.

drivers/st7735/Kconfig Show resolved Hide resolved
drivers/st7735/Kconfig Outdated Show resolved Hide resolved
@@ -26,6 +26,12 @@ config MODULE_ST7789
bool "ST7789 display driver"
select MODULE_ST7735

config ST77XX
Copy link
Contributor

Choose a reason for hiding this comment

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

BTW, I have already a fix here and would solve it in a different way since the driver ST7735 is the selected component:

diff --git a/drivers/st7735/Kconfig b/drivers/st7735/Kconfig
index 40fa5643b7..0a27a4ef63 100644
--- a/drivers/st7735/Kconfig
+++ b/drivers/st7735/Kconfig
@@ -22,16 +22,16 @@ config HAVE_ST7735
     help
       Indicates that an ST7735 display is present.
 
-config MODULE_ST7789
-    bool "ST7789 display driver"
-    select MODULE_ST7735
-
 config HAVE_ST7789
     bool
-    select MODULE_ST7789 if MODULE_DISP_DEV
     help
       Indicates that an ST7789 display is present.
 
+config MODULE_ST7789
+    bool
+    depends on HAVE_ST7789
+    default y if MODULE_ST7735
+
 menuconfig KCONFIG_USEMODULE_ST7735
     bool "Configure ST7735 driver"
     depends on USEMODULE_ST7735

That is, of the board selects HAVE_ST7789, MODULE_ST7789 is enabled by default if MODULE_ST7735 is used.

I have already locally an according extension for ST7796.

Copy link
Member Author

Choose a reason for hiding this comment

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

That's better, I'll update when I'm back at the keyboard

@github-actions github-actions bot removed the Area: tests Area: tests and testing framework label Jul 9, 2023
In KConfig `MODULE_ST7789` now is hidden module that automatically
gets selected when `HAVE_ST7789` is selected in `MODULE_ST7735` is used.

Co-authored-by: Gunar Schorcht <[email protected]>
Copy link
Contributor

@gschorcht gschorcht left a comment

Choose a reason for hiding this comment

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

Looks good

@gschorcht
Copy link
Contributor

bors merge

@bors
Copy link
Contributor

bors bot commented Jul 9, 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.

@maribu maribu added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Jul 9, 2023
@maribu
Copy link
Member Author

maribu commented Jul 9, 2023

bors retry

@bors
Copy link
Contributor

bors bot commented Jul 9, 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 123cce9 into RIOT-OS:master Jul 9, 2023
@benpicco benpicco added this to the Release 2023.07 milestone Aug 2, 2023
bors bot added a commit that referenced this pull request Aug 7, 2023
19824: boards/sipeed_longan_nano: separate board definition for Sipeed Longan Nano TFT r=benpicco a=gschorcht

### Contribution description

This PR provides a minimal separate board definition for the Sipeed Longan Nano version with TFT display which is just an extension of `boards/sipeed-longan-nano` with enabled TFT display module.

From the lessons we had to learn with the Kconfig modelling of optional hardware, the TFT version of the Sipeed Longan Nano board has been split off into its own board definition based on the existing Siepeed Longan Nano board.

Commits ba29a5e, 237819e, 6d8b56d and c5faf34 are small cleanups of peripheral configurations and could be split from this PR as follow-up PR (changes +70 -36).

### Testing procedure

Green CI

```
BOARD=sipeed-longan-nano-tft make -j8 -C tests/drivers/st77xx flash
```
should work

### Issues/PRs references

Follow up to PR #19813 and PR #19814
Prerequisite for PR #19825 and PR #19827 

19855: gnrc_static: fix static PID assignment r=benpicco a=benpicco



Co-authored-by: Gunar Schorcht <[email protected]>
Co-authored-by: Benjamin Valentin <[email protected]>
bors bot added a commit that referenced this pull request Aug 7, 2023
19824: boards/sipeed_longan_nano: separate board definition for Sipeed Longan Nano TFT r=benpicco a=gschorcht

### Contribution description

This PR provides a minimal separate board definition for the Sipeed Longan Nano version with TFT display which is just an extension of `boards/sipeed-longan-nano` with enabled TFT display module.

From the lessons we had to learn with the Kconfig modelling of optional hardware, the TFT version of the Sipeed Longan Nano board has been split off into its own board definition based on the existing Siepeed Longan Nano board.

Commits ba29a5e, 237819e, 6d8b56d and c5faf34 are small cleanups of peripheral configurations and could be split from this PR as follow-up PR (changes +70 -36).

### Testing procedure

Green CI

```
BOARD=sipeed-longan-nano-tft make -j8 -C tests/drivers/st77xx flash
```
should work

### Issues/PRs references

Follow up to PR #19813 and PR #19814
Prerequisite for PR #19825 and PR #19827 

Co-authored-by: Gunar Schorcht <[email protected]>
bors bot added a commit that referenced this pull request Aug 7, 2023
19824: boards/sipeed_longan_nano: separate board definition for Sipeed Longan Nano TFT r=benpicco a=gschorcht

### Contribution description

This PR provides a minimal separate board definition for the Sipeed Longan Nano version with TFT display which is just an extension of `boards/sipeed-longan-nano` with enabled TFT display module.

From the lessons we had to learn with the Kconfig modelling of optional hardware, the TFT version of the Sipeed Longan Nano board has been split off into its own board definition based on the existing Siepeed Longan Nano board.

Commits ba29a5e, 237819e, 6d8b56d and c5faf34 are small cleanups of peripheral configurations and could be split from this PR as follow-up PR (changes +70 -36).

### Testing procedure

Green CI

```
BOARD=sipeed-longan-nano-tft make -j8 -C tests/drivers/st77xx flash
```
should work

### Issues/PRs references

Follow up to PR #19813 and PR #19814
Prerequisite for PR #19825 and PR #19827 

19855: gnrc_static: fix static PID assignment r=benpicco a=benpicco



19862: cpu/riscv_common: remove picolibc from blacklisting in CI r=benpicco a=gschorcht

### Contribution description

`picolibc` is now supported by the RISC-V toolchain in `riotdocker`. It is not necessary to blacklist it in CI any longer.

Reference: `picolib` was blacklisted by commit 45270da in PR #15011.

### Testing procedure

1. Green CI
2. Check feature list for CI compilation:
   ```
   BUILD_IN_DOCKER=1 RIOT_CI_BUILD=1 FEATURES_OPTIONAL=picolibc \
   BOARD=sipeed-longan-nano make -j8 -C tests/sys/shell info-modules | grep picolib
   picolibc
   picolibc_stdout_buffered
   picolibc_syscalls_default
   ```

### Issues/PRs references



Co-authored-by: Gunar Schorcht <[email protected]>
Co-authored-by: Benjamin Valentin <[email protected]>
@maribu maribu deleted the kconfig-fix branch December 5, 2023 08:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: boards Area: Board ports Area: drivers Area: Device drivers Area: Kconfig Area: Kconfig integration CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants