Skip to content

Conversation

aabadie
Copy link
Contributor

@aabadie aabadie commented May 5, 2023

Contribution description

This PR is similar as #19435 and #19551 but applied to periph test applications.

Testing procedure

Green CI

Issues/PRs references

#19435 and #19551

@aabadie aabadie added the Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation label May 5, 2023
@github-actions github-actions bot added Area: build system Area: Build system Area: doc Area: Documentation Area: Kconfig Area: Kconfig integration Area: tests Area: tests and testing framework labels May 5, 2023
@aabadie aabadie requested a review from gschorcht May 5, 2023 08:33
@aabadie aabadie added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label May 5, 2023
@riot-ci
Copy link

riot-ci commented May 5, 2023

Murdock results

✔️ PASSED

da95d2c treewide: replace occurrences of tests/periph_ with new path

Success Failures Total Runtime
6931 0 6931 08m:38s

Artifacts

@aabadie aabadie force-pushed the pr/tests/periph_move branch 2 times, most recently from 155396f to ccf61fb Compare May 5, 2023 09:56
@gschorcht
Copy link
Contributor

gschorcht commented May 5, 2023

The same here tests/periphs versus tests/periph, although this does not really correspond to the directory structure of the source code.

@aabadie
Copy link
Contributor Author

aabadie commented May 5, 2023

The same here tests/periphs versus tests/periph, although this does not really correspond to the directory structure of the source code.

How about we move it to tests/drivers/periph ?

@gschorcht
Copy link
Contributor

I was also thinking about that. On one hand it would reflect the source code directory structure. On the other hand this single directory would be one in between all the driver directories which would make navigation more difficult. I don't have strong opinion about that but I would tend to leave it in tests/periph.

@aabadie aabadie force-pushed the pr/tests/periph_move branch from ccf61fb to ae554d2 Compare May 5, 2023 16:03
@aabadie
Copy link
Contributor Author

aabadie commented May 5, 2023

I would tend to leave it in tests/periph.

I just did that

@aabadie aabadie force-pushed the pr/tests/periph_move branch from ae554d2 to da95d2c Compare May 6, 2023 13:35
@aabadie aabadie requested review from dylad and kaspar030 as code owners May 6, 2023 13:35
@github-actions github-actions bot added Area: boards Area: Board ports Area: CI Area: Continuous Integration of RIOT components labels May 6, 2023
@github-actions github-actions bot added Area: cpu Area: CPU/MCU ports Platform: ESP Platform: This PR/issue effects ESP-based platforms labels May 6, 2023
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 May 8, 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 fcb8283 into RIOT-OS:master May 8, 2023
@aabadie aabadie deleted the pr/tests/periph_move branch May 8, 2023 12:43
bors bot added a commit that referenced this pull request Jul 2, 2023
19785: cpu/esp8266: fix region overflow with '*periph' directory in app path r=gschorcht a=hugueslarrive

### Contribution description
This pull request addresses an issue where a region overflow occurs during the generation of the `.elf` file when the application path contains a directory named '*periph'. The problem arises due to an incorrect section mapping in the `esp8266.riot-os.ld` linker script.

The issue was causing the compilation of all tests which were moved to the `tests/periph` directory by PR #19552 to fail on the ESP8266 architecture.

To resolve the issue, the `esp8266.riot-os.ld` script has been modified. The section mapping for the `*periph` directory has been changed to `*esp_common_periph`, ensuring that the correct sections are included in the final binary.

### Testing procedure
To verify the effectiveness of this fix, the following testing procedure was performed:
1. Checked out the `master` branch of the RIOT repository
2. Cleaned and built the `blinky` example for the `esp8266-esp-12x` board:
- **Expected Result**: The build process completes successfully without any region overflow errors. The resulting `blinky.elf` file is generated.
```
hugues@p700:~/github/riot_small_fixes/RIOT$ BOARD=esp8266-esp-12x make -j64 -C examples/blinky clean all | grep -v make
rm -rf /home/hugues/github/riot_small_fixes/RIOT/examples/blinky/bin/esp8266-esp-12x/pkg-build/esp8266_sdk
Building application "blinky" for "esp8266-esp-12x" with MCU "esp8266".

esptool.py v2.4.0
   text	   data	    bss	    dec	    hex	filename
 278112	   5228	  28640	 311980	  4c2ac	/home/hugues/github/riot_small_fixes/RIOT/examples/blinky/bin/esp8266-esp-12x/blinky.elf
esptool.py v2.4.0
Parsing CSV input...
```
3. Created a directory named `somethingperiph` and copied the `blinky` example into it:
```
mkdir somethingperiph && cp -a examples/blinky/ somethingperiph
```
4. Cleaned and attempted to build the `blinky` example in the `somethingperiph` directory:
- **Expected Result**: The build process fails with a linker error, indicating a region overflow issue.
```
hugues@p700:~/github/riot_small_fixes/RIOT$ BOARD=esp8266-esp-12x make -j64 -C somethingperiph/blinky clean all | grep -v make
Building application "blinky" for "esp8266-esp-12x" with MCU "esp8266".
rm -rf /home/hugues/github/riot_small_fixes/RIOT/somethingperiph/blinky/bin/esp8266-esp-12x/pkg-build/esp8266_sdk

esptool.py v2.4.0
/home/hugues/esp/xtensa-esp8266-elf/bin/../lib/gcc/xtensa-esp8266-elf/5.2.0/../../../../xtensa-esp8266-elf/bin/ld: /home/hugues/github/riot_small_fixes/RIOT/somethingperiph/blinky/bin/esp8266-esp-12x/blinky.elf section `.text' will not fit in region `iram1_0_seg'
/home/hugues/esp/xtensa-esp8266-elf/bin/../lib/gcc/xtensa-esp8266-elf/5.2.0/../../../../xtensa-esp8266-elf/bin/ld: region `iram1_0_seg' overflowed by 24372 bytes
collect2: error: ld returned 1 exit status
make: *** [/home/hugues/github/riot_small_fixes/RIOT/somethingperiph/blinky/../../Makefile.include:761 : /home/hugues/github/riot_small_fixes/RIOT/somethingperiph/blinky/bin/esp8266-esp-12x/blinky.elf] Erreur 1
```
5. Checked out this PR's branch
6. Cleaned and attempted to build the `blinky` example in the `somethingperiph` directory again:
- **Expected Result**: The build process completes successfully without any region overflow errors. The resulting `blinky.elf` file is generated.
```
hugues@p700:~/github/riot_small_fixes/RIOT$ BOARD=esp8266-esp-12x make -j64 -C somethingperiph/blinky clean all | grep -v make
Building application "blinky" for "esp8266-esp-12x" with MCU "esp8266".
rm -rf /home/hugues/github/riot_small_fixes/RIOT/somethingperiph/blinky/bin/esp8266-esp-12x/pkg-build/esp8266_sdk

esptool.py v2.4.0
   text	   data	    bss	    dec	    hex	filename
 278080	   5228	  28640	 311948	  4c28c	/home/hugues/github/riot_small_fixes/RIOT/somethingperiph/blinky/bin/esp8266-esp-12x/blinky.elf
esptool.py v2.4.0
Parsing CSV input...
```
7. Verified the size of the `blinky.elf` file:
- **Expected Result**: The size of the `blinky.elf` file is similar to the size obtained in step 2, indicating a successful build. However, a slight difference in size was observed due to the linker script modification.

8. Cleaned and attempted to build the `blinky` example in the `examples` directory again:
- **Expected Result**: The size of the `blinky.elf` file is similar to the size obtained in step 6.
```
hugues@p700:~/github/riot_small_fixes/RIOT$ BOARD=esp8266-esp-12x make -j64 -C examples/blinky clean all | grep -v make
Building application "blinky" for "esp8266-esp-12x" with MCU "esp8266".
rm -rf /home/hugues/github/riot_small_fixes/RIOT/examples/blinky/bin/esp8266-esp-12x/pkg-build/esp8266_sdk

esptool.py v2.4.0
   text	   data	    bss	    dec	    hex	filename
 278080	   5228	  28640	 311948	  4c28c	/home/hugues/github/riot_small_fixes/RIOT/examples/blinky/bin/esp8266-esp-12x/blinky.elf
esptool.py v2.4.0
Parsing CSV input...
```
The tests confirm that this pull request resolves the region overflow issue when the application path contains a directory named '*periph'. Additionally, all relevant tests in the `tests/periph` directory successfully compile again on the ESP8266 architecture:
```
hugues@p700:~/github/riot_small_fixes/RIOT$ for p in tests/periph/*; do BOARD=esp8266-esp-12x make -j64 -C $p clean all 2>&1 | grep elf; done
 279368	   5200	  28728	 313296	  4c7d0	/home/hugues/github/riot_small_fixes/RIOT/tests/periph/adc/bin/esp8266-esp-12x/tests_adc.elf
 277492	   5192	  28672	 311356	  4c03c	/home/hugues/github/riot_small_fixes/RIOT/tests/periph/cpuid/bin/esp8266-esp-12x/tests_cpuid.elf
 289044	   5264	  28856	 323164	  4ee5c	/home/hugues/github/riot_small_fixes/RIOT/tests/periph/gpio/bin/esp8266-esp-12x/tests_gpio.elf
 288884	   5288	  29160	 323332	  4ef04	/home/hugues/github/riot_small_fixes/RIOT/tests/periph/gpio_arduino/bin/esp8266-esp-12x/tests_gpio_arduino.elf
 279520	   5252	  28712	 313484	  4c88c	/home/hugues/github/riot_small_fixes/RIOT/tests/periph/hwrng/bin/esp8266-esp-12x/tests_hwrng.elf
 292312	   5268	  28872	 326452	  4fb34	/home/hugues/github/riot_small_fixes/RIOT/tests/periph/i2c/bin/esp8266-esp-12x/tests_i2c.elf
 284064	   5208	  28720	 317992	  4da28	/home/hugues/github/riot_small_fixes/RIOT/tests/periph/pm/bin/esp8266-esp-12x/tests_pm.elf
 285800	   5248	  28856	 319904	  4e1a0	/home/hugues/github/riot_small_fixes/RIOT/tests/periph/pwm/bin/esp8266-esp-12x/tests_pwm.elf
 281756	   5252	  28768	 315776	  4d180	/home/hugues/github/riot_small_fixes/RIOT/tests/periph/rtc/bin/esp8266-esp-12x/tests_rtc.elf
 278832	   5192	  28704	 312728	  4c598	/home/hugues/github/riot_small_fixes/RIOT/tests/periph/rtt/bin/esp8266-esp-12x/tests_rtt.elf
 280552	   5252	  28736	 314540	  4ccac	/home/hugues/github/riot_small_fixes/RIOT/tests/periph/rtt_min/bin/esp8266-esp-12x/tests_rtt_min.elf
 294164	   5296	  30104	 329564	  5075c	/home/hugues/github/riot_small_fixes/RIOT/tests/periph/spi/bin/esp8266-esp-12x/tests_spi.elf
 279440	   5192	  28800	 313432	  4c858	/home/hugues/github/riot_small_fixes/RIOT/tests/periph/timer/bin/esp8266-esp-12x/tests_timer.elf
 278884	   5192	  28712	 312788	  4c5d4	/home/hugues/github/riot_small_fixes/RIOT/tests/periph/timer_short_relative_set/bin/esp8266-esp-12x/tests_timer_short_relative_set.elf
 286756	   5248	  31920	 323924	  4f154	/home/hugues/github/riot_small_fixes/RIOT/tests/periph/uart/bin/esp8266-esp-12x/tests_uart.elf
 275572	   5184	  28800	 309556	  4b934	/home/hugues/github/riot_small_fixes/RIOT/tests/periph/uart_mode/bin/esp8266-esp-12x/tests_uart_mode.elf
hugues@p700:~/github/riot_small_fixes/RIOT$ git checkout master
Basculement sur la branche 'master'
Votre branche est à jour avec 'origin/master'.
hugues@p700:~/github/riot_small_fixes/RIOT$ for p in tests/periph/*; do BOARD=esp8266-esp-12x make -j64 -C $p clean all 2>&1 | grep Erreur; done
make: *** [/home/hugues/github/riot_small_fixes/RIOT/tests/periph/adc/../../../Makefile.include:761 : /home/hugues/github/riot_small_fixes/RIOT/tests/periph/adc/bin/esp8266-esp-12x/tests_adc.elf] Erreur 1
make: *** [/home/hugues/github/riot_small_fixes/RIOT/tests/periph/cpuid/../../../Makefile.include:761 : /home/hugues/github/riot_small_fixes/RIOT/tests/periph/cpuid/bin/esp8266-esp-12x/tests_cpuid.elf] Erreur 1
make: *** [/home/hugues/github/riot_small_fixes/RIOT/tests/periph/gpio/../../../Makefile.include:761 : /home/hugues/github/riot_small_fixes/RIOT/tests/periph/gpio/bin/esp8266-esp-12x/tests_gpio.elf] Erreur 1
make: *** [/home/hugues/github/riot_small_fixes/RIOT/tests/periph/gpio_arduino/../../../Makefile.include:761 : /home/hugues/github/riot_small_fixes/RIOT/tests/periph/gpio_arduino/bin/esp8266-esp-12x/tests_gpio_arduino.elf] Erreur 1
make: *** [/home/hugues/github/riot_small_fixes/RIOT/tests/periph/hwrng/../../../Makefile.include:761 : /home/hugues/github/riot_small_fixes/RIOT/tests/periph/hwrng/bin/esp8266-esp-12x/tests_hwrng.elf] Erreur 1
make: *** [/home/hugues/github/riot_small_fixes/RIOT/tests/periph/i2c/../../../Makefile.include:761 : /home/hugues/github/riot_small_fixes/RIOT/tests/periph/i2c/bin/esp8266-esp-12x/tests_i2c.elf] Erreur 1
make: *** [/home/hugues/github/riot_small_fixes/RIOT/tests/periph/pm/../../../Makefile.include:761 : /home/hugues/github/riot_small_fixes/RIOT/tests/periph/pm/bin/esp8266-esp-12x/tests_pm.elf] Erreur 1
make: *** [/home/hugues/github/riot_small_fixes/RIOT/tests/periph/pwm/../../../Makefile.include:761 : /home/hugues/github/riot_small_fixes/RIOT/tests/periph/pwm/bin/esp8266-esp-12x/tests_pwm.elf] Erreur 1
make: *** [/home/hugues/github/riot_small_fixes/RIOT/tests/periph/rtc/../../../Makefile.include:761 : /home/hugues/github/riot_small_fixes/RIOT/tests/periph/rtc/bin/esp8266-esp-12x/tests_rtc.elf] Erreur 1
make: *** [/home/hugues/github/riot_small_fixes/RIOT/tests/periph/rtt/../../../Makefile.include:761 : /home/hugues/github/riot_small_fixes/RIOT/tests/periph/rtt/bin/esp8266-esp-12x/tests_rtt.elf] Erreur 1
make: *** [/home/hugues/github/riot_small_fixes/RIOT/tests/periph/rtt_min/../../../Makefile.include:761 : /home/hugues/github/riot_small_fixes/RIOT/tests/periph/rtt_min/bin/esp8266-esp-12x/tests_rtt_min.elf] Erreur 1
make: *** [/home/hugues/github/riot_small_fixes/RIOT/tests/periph/spi/../../../Makefile.include:761 : /home/hugues/github/riot_small_fixes/RIOT/tests/periph/spi/bin/esp8266-esp-12x/tests_spi.elf] Erreur 1
make: *** [/home/hugues/github/riot_small_fixes/RIOT/tests/periph/timer/../../../Makefile.include:761 : /home/hugues/github/riot_small_fixes/RIOT/tests/periph/timer/bin/esp8266-esp-12x/tests_timer.elf] Erreur 1
make: *** [/home/hugues/github/riot_small_fixes/RIOT/tests/periph/timer_short_relative_set/../../../Makefile.include:761 : /home/hugues/github/riot_small_fixes/RIOT/tests/periph/timer_short_relative_set/bin/esp8266-esp-12x/tests_timer_short_relative_set.elf] Erreur 1
make: *** [/home/hugues/github/riot_small_fixes/RIOT/tests/periph/uart/../../../Makefile.include:761 : /home/hugues/github/riot_small_fixes/RIOT/tests/periph/uart/bin/esp8266-esp-12x/tests_uart.elf] Erreur 1
make: *** [/home/hugues/github/riot_small_fixes/RIOT/tests/periph/uart_mode/../../../Makefile.include:761 : /home/hugues/github/riot_small_fixes/RIOT/tests/periph/uart_mode/bin/esp8266-esp-12x/tests_uart_mode.elf] Erreur 1
```

### Issues/PRs references
Highlighted in #16727, appeared whith project tree as a side effect from #19552 but pre-existing.


Co-authored-by: Hugues Larrive <hlarrive@pm.me>
@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: boards Area: Board ports Area: build system Area: Build system Area: CI Area: Continuous Integration of RIOT components Area: cpu Area: CPU/MCU ports Area: doc Area: Documentation Area: Kconfig Area: Kconfig integration Area: tests Area: tests and testing framework CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Platform: ESP Platform: This PR/issue effects ESP-based platforms Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants