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

improvements to esp32 build configuration #1163

Draft
wants to merge 2 commits into
base: release-0.6
Choose a base branch
from

Conversation

UncleGrumpy
Copy link
Collaborator

To streamline the esp32 build process and simplify deploying custom built images, idf.py build will now create the complete atomvm-${target}.img, including the libraries, if esp32boot.avm is found from a previously run generic_unix build. To help minimize confusion for users, there is also a prominent message with instruction for flashing the complete image.

Theses changes also expose cmake build options in the idf.py menuconfig utility.

These changes are made under both the "Apache 2.0" and the "GNU Lesser General
Public License 2.1 or later" license terms (dual license).

SPDX-License-Identifier: Apache-2.0 OR LGPL-2.1-or-later

@UncleGrumpy UncleGrumpy changed the title Enhancements to esp32 build configuration improvements to esp32 build configuration May 27, 2024
Rather than having the user or CI scripts run the extra ./build/mkimage.sh
sript, it will be run automatically if the `esp32boot.avm` libraries are found
in the `AtomVM/build/libs/esp32boot` directory from a previous generic_unix
build. This includes a prominet message for the user with instruction to flash
the complete image to the device.

Updates the esp32-build.yaml workflow to use the updated build steps, and only
create the necessary esp32 related libraries, excluding the creation of uf2 files.

Signed-off-by: Winford <winford@object.stream>
Exposes the settable cmake build options for the esp32 platform to the `idf.py
menuconfig` utility.

Signed-off-by: Winford <winford@object.stream>
@petermm
Copy link
Contributor

petermm commented Jun 10, 2024

Not sure I like this bundling of commands:

  1. Imho it's a coupling, implicit behaviour change - which might hinder future things, like point 2..

  2. In a simulator flow I use: idf.py build && touch ~/path_to_project/atomvm_wokwi_bins/flasher_args.json (as to build and then reboot sim) - and having mkimage in there would slow things down.

But I'm also a poweruser of idf.py build && ./build/mkimage.sh - so I get the drift, but not sure I understand how it simplifies deploying custom images..

Copy link
Collaborator

@bettio bettio left a comment

Choose a reason for hiding this comment

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

I'm not sure if it is an addition that fills some kind of huge gap in v0.6 (or not), so regardless to the outcome of this discussion I would target main branch anyway.

@UncleGrumpy
Copy link
Collaborator Author

@petermm, I don’t want to disrupt anyone’s workflows. My goal was just to streamline the process for newer users who need to build a custom image to include additional components, like atomvm_mqtt. It makes more sense for me to add an additional cmake job called “deploy” (or similar) that will build the complete image and flash it to the target device using the generated ./build/flasjimage.sh script, leaving the default “build” task unchanged, this will better my goal and should have no affect on any pre-existing workflows.

@UncleGrumpy UncleGrumpy marked this pull request as draft June 14, 2024 15:40
@@ -11,6 +11,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Fix bug (with code compiled with OTP-21) with binary pattern matching: the fix introduced with
`02411048` was not completely right, and it was converting match context to bogus binaries.

### Changed

- ESP32 builds now assemble the complet image automatically if `esp32boot.avm` is found from a previous generic_unix build.
Copy link
Collaborator

Choose a reason for hiding this comment

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

s/complet/complete/

@pguyot
Copy link
Collaborator

pguyot commented Jun 22, 2024

Should we keep a way to not create the image? (it's not much longer anyway). When working on the VM, I usually only reflash with idf.py flash which is faster than flashing the whole image.

Also, one of my workflows (la machine, obviously) does not include esp32boot.avm at all because it seems to me that esp32boot.avm doesn't bring anything to production images. I'll definitely work around this improvement for newcomers, though.

Likewise, qemu-based tests only use artifacts created by idf.py build (the VM, the partition map and the bootloader) and not the full image.

@UncleGrumpy
Copy link
Collaborator Author

UncleGrumpy commented Jun 23, 2024

Should we keep a way to not create the image?

I converted this back to draft, because I realize after @petermm’s comments that could be intrusive on existing workflows, so instead I will create and document a new build task that can be used as an alternative to idf.py build that will create the ready to deploy image, or even flash the image after building so that flashing a new custom image can be achieved with one command.

@pguyot
Copy link
Collaborator

pguyot commented Jun 23, 2024

Should we keep a way to not create the image?

I converted this back to draft, because I realize after @petermm’s comments that could be intrusive on existing workflows, so instead I will create and document a new build task that can be used as an alternative to idf.py build that will create the ready to deploy image, or even flash the image after building so that flashing a new custom image can be achieved with one command.

This sounds like a better approach indeed and would be very welcome. I also find it frustrating to have to call the bash script to build the image when I need it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants