Closed alirana01 closed 1 month ago
[!WARNING]
Rate limit exceeded
@alirana01 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 5 minutes and 56 seconds before requesting another review.
How to resolve this issue?
After the wait time has elapsed, a review can be triggered using the `@coderabbitai review` command as a PR comment. Alternatively, push new commits to this PR. We recommend that you space out your commits to avoid hitting the rate limit.How do rate limits work?
CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our [FAQ](https://coderabbit.ai/docs/faq) for further information.Commits
Files that changed from the base of the PR and between 2dbc8e2eb4a7f3a971ec99a26f1e01ec8b410d73 and d6106de798ef8601596a9e09d81f7dfe59261bce.
The primary change across the files involves replacing direct system environment variable access (System.getenv()
) with a new utility method (IDFUtil.getSystemEnv()
). This refactor centralizes environment variable management, enhancing maintainability and consistency throughout the codebase. Additionally, some files now facilitate IDF tools path configuration through the UI, improving the user experience.
File | Change Summary |
---|---|
ESPToolChainManager.java , ToolsUtility.java , IDFMonitor.java , ToolsJob.java |
Replaced System.getenv() with IDFUtil.getSystemEnv() to retrieve environment variables. |
IDFUtil.java |
Added new methods: resolveEnvVariable(String path) and getSystemEnv() . |
IDFEnvironmentVariables.java , AppLvlTracingDialog.java , AbstractToolsHandler.java |
Updated environment variable initialization to use IDFUtil.getSystemEnv() . |
EspToolCommands.java |
Refactored process creation to use ProcessBuilder with environment settings from IDFUtil . |
IDFBuildConfiguration.java |
Added handling for IDF tools path environment variable in the build process. |
SbomCommandDialog.java , ProductInformationHandler.java , ExportIDFTools.java , InstallToolsHandler.java |
Replaced System.getenv() with IDFUtil.getSystemEnv() in various command execution methods. |
EspresssifPreferencesPage.java |
Added UI elements and logic for IDF tools path configuration. |
Messages.java |
Added new string constants for ESP-IDF tools installation directory selection messages. |
IDFSizeDataManager.java |
Updated method call to use IDFUtil.getSystemEnv() . |
sequenceDiagram
participant User
participant UI
participant ESPToolChainManager
participant IDFUtil
User->>UI: Trigger action requiring env variables
UI->>ESPToolChainManager: Request to run command
ESPToolChainManager->>IDFUtil: Call getSystemEnv()
IDFUtil-->>ESPToolChainManager: Return environment variables
ESPToolChainManager->>UI: Execute command with env variables
UI-->>User: Display results
sequenceDiagram
participant User
participant UI
participant EspresssifPreferencesPage
participant IDFUtil
User->>UI: Open preferences
UI->>EspresssifPreferencesPage: Load preferences page
EspresssifPreferencesPage->>IDFUtil: Retrieve current IDF tools path
IDFUtil-->>EspresssifPreferencesPage: Return IDF tools path
User->>EspresssifPreferencesPage: Set new IDF tools path
EspresssifPreferencesPage->>IDFUtil: Save new IDF tools path
User->>UI: Confirm changes
UI-->>User: Display confirmation
In the code, a change so grand,
Environment managed by a single hand.
WithIDFUtil
, the path's now clear,
For tools and builds, there's naught to fear.
Preferences set with ease and grace,
A smoother experience in every case.
🎉🐇
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?
@alirana01 hi ! Tested under: OS - Windows 10 ESP-IDF: v5.2
My previous tools were installed in the /TESTTOOL/Espressif folder. I have set this folder as a user variable.
Using this PR I have configured new IDF with next(default) tools folder:
Once tools are installed, I do see it still execute python from /TESTTOOL/Espressif
folder. So, tools are installed to user env PATH.
Hi @AndriiFilippov Please verify this
@AndriiFilippov please try once the builds are fine, the issue was that the process runner was not resolving the home directory vars automatically. Also the config now should show the resolved value for the user home directory
@alirana01 hi !
now I could see .espressif
folder created and tools installed there.
But once I build project I could see it is still using user env path tools.
see logs. buildLOG.txt
@alirana01 hi !
it still takes some python dependencies from user env path during build process:
Building in: C:\Users\AndriiFilippov\workspaceTEST1ALI111\jhjy\build
Configuring in: C:\Users\AndriiFilippov\workspaceTEST1ALI111\jhjy\build
cmake -G Ninja -DCMAKE_TOOLCHAIN_FILE=C:\Users\AndriiFilippov\e\esp-id f\esp-idf-v5.2.2\tools\cmake\toolchain-esp32.cmake -DCMAKE_EXPORT_COMPILE_COMMANDS=ON -DCCACHE_ENABLE=1 -DIDF_TARGET=esp32 C:\Users\AndriiFilippov\workspaceTEST1ALI111\jhjy
-- Found Git: C:/Program Files/Git/cmd/git.exe (found version "2.28.0.windows.1")
-- ccache will be used for faster recompilation
-- The C compiler identification is GNU 13.2.0
-- The CXX compiler identification is GNU 13.2.0
-- The ASM compiler identification is GNU
-- Found assembler: C:/Users/AndriiFilippov/.espressif/tools/xtensa-esp-elf/esp-13.2.0_20230928/xtensa-esp-elf/bin/xtensa-esp32-elf-gcc.exe
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working C compiler: C:/Users/AndriiFilippov/.espressif/tools/xtensa-esp-elf/esp-13.2.0_20230928/xtensa-esp-elf/bin/xtensa-esp32-elf-gcc.exe - skipped
-- Detecting C compile features
-- Detecting C compile features - done
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: C:/Users/AndriiFilippov/.espressif/tools/xtensa-esp-elf/esp-13.2.0_20230928/xtensa-esp-elf/bin/xtensa-esp32-elf-g++.exe - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- Building ESP-IDF components for target esp32
-- Checking Python dependencies...
Constraint file: C:\Users\AndriiFilippov\tool1\espidf.constraints.v5.2.txt
Requirement files:
- C:\Users\AndriiFilippov\e\esp-id f\esp-idf-v5.2.2\tools\requirements\requirements.core.txt
Python being checked: C:\Users\AndriiFilippov\.espressif\python_env\idf5.2_py3.12_env\Scripts\python.exe
Python requirements are satisfied.
-- Project sdkconfig file C:/Users/AndriiFilippov/workspaceTEST1ALI111/jhjy/sdkconfig
-- Compiler supported targets: xtensa-esp-elf
-- Looking for sys/types.h
-- Looking for sys/types.h - found
-- Looking for stdint.h
-- Looking for stdint.h - found
-- Looking for stddef.h
-- Looking for stddef.h - found
-- Check size of time_t
@alirana01 hi !
it still takes some python dependencies from user env path during build process:
@AndriiFilippov can you please try again with latest build
@alirana01 hi !
Using .espressif
tool folder and v5.1 ESP-IDF.
Still referring to user tool path -> during "ESP-IDF: Project Full Clean" execution:
ERROR: C:\Users\AndriiFilippov\tool1\espidf.constraints.v5.1.txt doesn't exist. Perhaps you've forgotten to run the install scripts. Please check the installation guide for more information.
ESP-IDF v5.1.4-474-gbae9a3a29f
@alirana01 hi !
Windows 10
affected (still refer to usr tool path) :
Hi @alirana01 Could you address the affected functionality as well?
@AndriiFilippov can you verify these things again please make sure to test on other platforms as well by setting the IDF_TOOLS_PATH in system env to some other value
@alirana01 hi !
Tested under: OS - Windows 10/11, Linux, MacOS ESP-IDF: v5.1 / v5.2
"Espressif" -> Product Information ✅ ESP-IDF: Python Clean ✅ ESP-IDF: Project Clean ✅ ESP-IDF: Project Full Clean ✅ ESP-IDF: Application Size Analyzer ✅ ESP-IDF: Install New Component ✅ Can open sdkconfig files ✅
LGTM 👍
Description
A new setting in Espressif preference page is added that can allow the user to configure the idf tools path for installation.
Fixes # (IEP-1043)
Type of change
Please delete options that are not relevant.
How has this been tested?
Configure the property in the Preferences page to something else and install tools they should be installed and builds and basic operations should be working fine.
Test Configuration:
Checklist
Summary by CodeRabbit
Refactor
System.getenv()
withIDFUtil.getSystemEnv()
across various components for consistent environment variable handling.User Interface
New Features
Bug Fixes