terjeio / grblHAL

This repo has moved to a new home https://github.com/grblHAL
232 stars 90 forks source link

ESP32: use ESP IDF CMake API #262

Closed kdomanski closed 3 years ago

kdomanski commented 3 years ago

This PR modifies the CMakeLists.txt files to use the ESP IDF CMake API. See also https://docs.espressif.com/projects/esp-idf/en/latest/esp32/api-guides/build-system.html

As a result, ESP IDF v4.x is now required to build.

Additionally, the PR introduces a Github Action that automatically builds the ESP32 version of GRBL in its default configuration and exports the built binary as an artifact.

terjeio commented 3 years ago

Thanks. I tried to set up VS Code for the ESP32 driver yesterday but id did not work out of the box (I left it spinning for hours on end to no avail). Unlike the VS Code setup for the RP2040 driver that just worked... Perhaps this PR will make it work?

The github action means that a ESP32 binary is always added to a checkout? If so then another reason to change to submodules and organize the drivers as separate repositories?

kdomanski commented 3 years ago

The github action means that a ESP32 binary is always added to a checkout? If so then another reason to change to submodules and organize the drivers as separate repositories?

Nope, it's just attached to the build job in the "Actions" tab. For free repos they also expire after a while (30 days)? I though it might be handy for debugging.

terjeio commented 3 years ago

Nope, it's just attached to the build job in the "Actions" tab

Ok, good.

I see that you have removed many config options from CMakeLists.txt but not all. Why only some? And sdkconfig should be moved up one level? VSCode put it there.

kdomanski commented 3 years ago

I see that you have removed many config options from CMakeLists.txt but not all. Why only some?

Oh, that's a copy-past mistake on my part. I'll fix it.

And sdkconfig should be moved up one level? VSCode put it there.

I don't know about that. When I build the driver locally, the toolchain updates some values in the existing sdkconfig.

kdomanski commented 3 years ago

Oh I see, I based the initial work on CMakeLists.txt on the master branch instead of test. Should be all right now.