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

add idfTarget to project-conf editor #983

Merged
merged 7 commits into from
Jun 15, 2023

Conversation

brianignacio5
Copy link
Collaborator

Description

Add IDF Target to Project Configuration editor and object.

Move project configuration logic to project-specific configuration file.

Type of change

  • New feature (non-breaking change which adds functionality)

Steps to test this pull request

Provide a list of steps to test changes in this PR and required output

  1. Click on "ESP-IDF: Open Project Configuration"
  2. Add a new profile using IDF Target and monitor baud rate
  3. Try using extension command after selecting the new profile and see if values are being used.
  • Expected behaviour:

  • Expected output:

How has this been tested?

Test Configuration:

  • ESP-IDF Version: 5.0
  • OS (Windows,Linux and macOS): macOS

Checklist

  • PR Self Reviewed
  • Applied Code formatting
  • Added Documentation
  • Added Unit Test
  • Verified on all platforms - Windows,Linux and macOS

@brianignacio5 brianignacio5 self-assigned this May 30, 2023
@github-actions
Copy link

github-actions bot commented May 31, 2023

Download the artifacts for this pull request:

@brianignacio5 brianignacio5 mentioned this pull request Jun 1, 2023
5 tasks
@brianignacio5 brianignacio5 mentioned this pull request Jun 5, 2023
5 tasks
Comment on lines 33 to 34
In the ESP-IDF CMake build system, the project configuration settings are saved using the `idf.py menuconfig` (or the SDK Configuration editor in IDEs) which store these values in a `sdkconfig` file. This file is used by the build system and the source code. When the current ESP-IDF project is under version control system, the `sdkconfig` can change on any user build due to many local environment configuration which can alter the project expected behavior. For such a reason is better to move those project specific settings to an `sdkconfig.defaults` which is not modified by the build system and `sdkconfig` can be added to the `.gitignore` list. This `sdkconfig.defaults` can be generated by `idf.py save-defconfig`.

Copy link

@chipweinberger chipweinberger Jun 5, 2023

Choose a reason for hiding this comment

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

  • I would make it more clear when you are talking about ./build/sdkconfig and when you are talking about ./sdkconfig.

  • "the sdkconfig" (which one?) "can change on any user build due to many local environment configuration which can alter the project expected behavior."


In the ESP-IDF CMake build system, the project configuration settings are saved using the `idf.py menuconfig` (or the SDK Configuration editor in IDEs) which store these values in a `sdkconfig` file. This file is used by the build system and the source code. When the current ESP-IDF project is under version control system, the `sdkconfig` can change on any user build due to many local environment configuration which can alter the project expected behavior. For such a reason is better to move those project specific settings to an `sdkconfig.defaults` which is not modified by the build system and `sdkconfig` can be added to the `.gitignore` list. This `sdkconfig.defaults` can be generated by `idf.py save-defconfig`.

The default case, without using any project configuration editor setting would be to create an `sdkconfig` file in the ESP-IDF project root directory. Initially the user would use the default case but later it would like to commit the project for version control. Since `sdkconfig` contains many values that are not specific to the project, the user would like to save only those values that are required for the project. These values would be the first **production** profile.

Choose a reason for hiding this comment

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

i think you could describe what is a project first.

side comment: maybe the name project is too general. What if the formal / full name was renamed to esp-project? or esp-vsc-project? Maybe it's okay as-is, but perhaps the concept needs more introduction.


2. Run the `idf.py save-defconfig` command to generate a `sdkconfig.defaults` file.

3. Use the `ESP-IDF: Open Project Configuration` and create a new profile with name `production`. Set `sdkconfig defaults` the previous `sdkconfig.defaults` file. If you want to separate generated binaries of this new **production** profile from the default case, specify a different build folder using the `Build Directory path` to something like `/path/to/current/project/build_production` and the `sdkconfig file path` to something like `/path/to/current/project/build_production/sdkconfig` to avoid mixing development sdkconfig with production sdkconfig files.

Choose a reason for hiding this comment

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

  • "sdkconfig file path" should be renamed "sdkconfig build path"

  • shouldn't sdkconfig defaults be set to sdkconfig.defaults + sdkconfig.prod?

  • i'm confused why it is called sdkconfig defaults. Shouldn't it be called sdkconfigPath? It does not have to be the sdkconfig.defaults file, does it?

  • "Use the 'ESP-IDF: Open Project
    Configuration' and create a new profile with name 'production'. Set sokconfig defaults' the previous 'sdkconfig.defaults' file, for example XXXXXXXXX"

  • "if you want to separate generated binaries..." should be rephrased, "Some people find it clearer to use a separate build folder for each project, to avoid confusion. for project 'prod' produces /build_prod/, and project YYYYYY for produces YYYYYY. This can be achieved by ...."

  • ^^ also, why not just make the above the default? I think 99% of people want this setting

Comment on lines 43 to 44
4. Create a new profile with name `development`. You can set the build folder using the `Build Directory path` to something like `/path/to/current/project/build_dev` and the `sdkconfig file path` to something like `/path/to/current/project/build_dev/sdkconfig` to avoid mixing development sdkconfig with production sdkconfig files.

Choose a reason for hiding this comment

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

but what should we set sdkconfig defaults to?


7. These profiles and each profile settings are going to be saved in the `/path/to/current/project/esp_idf_project_configuration.json`.

The production environment could be split into multiple production profiles, as it is done in the [ESP-IDF CMake Multiple configuration example](https://github.com/espressif/esp-idf/tree/master/examples/build_system/cmake/multi_config) and the [multiple configuration tutorial](./multiple_config.md) by separating common sdkconfig settings in a `sdkconfig.prod_common` and product specific settings in `sdkconfig.prod1` and `sdkconfig.prod2` respectively. Multiple sdkconfig defaults files can be specified in the project configuration editor profile `sdkconfig defaults` field as `sdkconfig.prod_common;sdkconfig.prod1` where the values are loaded in order as explained in [here](https://docs.espressif.com/projects/esp-idf/en/latest/esp32/api-guides/build-system.html?highlight=sdkconfig%20defaults#custom-sdkconfig-defaults).

Choose a reason for hiding this comment

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

Ahh, now i get it.

before you were describing a simpler approach with fully separate sdkconfig input files.

you need to move this section earlier!

@chipweinberger
Copy link

I read your updated docs and they are much more clear.

Nice job, thanks for making this Brian.

@brianignacio5 brianignacio5 merged commit 3c41028 into master Jun 15, 2023
@brianignacio5 brianignacio5 deleted the enhance/project-editor-conf-file branch June 15, 2023 02:26
@brianignacio5 brianignacio5 added this to the 1.6.4 milestone Jun 29, 2023
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.

2 participants