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

[VSC-1472/1207] add new idf size format support #1330

Merged
merged 2 commits into from
Oct 29, 2024

Conversation

brianignacio5
Copy link
Collaborator

@brianignacio5 brianignacio5 commented Oct 23, 2024

Description

Support new esp-idf-size JSON format which provide more map file information in ESP-IDF: Size analysis of the binaries command UI.

This feature is supported for ESP-IDF v5.3 and higher, when older versions are used the extension will use previous IDF Size UI
Screenshot 2024-10-23 at 17 10 38

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 menu View, Command Palette and search for ESP-IDF: Size analysis of the binaries command with ESP-IDF v5.3.0 or higher
  2. Execute action. The rendered UI should show more fields and information.
  3. Observe results.
  4. Repeat step 1 with ESP-IDF version older than v5.3.0. Results should show previous behavior.

https://docs.espressif.com/projects/esp-idf/en/latest/esp32/api-guides/tools/idf-size.html?highlight=size

How has this been tested?

Manual testing of extension command using both v5.4.0 and v.5.2.2

Test Configuration:

  • ESP-IDF Version: v5.4.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 added this to the 1.9.0 milestone Oct 23, 2024
@brianignacio5 brianignacio5 self-assigned this Oct 23, 2024
Copy link

github-actions bot commented Oct 23, 2024

Download the artifacts for this pull request:
You can test these changes by installing this VSIX by click menu View -> Command Palette..., type Install from VSIX and then select downloaded esp-idf-extension.vsix file to install the extension.

@Fabricio-ESP Fabricio-ESP changed the title add new idf size format support [VSC-1472/1207] add new idf size format support Oct 23, 2024
@Fabricio-ESP
Copy link
Collaborator

Having trouble to run the command using IDF 5.3.1 on a Windows + WSL environment.
IDF.py size command is successful in the terminal. After the project is built I can see the table printed with some size allocation and an error (straight from IDF) which is probably unrelated. Still the extension shows a pop up message "Error encountered while calling idf_size.py.

image

Running on Windows only I see different contents between IDF 5.2.3:
image

image

Compared to 5.3.1:
image

image

But it seems the submenus are not being displayed for all files, not sure why.

@Fabricio-ESP
Copy link
Collaborator

The issue seems to be associated to the selected target. For ESP32 > Wrover Kit I see the table being rendered incorrectly on windows, but complete on WSL, while when selecting ESP32C6 I see the error popup without any size table.

@radurentea
Copy link
Collaborator

  1. I've run into the same issue, but it seems this happens only with an example I've already had in my PC. When I have this error, the Size Analysis Webview doesn't open.
    image
    After reconfiguring the extension with esp-idf 5.3.1 and building a new blink example, I did not have this issue anymore.

  2. I've also noticed that for the new Size Analysis, there is only "libesp_app_format.a" which can be extended. Probably there is less data or the data has changed and we don't create the fields properly?

UI Improvement suggestion for future, not related to this PR

  1. The rows to have 2 different background colors, one darker and one lighter this would help with readability. I think the term is zebra striping.
    image

  2. The columns to be aligned, when there is a sub-menu like in "libesp_app_format.a" the columns become unaligned with the others
    image

@brianignacio5
Copy link
Collaborator Author

brianignacio5 commented Oct 28, 2024

Fixed the issues of files not being added into their respective archive on Windows.

  1. Added the striped to the file table under archive.
  2. Columns were not aligned because files were not added properly on Windows so the last icon was not rendered for many archives. Last commit should fix the issue.

PTAL again @Fabricio-ESP @radurentea

@Fabricio-ESP
Copy link
Collaborator

Data is rendering as expected now.

Th eone item that can be treated in other PR, related to customer experience improvement. The size chart and table are not updated if the user re-runs the command before closing the previous results. Either we close it and open a new window with the resilts, or update the results in place. It is not clear to the user that the results presented are outdated.

Copy link
Collaborator

@radurentea radurentea left a comment

Choose a reason for hiding this comment

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

LGTM

@brianignacio5 brianignacio5 merged commit fe129b9 into master Oct 29, 2024
6 checks passed
@brianignacio5 brianignacio5 deleted the feature/new-idf-size branch October 29, 2024 09:04
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.

3 participants