PerMalmberg / Smooth

C++ framework for embedded programming on top of Espressif's ESP-IDF.
Apache License 2.0
325 stars 30 forks source link

build in parallel and some cleanup #156

Closed peterus closed 3 years ago

peterus commented 3 years ago

I did some small tasks in this pull request:

PerMalmberg commented 3 years ago

Should we take the opportunity to also set a specific IDF-version as discussed in #150 in this PR or shall we do that in a separate PR?

I'm thinking that using a .env file with the IDF-version would be the best way. That'd allow for a single place to set the IDF-version from which both docker build and docker-compose can pick it up, both when run locally and and in CI, assuming the file can be placed in a location found by both.

By updating docker.compose.yml, Dockerfile and build-docker-image.sh we could even allow easy switching between versions locally if someone wants to run against another version than what we set as default (v4.2?)

peterus commented 3 years ago

from an other thread I was reading that you like building against the latest master branch. So I was not considering this solution. But if you like to build against a dedicated IDF version I can definitely do this with the .env file. I like your idea with the .env file, with this everything is in one place.

PerMalmberg commented 3 years ago

I do like building against the master branch, but now that Espressif (allegedly) have fixed the PSRAM issues that were only available in master, that need has lessened. And, with the sudden upsurge in users of Smooth, it makes sense to build against an official release of IDF, while still giving the option to build against master by simply changing a file and building the image locally.

peterus commented 3 years ago

as you can see now, with the .env file it is very easy to switch versions. also changing the docker image name is now simple.

If it is okay for you I would try to implement some kind of check if there is a newer IDF image available, building the new image and uploading it to your docker hub or github packages if all tests are successful. of course i would need some kind of API key in the github actions environment.

edit: it would be even possible to try/check different IDF-versions on github actions.

PerMalmberg commented 3 years ago

Very nice.

I'm not sure automatically updating the image is the right way to go as it introduces a moving target in he build chain. However, a step that fails if there is a new release available could be a good thing if it isn't set to be a required success in the branch protection settings.

I like the idea of building against both latest and the latest major branch of IDF.

peterus commented 3 years ago

I had an idea which I want to present to you:

I like this approach very much as it is very clean and easy to maintain.

PerMalmberg commented 3 years ago

Oh, that's a really nice solution. Once Ubuntu 20.04 is available we can update the script to that.

peterus commented 3 years ago

Ubuntu 20.04 is already available in docker :) I updated the Dockerfile

edit: i just checked Dockerhub: there is even a 20.10 and 21.04 available. I will try the 21.04 in the end. edit: Ubuntu 21.04 looks like is not stable. In the CI there was a hickup with the unit test, locally there was an issue with apt-get update. So I switched to 20.10 which is running fine.

peterus commented 3 years ago

From my side I am now happy with the current changes. If you still miss something, I would be happy to hear them.

peterus commented 3 years ago

Ah stupid mistakes. I fixed them all.

PerMalmberg commented 3 years ago

Awesome, merged!