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

make/esptool: fix FFLAGS inclusion order for qemu #16059

Merged
merged 1 commit into from
Feb 22, 2021

Conversation

aabadie
Copy link
Contributor

@aabadie aabadie commented Feb 22, 2021

Contribution description

This PR fixes the inclusion order of FFLAGS with the esptool when using the esp_qemu module. In master, this is currently broken (probably because of #15475).

The fix proposed by this PR is to move the logic from esp32 to esptool.inc.mk, which is now included after the cpu Makefile.include is included.

This PR is a quick fix and IMHO a more proper fix is to reuse the EMULATE=1 variable and integrate the esp emulator into the qemu.inc.mk mechanism, instead of relying on the module mechanism. I leave this as a follow-up.

Another cleanup that is worth is to get rid of all this commands configured in the FFLAGS variable, this is an abuse of this variable that should only contain parameters of the flasher tool (configured in FLASHER). I'm also leaving this as a follow-up.

Testing procedure

  • Try to flash an application while using USEMODULE=esp_qemu: in master this fails, with this PR it works:
this PR:
$ USEMODULE=esp_qemu make BOARD=esp32-wroom-32 -C examples/hello-world flash-only --no-print-directory 
ESP32_SDK_DIR should be defined as /path/to/esp-idf directory
ESP32_SDK_DIR is set by default to /opt/esp/esp-idf
/work/riot/RIOT/dist/tools/esptool/esptool.py --chip esp32 elf2image --flash_mode dout --flash_size 4MB --flash_freq 40m     -o /work/riot/RIOT/examples/hello-world/bin/esp32-wroom-32/hello-world.elf.bin /work/riot/RIOT/examples/hello-world/bin/esp32-wroom-32/hello-world.elf; printf "\n" > /work/riot/RIOT/examples/hello-world/bin/esp32-wroom-32/partitions.csv; printf "nvs, data, nvs, 0x9000, 0x6000\n" >> /work/riot/RIOT/examples/hello-world/bin/esp32-wroom-32/partitions.csv; printf "phy_init, data, phy, 0xf000, 0x1000\n" >> /work/riot/RIOT/examples/hello-world/bin/esp32-wroom-32/partitions.csv; printf "factory, app, factory, 0x10000, " >> /work/riot/RIOT/examples/hello-world/bin/esp32-wroom-32/partitions.csv; ls -l /work/riot/RIOT/examples/hello-world/bin/esp32-wroom-32/hello-world.elf.bin | awk '{ print $5 }' >> /work/riot/RIOT/examples/hello-world/bin/esp32-wroom-32/partitions.csv; python3 /work/riot/RIOT/dist/tools/esptool/gen_esp32part.py --verify /work/riot/RIOT/examples/hello-world/bin/esp32-wroom-32/partitions.csv /work/riot/RIOT/examples/hello-world/bin/esp32-wroom-32/partitions.bin
esptool.py v2.4.0
Parsing CSV input...
dd if=/dev/zero bs=1M count=4 | tr "\\000" "\\377" > tmp.bin && cat tmp.bin | head -c $((0x1000)) | cat - /work/riot/RIOT/cpu/esp32/bin/bootloader.bin tmp.bin | head -c $((0x8000)) | cat - /work/riot/RIOT/examples/hello-world/bin/esp32-wroom-32/partitions.bin tmp.bin | head -c $((0x10000)) | cat - /work/riot/RIOT/examples/hello-world/bin/esp32-wroom-32/hello-world.elf.bin tmp.bin | head -c 4MB > /work/riot/RIOT/examples/hello-world/bin/esp32-wroom-32/esp32flash.bin && rm tmp.bin; cp /work/riot/RIOT/cpu/esp32/bin/rom_0x3ff90000_0x00010000.bin /work/riot/RIOT/examples/hello-world/bin/esp32-wroom-32/rom1.bin && cp /work/riot/RIOT/cpu/esp32/bin/rom_0x40000000_0x000c2000.bin /work/riot/RIOT/examples/hello-world/bin/esp32-wroom-32/rom.bin
4+0 records in
4+0 records out
4194304 bytes (4,2 MB, 4,0 MiB) copied, 0,0091344 s, 459 MB/s
master:
$ USEMODULE=esp_qemu make BOARD=esp32-wroom-32 -C examples/hello-world flash-only --no-print-directory 
ESP32_SDK_DIR should be defined as /path/to/esp-idf directory
ESP32_SDK_DIR is set by default to /opt/esp/esp-idf
/work/riot/RIOT/dist/tools/esptool/esptool.py --chip esp32 elf2image --flash_mode dout --flash_size 4MB --flash_freq 40m     -o /work/riot/RIOT/examples/hello-world/bin/esp32-wroom-32/hello-world.elf.bin /work/riot/RIOT/examples/hello-world/bin/esp32-wroom-32/hello-world.elf; printf "\n" > /work/riot/RIOT/examples/hello-world/bin/esp32-wroom-32/partitions.csv; printf "nvs, data, nvs, 0x9000, 0x6000\n" >> /work/riot/RIOT/examples/hello-world/bin/esp32-wroom-32/partitions.csv; printf "phy_init, data, phy, 0xf000, 0x1000\n" >> /work/riot/RIOT/examples/hello-world/bin/esp32-wroom-32/partitions.csv; printf "factory, app, factory, 0x10000, " >> /work/riot/RIOT/examples/hello-world/bin/esp32-wroom-32/partitions.csv; ls -l /work/riot/RIOT/examples/hello-world/bin/esp32-wroom-32/hello-world.elf.bin | awk '{ print $5 }' >> /work/riot/RIOT/examples/hello-world/bin/esp32-wroom-32/partitions.csv; python3 /work/riot/RIOT/dist/tools/esptool/gen_esp32part.py --verify /work/riot/RIOT/examples/hello-world/bin/esp32-wroom-32/partitions.csv /work/riot/RIOT/examples/hello-world/bin/esp32-wroom-32/partitions.bin
esptool.py v2.4.0
Parsing CSV input...
dd cp /work/riot/RIOT/cpu/esp32/bin/rom_0x3ff90000_0x00010000.bin /work/riot/RIOT/examples/hello-world/bin/esp32-wroom-32/rom1.bin && cp /work/riot/RIOT/cpu/esp32/bin/rom_0x40000000_0x000c2000.bin /work/riot/RIOT/examples/hello-world/bin/esp32-wroom-32/rom.bin if=/dev/zero bs=1M count=4 | tr "\\000" "\\377" > tmp.bin && cat tmp.bin | head -c $((0x1000)) | cat - /work/riot/RIOT/cpu/esp32/bin/bootloader.bin tmp.bin | head -c $((0x8000)) | cat - /work/riot/RIOT/examples/hello-world/bin/esp32-wroom-32/partitions.bin tmp.bin | head -c $((0x10000)) | cat - /work/riot/RIOT/examples/hello-world/bin/esp32-wroom-32/hello-world.elf.bin tmp.bin | head -c 4MB > /work/riot/RIOT/examples/hello-world/bin/esp32-wroom-32/esp32flash.bin && rm tmp.bin;
dd: unrecognized operand ‘cp’
Try 'dd --help' for more information.
make: *** [/work/riot/RIOT/examples/hello-world/../../Makefile.include:714: flash-only] Error 1

=> one can see that the cp commands related to qemu are called before the other commands configured in the FFLAGS, so, right after dd, which is the flasher in this case. This is why it fails badly.

Issues/PRs references

Noticed that problem while working on #15970

@aabadie aabadie added Area: build system Area: Build system Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation Platform: ESP Platform: This PR/issue effects ESP-based platforms labels Feb 22, 2021
@aabadie aabadie requested a review from fjmolinas February 22, 2021 09:47
@aabadie aabadie requested a review from gschorcht as a code owner February 22, 2021 09:47
@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 Feb 22, 2021
@benpicco benpicco added the Impact: minor The PR is small in size and might only require a quick look of a knowledgeable reviewer label Feb 22, 2021
@aabadie aabadie merged commit 2418561 into RIOT-OS:master Feb 22, 2021
@aabadie aabadie deleted the pr/make/esptool_qemu_fix branch February 22, 2021 13:19
@kaspar030 kaspar030 added this to the Release 2021.04 milestone Apr 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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 Impact: minor The PR is small in size and might only require a quick look of a knowledgeable reviewer 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.

3 participants