Skip to content

Conversation

@nebkat
Copy link
Contributor

@nebkat nebkat commented Jun 6, 2025

Description

Adds colors to addr2line output:

image

Related

Requires espressif/esp-idf-panic-decoder#2

Testing

Tested locally with a variety of inputs.

Checklist

Before submitting a Pull Request, please ensure the following:

  • 🚨 This PR does not introduce breaking changes.
  • All CI checks (GH Actions) pass.
  • Documentation is updated as needed.
  • Tests are updated or added as necessary.
  • Code is well-commented, especially in complex areas.
  • Git history is clean — commits are squashed to the minimum necessary.

@github-actions
Copy link

github-actions bot commented Jun 6, 2025

Warnings
⚠️ The PR description looks very brief, please check if more details can be added.

👋 Hi nebkat, thank you for your another contribution to espressif/esp-idf-monitor project!

If the change is approved and passes the tests and human review, it will appear in this public Github repository after merge.


🔁 You can re-run automatic PR checks by retrying the DangerJS action


⚠️ Deprecation Notice: This GitHub action is scheduled for removal in March 2024. We recommend migrating to the latest version available in the shared-github-danger project.

Generated by 🚫 dangerJS against d8932f3

@github-actions github-actions bot changed the title feat: Improved addr2line output formatting feat: Improved addr2line output formatting (IDFGH-15432) Jun 6, 2025
@peterdragun
Copy link
Collaborator

Overall, this looks like a nice feature. Thank you for contributing.

The pre-commit check is failing. Can you please fix this?

@nebkat nebkat force-pushed the master branch 3 times, most recently from aa752ff to 1197c86 Compare June 9, 2025 16:22
@peterdragun peterdragun requested a review from Copilot June 10, 2025 08:58
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR improves the addr2line output formatting by adding color-coded output for enhanced readability. Key changes include:

  • Updating the logger module to import additional color printing helpers.
  • Replacing a single-address decoder call with a multi-address decoding approach.
  • Iteratively printing formatted output for each decoded address entry, including inlined function names.

@peterdragun
Copy link
Collaborator

Hi @nebkat, I have one more request. When I ran the PR through our internal process, I realized that there is one test failing: https://github.com/espressif/esp-idf-monitor/blob/master/test/test_apps/monitor_addr_lookup/pytest_monitor_addr_lookup.py
The reason for failure is that you changed the "at" to "in" in the decoded address. Could you please either update the test or revert it to "at" to unblock us from merging your PR?

@nebkat
Copy link
Contributor Author

nebkat commented Jun 17, 2025

I wasn't able to get the test to run on Windows but isn't this the expected output?
image

Uses at for known locations, in ROM for unknown locations in the ROM ELF.

@peterdragun
Copy link
Collaborator

Oh, I am sorry for the confusion. I misread the log. The issue is actually the coloring between the "app_main" and "at". If you are unable to run this test, I can create a fix for you in a separate commit.

@espressif-bot espressif-bot merged commit d8932f3 into espressif:master Jun 24, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants