cpu/esp32: enforce MAXTHREADS is at least 3 by maribu · Pull Request #18210 · RIOT-OS/RIOT

maribu · 2022-06-15T08:02:21Z

Contribution description

ESP32 has an esp_timer thread in addition to the main and the idle thread. So applications that work fine with 2 threads on other platforms will break on ESP32. This adds a static_assert() to enforce MAXTHREADS is never less than 3 on ESP32 and fixes the offenders.

Testing procedure

Green Murdock and tests/pkg_fff should now pass on ESP32 boards.

Issues/PRs references

#17076 (comment)

maribu · 2022-06-15T08:05:37Z

Test now passes:

$ make BOARD=esp32-mh-et-live-minikit BUILD_IN_DOCKER=1 flash test -C tests/pkg_fff
[...]
Help: Press s to start test, r to print it is ready
READY
s
START
main(): This is RIOT! (Version: 2022.07-devel-773-gad07a-cpu/esp32)
Testing fff
FFF test completed: SUCCESS
All tests successful

gschorcht · 2022-06-15T09:44:55Z

tests/periph_gpio_ll/Makefile

 CFLAGS += -DPIN_OUT_1=$(PIN_OUT_1)
-# We only need 1 thread (+ the Idle thread on some platforms) and we really want
-# this app working on all boards.
 CFLAGS += -DMAXTHREADS=2


Suggested change

CFLAGS += -DMAXTHREADS=2

MAXTHREADS is set in the following ifeq ... else ... endif in any case, so this line should be removed here.

gschorcht · 2022-06-16T07:23:18Z

Here again the hashes problem. It's really strange that it occurs in different PRs for exactly one but different app that is nt related to the changes in the PR and exactly one but different board:

PR #17040: tests_mtd_mapper for esp32-ethernet-kit-v1_1 (worker comsys)
PR #17844: tests_xtimer_periodic_wakeup for esp32-heltec-lora32-v2 (worker comsys)
PR #18178: tests_touch_dev for esp32-heltec-lora32-v2 (worker comsys)
PR #18207: tests/periph_i2c for esp32-heltec-lora32-v2 (worker comsys)
PR #18209: tests/xtimer_remove for esp32-heltec-lora32-v2 (worker comsys)
PR #18210: tests_pkg_fff for esp32-heltec-lora32-v2 (worker breeze)

The problem occurs mostly for esp32-heltec-lora32-v2 and mostly on worker comsys.

At the moment many compilations fail because of this problem. I have already tried to figure out what's in the additional debug information - without success.

maribu · 2022-06-16T07:56:09Z

Note that I can reproduce the issue locally with BUILD_IN_DOCKER=1, so that is unrelated to the worker they are executed on. With elf_diff I can see that no code has changed. A diff of the output of objdump -D of the two files looks like this:

--- a
+++ b
@@ -1,5 +1,5 @@
 
-a.elf:     file format elf32-xtensa-le
+b.elf:     file format elf32-xtensa-le
 
 
 Disassembly of section .rtc.text:
@@ -31826,145 +31826,137 @@
 00000000 <.debug_info>:
        0:	00000004000024fe 	{ excw; excw; excw }
        8:	040000               	extui	a0, a0, 0, 1
-       b:	97e001               	l32r	a0, fffe5f8c <_rtc_bss_rtc_end+0xaffe5f64>
+       b:	9ecb01               	l32r	a0, fffe7b38 <_rtc_bss_rtc_end+0xaffe7b10>
        e:	0c0000               	excw
-      11:	0056b5               	call12	57c <XtExcFrameSize+0x50c>
-      14:	b3d000               	movgez	a13, a0, a0
+      11:	005c33               	excw
+      14:	bacf00               	ceil.s	a12, f15, 0
[...]

And it is really long. With objdump -d the diff is this:

--- a
+++ b
@@ -1,5 +1,5 @@
 
-a.elf:     file format elf32-xtensa-le
+b.elf:     file format elf32-xtensa-le
 
 
 Disassembly of section .rtc.text:

So a white-space only change. The issue really is that the .debug section differs.

maribu · 2022-06-16T08:03:20Z

#18216 should fix the issue *fingers crossed*

chrysn · 2022-06-30T15:33:24Z

CI reports:

Building application "tests_pkg_fff" for "esp32-heltec-lora32-v2" with MCU "esp32".

[...]
startup.c: In function 'system_init':
startup.c:302:5: error: static assertion failed: "ESP32 requires at least 3 threads, esp_timer, idle, and main"
     static_assert(MAXTHREADS >= 3,
     ^~~~~~~~~~~~~

If I understand this correctly, where this is tested (esp32/startup.c) has different conditions than when it is set (filter arch_esp32,$(FEATURES_USED)) in the fff pkg -- is there a better criterion for the Makefiles?

maribu · 2022-07-04T12:42:47Z

Interesting, I cannot reproduce (after a rebase) the issue Murdock found. Let me push the rebase and give it another round

maribu · 2022-07-06T06:41:40Z

Aha, it fails withs TEST_KCONFIG=1 because FEATURES_USED will be empty then.

ESP32 has an esp_timer thread in addition to the main and the idle thread. So applications that work fine with 2 threads on other platforms will break on ESP32.

maribu · 2022-07-06T10:14:00Z

Hooray! All green now :)

chrysn

Thanks, LGTM.

chrysn · 2022-07-06T11:23:51Z

As this is supposed to fix tests that currently fail, you could enable tests for merging. (But there might still be other unrelated failures -- I don''t get the good overview from mobile).

maribu · 2022-07-06T15:57:43Z

Wow, all tests succeeded :) Citing https://ci.riot-os.org/RIOT-OS/RIOT/18210/026854d788a4038e28549f577170dfc1952a01be/output/run_test/tests/pkg_fff/esp32-wroom-32:gnu.txt for the most relevant one:

-- running on worker pi-67e8efb7 thread 1, build number 11193.
-- executing tests for tests/pkg_fff on esp32-wroom-32 (compiled with gnu toolchain):
- executing hook run_test_pre
- resetting USB power...
- waiting for tty to re-attach...
- hook run_test_pre finished
make: Entering directory '/tmp/dwq.0.356379192510457/f78098f9135f81be87c0eee9eec79ad9/tests/pkg_fff'
esptool.py --chip esp32 --port /dev/ttyUSB0 --baud 460800 --before default_reset write_flash -z --flash_mode dout --flash_freq 40m    --flash_size detect 0x1000 /tmp/dwq.0.356379192510457/f78098f9135f81be87c0eee9eec79ad9/tests/pkg_fff/bin/esp32-wroom-32/esp_bootloader/bootloader.bin 0x8000 /tmp/dwq.0.356379192510457/f78098f9135f81be87c0eee9eec79ad9/tests/pkg_fff/bin/esp32-wroom-32/partitions.bin 0x10000 /tmp/dwq.0.356379192510457/f78098f9135f81be87c0eee9eec79ad9/tests/pkg_fff/bin/esp32-wroom-32/tests_pkg_fff.elf.bin
esptool.py v2.6
Serial port /dev/ttyUSB0
Connecting........_
Chip is ESP32D0WDQ6 (revision 1)
Features: WiFi, BT, Dual Core, 240MHz, VRef calibration in efuse, Coding Scheme None
MAC: 30:ae:a4:f4:7a:e0
Uploading stub...
Running stub...
Stub running...
Changing baud rate to 460800
Changed.
Configuring flash size...
Auto-detected Flash size: 4MB
Compressed 15936 bytes to 11160...

Writing at 0x00001000... (100 %)
Wrote 15936 bytes (11160 compressed) at 0x00001000 in 0.3 seconds (effective 417.8 kbit/s)...
Hash of data verified.
Compressed 3072 bytes to 85...

Writing at 0x00008000... (100 %)
Wrote 3072 bytes (85 compressed) at 0x00008000 in 0.0 seconds (effective 3807.3 kbit/s)...
Hash of data verified.
Compressed 101344 bytes to 42502...

Writing at 0x00010000... (33 %)
Writing at 0x00014000... (66 %)
Writing at 0x00018000... (100 %)
Wrote 101344 bytes (42502 compressed) at 0x00010000 in 1.3 seconds (effective 618.9 kbit/s)...
Hash of data verified.

Leaving...
Hard resetting via RTS pin...
make: Nothing to be done for 'termdeps'.
make: Leaving directory '/tmp/dwq.0.356379192510457/f78098f9135f81be87c0eee9eec79ad9/tests/pkg_fff'
make: Entering directory '/tmp/dwq.0.356379192510457/f78098f9135f81be87c0eee9eec79ad9/tests/pkg_fff'
r
socat - open:/dev/ttyUSB0,b115200,echo=0,raw,cs8,parenb=0,cstopb=0 
32 kHz XTAL not found, switching to internal 150 kHz oscillator


Help: Press s to start test, r to print it is ready
READY
s
START
main(): This is RIOT! (Version: buildtest)
Testing fff
FFF test completed: SUCCESS
{ "All tests successful

make: Leaving directory '/tmp/dwq.0.356379192510457/f78098f9135f81be87c0eee9eec79ad9/tests/pkg_fff'
-- saving test result to cache: OK

maribu · 2022-07-06T15:57:58Z

Thx everyone :)

maribu requested review from MichelRottleuthner, aabadie, fjmolinas, gschorcht, leandrolanzieri and smlng as code owners June 15, 2022 08:02

github-actions bot added the Area: cpu Area: CPU/MCU ports label Jun 15, 2022

jenswet mentioned this pull request Jun 15, 2022

tests/pkg_fff: increase MAXTHREADS #18211

Closed

gschorcht reviewed Jun 15, 2022

View reviewed changes

maribu force-pushed the cpu/esp32 branch from ad07a58 to b3cd15f Compare June 15, 2022 10:12

This was referenced Jun 30, 2022

Add some Rust library building infrastructure #16833

Merged

gcoap_dns: initial import of a DNS over CoAP (DoC) client #16705

Merged

maribu force-pushed the cpu/esp32 branch from b3cd15f to bd3d3ed Compare July 4, 2022 12:42

maribu added 3 commits July 6, 2022 09:00

tests/pkg_fff: Account for extra thread needed for ESP32

91afd31

tests/periph_gpio_ll: Account for extra thread needed for ESP32

f59e910

cpu/esp32: make sure MAXTHREADS is at least 3

026854d

ESP32 has an esp_timer thread in addition to the main and the idle thread. So applications that work fine with 2 threads on other platforms will break on ESP32.

maribu force-pushed the cpu/esp32 branch from c05d878 to 026854d Compare July 6, 2022 07:00

chrysn approved these changes Jul 6, 2022

View reviewed changes

maribu merged commit 29d2193 into RIOT-OS:master Jul 6, 2022

maribu deleted the cpu/esp32 branch July 6, 2022 15:57

chrysn added this to the Release 2022.07 milestone Aug 25, 2022

Conversation

maribu commented Jun 15, 2022

Contribution description

Testing procedure

Issues/PRs references

Uh oh!

maribu commented Jun 15, 2022

Uh oh!

gschorcht Jun 15, 2022

Choose a reason for hiding this comment

Uh oh!

gschorcht commented Jun 16, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

maribu commented Jun 16, 2022

Uh oh!

maribu commented Jun 16, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chrysn commented Jun 30, 2022

Uh oh!

maribu commented Jul 4, 2022

Uh oh!

maribu commented Jul 6, 2022

Uh oh!

maribu commented Jul 6, 2022

Uh oh!

chrysn left a comment

Choose a reason for hiding this comment

Uh oh!

chrysn commented Jul 6, 2022

Uh oh!

maribu commented Jul 6, 2022

Uh oh!

maribu commented Jul 6, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

gschorcht commented Jun 16, 2022 •

edited

Loading

maribu commented Jun 16, 2022 •

edited

Loading