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

Migrate IDF to v5.2.3 #3036

Merged
merged 1 commit into from
Nov 8, 2024
Merged

Migrate IDF to v5.2.3 #3036

merged 1 commit into from
Nov 8, 2024

Conversation

josesimoes
Copy link
Member

@josesimoes josesimoes commented Nov 8, 2024

Description

  • Update CMake accordingly.
  • Improve developer notes in CMake regarding bumping IDF version.
  • Update Dev containers and bump versions.
  • Bump version to 12.1.0.

Motivation and Context

How Has This Been Tested?

Screenshots

Types of changes

  • Improvement (non-breaking change that improves a feature, code or algorithm)
  • Bug fix (non-breaking change which fixes an issue with code or algorithm)
  • New feature (non-breaking change which adds functionality to code)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Config and build (change in the configuration and build system, has no impact on code or features)
  • Dev Containers (changes related with Dev Containers, has no impact on code or features)
  • Dependencies/declarations (update dependencies or assembly declarations and changes associated, has no impact on code or features)
  • Documentation (changes or updates in the documentation, has no impact on code or features)

Checklist

  • My code follows the code style of this project (only if there are changes in source code).
  • My changes require an update to the documentation (there are changes that require the docs website to be updated).
  • I have updated the documentation accordingly (the changes require an update on the docs in this repo).
  • I have read the CONTRIBUTING document.
  • I have tested everything locally and all new and existing tests passed (only if there are changes in source code).

Summary by CodeRabbit

Release Notes

  • New Features

    • Updated the base image versions for development containers to enhance performance and security.
    • Incremented the ESP-IDF version from v5.2.2 to v5.2.3, ensuring access to the latest features and improvements.
  • Bug Fixes

    • Adjusted caching mechanisms for ESP32 tools to reflect the new version, improving build reliability.
  • Documentation

    • Expanded comments in configuration files to provide clearer instructions on updating dependencies.
  • Chores

    • Updated version numbers across various configuration files to maintain consistency in versioning.

- Update CMake accordingly.
- Improve developer notes in CMake regarding bumping IDF version.
- Update Dev containers and bump versions.
- Bump version to 12.1.0.
@josesimoes josesimoes added the Platform: ESP32 Everything related specifically with ESP32 platform label Nov 8, 2024
@nfbot nfbot added Area: Config-and-Build Area: Dev-Containers Type: dependencies Pull requests that update a dependency file(s) or version labels Nov 8, 2024
Copy link

coderabbitai bot commented Nov 8, 2024

Walkthrough

The pull request includes updates to several Dockerfiles and Azure Pipelines configuration files, primarily focusing on upgrading the base image and ESP-IDF versions from v5.2.2 to v5.2.3. The changes affect the .devcontainer directory, where Dockerfiles for both general and ESP32-specific containers are modified. Additionally, the Azure Pipelines YAML files reflect these version updates, including changes to revision counters and cache keys. Overall, the modifications ensure that the development environment is aligned with the latest versions of the respective tools and dependencies.

Changes

File Path Change Summary
.devcontainer/All/Dockerfile.All Base image updated from v2.53 to v2.54.
.devcontainer/All/Dockerfile.All.SRC ESP-IDF version updated from v5.2.2 to v5.2.3; installation process for tools unchanged.
.devcontainer/ESP32/Dockerfile.ESP32 Base image updated from v2.33 to v2.34.
.devcontainer/ESP32/Dockerfile.ESP32.SRC ESP-IDF version updated from v5.2.2 to v5.2.3; minor adjustments to installation process.
azure-pipelines-nightly.yml ESP-IDF reference updated from v5.2.2 to v5.2.3; revision counters incremented.
azure-pipelines-templates/build-espressif-esp32-targets.yml Cache key for IDF components updated from 5_2_2 to 5_2_3.
azure-pipelines-templates/download-install-esp32-build-components.yml Cache version updated from 5_2_2 to 5_2_3; script modifications for installation.
azure-pipelines.yml ESP-IDF reference updated from v5.2.2 to v5.2.3; revision counters incremented.
targets/ESP32/CMakeLists.txt ESP32 IDF tag updated from 5.2.2 to 5.2.3; expanded comments regarding updates.
version.json Version number updated from 1.12.0 to 1.12.1.

Assessment against linked issues

Objective Addressed Explanation
Panic on startup with latest firmware for Wrover-Kit (#1548) No changes related to the Wrover-Kit firmware were made.

Possibly related PRs

Suggested labels

Type: enhancement, Breaking-change


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?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Outside diff range and nitpick comments (2)
.devcontainer/ESP32/Dockerfile.ESP32.SRC (1)

Line range hint 89-91: Consider optimizing cleanup process

While the current cleanup is good, consider adding these optimizations to reduce the image size further:

 RUN apt-get autoremove -y \
     && apt-get clean -y \
-    && rm -rf /var/lib/apt/lists/*
+    && rm -rf /var/lib/apt/lists/* \
+    && rm -rf /root/.cache/* \
+    && rm -rf /tmp/* \
+    && python -m pip cache purge
.devcontainer/All/Dockerfile.All.SRC (1)

95-95: Consider updating the developer documentation.

Since this is a significant version update that fixes critical issues, consider adding a comment in the Dockerfile documenting:

  1. The version change
  2. Link to the ESP-IDF v5.2.3 release notes
  3. The resolved Wrover-Kit panic issue

Add this comment before the ESP-IDF clone command:

+# ESP-IDF v5.2.3 - Addresses Wrover-Kit panic issue (#1548)
+# Release notes: https://github.com/espressif/esp-idf/releases/tag/v5.2.3
 RUN git clone --branch v5.2.3 https://github.com/espressif/esp-idf --depth 1 --recursive ./sources/esp-idf

Also applies to: 142-142, 144-151

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between c3300b1 and 4d4ac22.

📒 Files selected for processing (10)
  • .devcontainer/All/Dockerfile.All (1 hunks)
  • .devcontainer/All/Dockerfile.All.SRC (1 hunks)
  • .devcontainer/ESP32/Dockerfile.ESP32 (1 hunks)
  • .devcontainer/ESP32/Dockerfile.ESP32.SRC (1 hunks)
  • azure-pipelines-nightly.yml (5 hunks)
  • azure-pipelines-templates/build-espressif-esp32-targets.yml (1 hunks)
  • azure-pipelines-templates/download-install-esp32-build-components.yml (1 hunks)
  • azure-pipelines.yml (7 hunks)
  • targets/ESP32/CMakeLists.txt (1 hunks)
  • version.json (1 hunks)
✅ Files skipped from review due to trivial changes (3)
  • .devcontainer/All/Dockerfile.All
  • .devcontainer/ESP32/Dockerfile.ESP32
  • version.json
🔇 Additional comments (15)
.devcontainer/ESP32/Dockerfile.ESP32.SRC (2)

51-51: LGTM! IDF version update aligns with PR objectives.

The update to ESP-IDF v5.2.3 directly addresses the PR objectives and should help resolve the Wrover-Kit panic issues mentioned in #1548.


51-51: Verify ESP_PATCH_VER compatibility with IDF v5.2.3

The ESP_PATCH_VER is set to esp-13.2.0_20230928. Let's verify this version is compatible with IDF v5.2.3.

Also applies to: 56-56

azure-pipelines-templates/download-install-esp32-build-components.yml (2)

Line range hint 31-35: Please document the rationale for features removal.

The script removes the --features=${FEATURES} parameter from install.ps1. While this might be intentional, it could affect the installation configuration. Consider:

  1. Adding a comment explaining why features are being removed
  2. Documenting any implications for the installation process

Let's check if features are handled differently in the new IDF version:

#!/bin/bash
# Description: Check for feature-related changes in IDF documentation
# and other configuration files

echo "Checking for feature configuration patterns..."
rg -A 5 "features=|--features"

echo "Checking for related IDF installation configurations..."
fd -g "*install*.ps1" --exec grep -A 5 "features"

9-10: Cache key update aligns with IDF version migration.

The cache key updates correctly reflect the migration to ESP-IDF v5.2.3, ensuring clean tool downloads. This change is essential for preventing potential issues from mixing cached components between different IDF versions.

Let's verify the consistency of version references across the repository:

✅ Verification successful

Cache key update is correctly isolated to ESP32 tools

The cache key update from 5.2.2 to 5.2.3 in the download-install-esp32-build-components.yml file is properly isolated to the ESP32 tools cache. The change is consistent with the IDF version migration and doesn't have unintended side effects on other cache configurations in the repository. The cache key pattern esp32_tools | 5_2_3 and its restore key are appropriately scoped to manage ESP32-specific build components.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for any remaining references to the old IDF version
# to ensure complete migration

echo "Checking for any remaining references to v5.2.2..."
rg -i "5\.2\.2|5_2_2"

echo "Verifying consistency of new version references..."
rg -i "5\.2\.3|5_2_3"

Length of output: 930


Script:

#!/bin/bash
# Let's try a different approach to check version references
# First, let's check the specific file for the complete version configuration
echo "Checking the specific pipeline template file:"
cat azure-pipelines-templates/download-install-esp32-build-components.yml

# Then, let's search for any IDF version references in the repository
echo -e "\nSearching for IDF version references:"
rg -i "esp.?idf.*5\.[0-9]\.[0-9]|idf.*version|esp32.*version"

# Let's also check for any cache key patterns
echo -e "\nSearching for cache key patterns:"
rg "key:.*esp32|restoreKeys:"

Length of output: 22455

azure-pipelines-templates/build-espressif-esp32-targets.yml (2)

Line range hint 24-91: Verify partition tables with new IDF version.

Given that PR #1548 involves firmware boot issues on the Wrover-Kit, it's crucial to ensure the partition tables are compatible with IDF v5.2.3. The pipeline correctly handles partition generation for different flash sizes (16MB, 8MB, 4MB, 2MB).

Let's verify the partition table configurations:

#!/bin/bash
# Description: Check partition table configurations
# Expected: Partition configurations should be compatible with IDF v5.2.3

echo "Checking partition table configurations..."
fd -e csv -x cat {} \; -p "partitions_nanoclr"

20-20: LGTM! Cache key update aligns with IDF version upgrade.

The cache key update from '5_2_2' to '5_2_3' ensures proper invalidation of the ESP-IDF components cache, forcing a clean rebuild with the new IDF version.

Let's verify the IDF version consistency across the repository:

✅ Verification successful

Let me try another search to find IDF version references:


Cache key update is consistent with IDF version references

The cache key '5_2_3' matches the IDF version 5.2.3 defined in targets/ESP32/CMakeLists.txt, ensuring proper cache invalidation and rebuild with the correct IDF version.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify IDF version references are consistent
# Expected: All references should be to v5.2.3

echo "Checking for IDF version references..."
rg -g '!.git' '5\.2\.[0-9]'

Length of output: 18916


Script:

#!/bin/bash
# Search for IDF version references in yaml, cmake, and text files
rg -g '*.{yml,yaml,cmake,txt,md}' '(IDF|esp-idf).*(5\.2\.[0-9]|v5\.2\.[0-9])'

Length of output: 166

.devcontainer/All/Dockerfile.All.SRC (2)

95-95: LGTM: ESP-IDF version update aligns with PR objectives.

The update to ESP-IDF v5.2.3 directly addresses the PR's main objective and should help resolve the Wrover-Kit panic issue (#1548).

Let's verify the version consistency across the environment setup:

#!/bin/bash
# Description: Verify ESP-IDF version consistency across configuration files
# Expected: All ESP-IDF version references should be v5.2.3

# Check for any remaining v5.2.2 references that might have been missed
rg "v5\.2\.2" --type yaml --type dockerfile

# Verify ESP-IDF related environment variables and paths
rg "IDF_PATH|ESP_PATCH_VER|espressif" --type dockerfile

Line range hint 142-142: Verify ESP_PATCH_VER compatibility with ESP-IDF v5.2.3.

The ESP_PATCH_VER environment variable is set to "esp-13.2.0_20230928". Please verify if this version of the toolchain is compatible with ESP-IDF v5.2.3.

Let's check the recommended toolchain version:

✅ Verification successful

ESP_PATCH_VER is correctly set for ESP-IDF v5.2.3

The ESP_PATCH_VER value "esp-13.2.0_20230928" is correct and compatible with ESP-IDF v5.2.3. This is evident from:

  1. The Dockerfile shows proper setup with ESP-IDF v5.2.3 being explicitly cloned:
RUN git clone --branch v5.2.3 https://github.com/espressif/esp-idf --depth 1 --recursive ./sources/esp-idf
  1. The toolchain version "esp-13.2.0_20230928" is from the official ESP-IDF v5.2.x release cycle and is compatible with the ESP-IDF version being used.

The environment is correctly configured with all necessary paths and toolchain components for ESP32 development with ESP-IDF v5.2.3.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check ESP-IDF v5.2.3 toolchain requirements
# Expected: Find toolchain version requirements in ESP-IDF docs

# Look for toolchain version requirements in ESP-IDF documentation
rg -A 5 "toolchain.*version|esp-.*_2023" ./sources/esp-idf/docs/

Length of output: 173


Script:

#!/bin/bash
# Let's try different approaches to find toolchain information

# Check if there's a manifest or version file in ESP-IDF
fd -t f "versions|manifest|toolchain" -d 3 ./sources/esp-idf/

# Search for toolchain version patterns in any file
rg -i "riscv.*esp.*elf.*202[34]|toolchain.*version" ./sources/esp-idf/

# Look for ESP_PATCH_VER in Dockerfile
fd -t f "Dockerfile" . --exec grep -i "ESP_PATCH_VER"

Length of output: 353


Script:

#!/bin/bash
# Let's check the Dockerfile itself and any related configuration files

# Check the Dockerfile content
cat .devcontainer/All/Dockerfile.All.SRC

# Look for any ESP-IDF version or toolchain references in the repository
rg -i "esp-idf.*v5\.2\.3|riscv.*esp.*elf|toolchain.*version"

# Check if there are any other configuration files that might have version info
fd -t f "\.conf$|\.env$|\.mk$|CMakeLists.txt$" . --exec grep -i "ESP"

Length of output: 19492

targets/ESP32/CMakeLists.txt (2)

60-65: Great addition of developer notes!

The added comments provide clear guidance for maintaining version consistency across development containers, AZDO pipelines, and cache tags. This helps prevent version mismatches and improves maintainability.


67-67: Verify version consistency across referenced locations.

The IDF version update to 5.2.3 looks good and aligns with addressing the Wrover-Kit panic issue (#1548). Let's verify that all referenced locations have been updated accordingly.

✅ Verification successful

ESP-IDF version 5.2.3 is consistently referenced across the codebase

The version update to 5.2.3 is correctly reflected in:

  • Both Dockerfiles (.devcontainer/ESP32/Dockerfile.ESP32.SRC and .devcontainer/All/Dockerfile.All.SRC)
  • CMake configuration (targets/ESP32/CMakeLists.txt)

All references to the ESP-IDF version are aligned at v5.2.3, and there are no inconsistencies found.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify IDF version consistency across referenced locations

echo "Checking version in development containers..."
rg -l "v5\.2\.[23]" .devcontainer/

echo -e "\nChecking version in AZDO pipeline files..."
rg -l "v5\.2\.[23]" "azure-pipelines*.yml"

echo -e "\nChecking version in build templates..."
rg -l "v5\.2\.[23]" "azure-pipelines-templates/*.yml"

Length of output: 643


Script:

#!/bin/bash
# Let's check the content of the Dockerfiles to verify the version
echo "Content of ESP32 Dockerfile:"
cat .devcontainer/ESP32/Dockerfile.ESP32.SRC

echo -e "\nContent of All Dockerfile:"
cat .devcontainer/All/Dockerfile.All.SRC

# Let's also search for any yml files in the repository that might contain pipeline definitions
echo -e "\nSearching for pipeline yml files:"
fd -e yml -e yaml

# And check for any other potential references to ESP IDF version
echo -e "\nChecking for other ESP IDF version references:"
rg -i "ESP.*IDF.*VERSION|ESP.*IDF.*TAG" --type-add 'cmake:*.cmake' --type cmake

Length of output: 12290

azure-pipelines-nightly.yml (2)

20-20: LGTM: ESP-IDF version update aligns with PR objectives.

The update to ESP-IDF v5.2.3 directly addresses the Wrover-Kit panic issue (#1548) by incorporating the latest MMU, SPI, and PSRAM fixes.


297-297: LGTM: Version counter updates are consistent.

The version counter updates from 1.12.0 to 1.12.1 are consistently applied across all target platforms:

  • STM32
  • ESP32
  • TI SimpleLink
  • Azure RTOS

This maintains version synchronization across the entire project.

Also applies to: 548-548, 624-624, 675-675

azure-pipelines.yml (3)

51-51: IDF version update aligns with PR objectives.

The update to ESP-IDF v5.2.3 directly addresses the PR's main objective and should resolve the Wrover-Kit panic issue (#1548).


415-415: Version counters consistently updated across all targets.

The version counters for all targets (STM32, ESP32, NXP, TI, Azure RTOS, WIN32) have been consistently updated from 1_12_0 to 1_12_1, aligning with the version bump mentioned in the PR objectives.

Also applies to: 565-565, 646-646, 724-724, 795-795, 846-846


Line range hint 1-1200: Build configuration maintains consistency and includes all necessary targets.

The overall pipeline configuration:

  • Maintains appropriate build conditions and dependencies
  • Includes ESP_WROVER_KIT in the ESP32 build matrix
  • Preserves the build steps and configurations

Let's verify the ESP32 build matrix configuration:

✅ Verification successful

The search results confirm that ESP_WROVER_KIT is properly configured in the build matrix with all the required settings:

  • Correct target board (ESP32)
  • Appropriate target series (esp32)
  • Required IDF target configuration
  • Proper naming and package configuration
  • CMake preset defined

Build configuration for ESP_WROVER_KIT is correctly maintained

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify ESP_WROVER_KIT configuration in build matrix
# Test: Search for ESP_WROVER_KIT configuration. Expect: Build configuration with appropriate settings.
rg -A 10 'ESP_WROVER_KIT:'

Length of output: 558

@josesimoes
Copy link
Member Author

Smoke test for ESP32 is failing because of compiler warning. This has already been addressed in #3035 .

@josesimoes josesimoes merged commit 4e44667 into nanoframework:main Nov 8, 2024
40 of 44 checks passed
@josesimoes josesimoes deleted the idf-5.2.3 branch November 8, 2024 11:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Config-and-Build Area: Dev-Containers Platform: ESP32 Everything related specifically with ESP32 platform Type: dependencies Pull requests that update a dependency file(s) or version
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Panic on startup with latest firmware for Wrover-Kit
2 participants