Skip to content

Add environment checks and update partition sizes in CMake#56

Open
JoelHer wants to merge 1 commit intomainfrom
fix/cmake-espidf
Open

Add environment checks and update partition sizes in CMake#56
JoelHer wants to merge 1 commit intomainfrom
fix/cmake-espidf

Conversation

@JoelHer
Copy link
Copy Markdown
Member

@JoelHer JoelHer commented Apr 17, 2026

We had this issue with cmake and esp-idf

[bookmarks] Loaded 0 bookmarks
[proc] Executing command: /usr/bin/cmake --version
[proc] Executing command: /usr/bin/cmake -E capabilities
[kit] Successfully loaded 1 kits from /home/joelh/.local/share/CMakeTools/cmake-tools-kits.json
[variant] Loaded new set of variants
[proc] Executing command: /usr/bin/gcc -v
[main] Configuring project: vigilant-engine 
[proc] Executing command: /usr/bin/cmake -DCMAKE_BUILD_TYPE:STRING=Debug -DCMAKE_EXPORT_COMPILE_COMMANDS:BOOL=TRUE -DCMAKE_C_COMPILER:FILEPATH=/usr/bin/gcc -DCMAKE_CXX_COMPILER:FILEPATH=/usr/bin/g++ --no-warn-unused-cli -S /home/joelh/Dev/vigilant-engine -B /home/joelh/Dev/vigilant-engine/build -G Ninja
[cmake] Not searching for unused variables given on the command line.
[cmake] -- Configuring incomplete, errors occurred!
[cmake] CMake Error at CMakeLists.txt:10 (include):
[cmake]   include could not find requested file:
[cmake] 
[cmake]     /tools/cmake/project.cmake
[cmake] 
[cmake] 
[cmake] CMake Error at CMakeLists.txt:13 (idf_build_set_property):
[cmake]   Unknown CMake command "idf_build_set_property".
[cmake] 
[cmake] 
[proc] The command: /usr/bin/cmake -DCMAKE_BUILD_TYPE:STRING=Debug -DCMAKE_EXPORT_COMPILE_COMMANDS:BOOL=TRUE -DCMAKE_C_COMPILER:FILEPATH=/usr/bin/gcc -DCMAKE_CXX_COMPILER:FILEPATH=/usr/bin/g++ --no-warn-unused-cli -S /home/joelh/Dev/vigilant-engine -B /home/joelh/Dev/vigilant-engine/build -G Ninja exited with code: 1

This pull request introduces several improvements and fixes related to build environment validation, partition layout, documentation, and test infrastructure. The most significant changes are the addition of environment checks for ESP-IDF, updates to the partition table and associated documentation, and the introduction of a basic unit test to verify the test framework setup.

This was entirely made by AI, so i dont actually know if this is the best way to fix the issues, but it works lol.

Build system and environment validation:

  • Added explicit checks in both CMakeLists.txt and vigilant-engine-recovery/CMakeLists.txt to ensure IDF_PATH is set, preventing builds from running without the ESP-IDF environment. Improved error messages guide users to use the ESP-IDF shell or VS Code extension. [1] [2]
  • Set a default IDF_TARGET to esp32c6 in vigilant-engine-recovery/CMakeLists.txt if not already defined, improving build reliability.

Partition layout and documentation updates:

  • Updated partitions.csv to increase the factory partition size and adjust the ota_0 partition start and size, reflecting these changes in docs/partitions.md. [1] [2]
  • Updated the recovery firmware documentation and build system to use the new output path for the embedded recovery HTML file. [1] [2]

Documentation improvements:

  • Enhanced troubleshooting documentation to clarify the need for the ESP-IDF environment and recommend the VS Code ESP-IDF extension over generic CMake Tools.

For later test infrastructure:

  • Added a minimal Unity test in test_vigilant_engine.c to confirm that the test framework is set up correctly.…CMake and documentation
  • Added checks for IDF_PATH in CMakeLists.txt for both main and recovery builds.
  • Updated partition sizes in partitions.md and partitions.csv for accuracy.
  • Enhanced troubleshooting documentation for ESP-IDF environment setup.
  • Introduced unit test for status_led component.

…CMake and documentation

- Added checks for IDF_PATH in CMakeLists.txt for both main and recovery builds.
- Updated partition sizes in partitions.md and partitions.csv for accuracy.
- Enhanced troubleshooting documentation for ESP-IDF environment setup.
- Introduced unit test for status_led component.
@JoelHer JoelHer requested review from Kampfdackel5 and tillx4 April 17, 2026 11:20
@JoelHer JoelHer self-assigned this Apr 17, 2026
@JoelHer JoelHer added the bug Something isn't working label Apr 17, 2026
@JoelHer JoelHer marked this pull request as ready for review April 17, 2026 11:20
Comment thread docs/partitions.md
- `factory`: Recovery firmware (1 MB at `0x10000`)
- `ota_0`: Main firmware (~2.94 MB at `0x110000`)
- `factory`: Recovery firmware (1.1875 MB at `0x10000`)
- `ota_0`: Main firmware (2.75 MB at `0x140000`)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Idk if it makes sense to include these if they change everytime

#include "unity.h"
#include "status_led.h"

TEST_CASE("Unity test framework is running", "[status_led]")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Isn't it called "unit test"? Or are we planning on using some external framework?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Unity is the "official" recommended test framework for testing by espressif


TEST_CASE("Unity test framework is running", "[status_led]")
{
TEST_ASSERT_TRUE(1);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Question is if unnecessary tests don't add too much bloat / if the tradeoff is given

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This is just a test test, and test are only flashed to an esp when you need to test code

endif()

if(NOT DEFINED IDF_TARGET AND NOT DEFINED ENV{IDF_TARGET})
set(IDF_TARGET "esp32c6")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

As someone who doesn't test with esp32c6 I am not a fan haha

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

xD

Comment thread partitions.csv
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Now I don't know about these. Are we sure?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants