Skip to content

Conversation

@Dzarda7
Copy link
Collaborator

@Dzarda7 Dzarda7 commented Dec 1, 2025

This commit adds support for flashing for all targets. Support is only basic without 32-bit addressing using ROM functions including legacy ones.

Description

Related

Testing

Tested on a few targets - ESP32, ESP32-S3, ESP32-C6, not all for now, as no big issue is expected with this.


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 Dec 1, 2025

Messages
📖 🎉 Good Job! All checks are passing!

👋 Hello Dzarda7, we appreciate your contribution to this project!


Click to see more instructions ...


This automated output is generated by the PR linter DangerJS, which checks if your Pull Request meets the project's requirements and helps you fix potential issues.

DangerJS is triggered with each push event to a Pull Request and modify the contents of this comment.

Please consider the following:
- Danger mainly focuses on the PR structure and formatting and can't understand the meaning behind your code or changes.
- Danger is not a substitute for human code reviews; it's still important to request a code review from your colleagues.
- To manually retry these Danger checks, please navigate to the Actions tab and re-run last Danger workflow.

Review and merge process you can expect ...


We do welcome contributions in the form of bug reports, feature requests and pull requests.

1. An internal issue has been created for the PR, we assign it to the relevant engineer.
2. They review the PR and either approve it or ask you for changes or clarifications.
3. Once the GitHub PR is approved we do the final review, collect approvals from core owners and make sure all the automated tests are passing.
- At this point we may do some adjustments to the proposed change, or extend it by adding tests or documentation.
4. If the change is approved and passes the tests it is merged into the default branch.

Generated by 🚫 dangerJS against 4101650

@Dzarda7 Dzarda7 requested review from antmak and erhankur December 1, 2025 12:38
@Dzarda7
Copy link
Collaborator Author

Dzarda7 commented Dec 1, 2025

@antmak @erhankur what do you guys say to something like this? It is just as simple as it can be for now, but necessary for us to move on. I read about future plans in #9, but not sure about final decision so I made this for us to have something working and later on we can move to esp_flash implementation from ESP-IDF for older targets.

BTW if you are okay with this, I am not sure to which functions add _rominit, I believe they should work for you too, but feel free to test it and let me know.

@Dzarda7 Dzarda7 force-pushed the feat/add_basic_flashing_support branch from 7176de8 to 07d539d Compare December 1, 2025 12:43
@erhankur
Copy link
Collaborator

erhankur commented Dec 1, 2025

@antmak @erhankur what do you guys say to something like this? It is just as simple as it can be for now, but necessary for us to move on. I read about future plans in #9, but not sure about final decision so I made this for us to have something working and later on we can move to esp_flash implementation from ESP-IDF for older targets.

BTW if you are okay with this, I am not sure to which functions add _rominit, I believe they should work for you too, but feel free to test it and let me know.

The plan is still the same, but I haven't had time to implement the flasher functionality on top of the esp_flash APIs. So please go ahead.

@Dzarda7
Copy link
Collaborator Author

Dzarda7 commented Dec 2, 2025

Thanks, I can definitely help with esp_flash implementation, should be especially useful for 32-bit flashing which we also need. This is just to have something working for us now.

@Dzarda7 Dzarda7 requested a review from dobairoland December 2, 2025 09:12
Copy link
Collaborator

@dobairoland dobairoland left a comment

Choose a reason for hiding this comment

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

LGTM. I have just some minor request regarding clarification.

This commit adds support for flashing for all targets. Support is only
basic without 32-bit addressing using ROM functions including legacy
ones.
@Dzarda7 Dzarda7 force-pushed the feat/add_basic_flashing_support branch from 07d539d to 4101650 Compare December 8, 2025 15:06
@Dzarda7
Copy link
Collaborator Author

Dzarda7 commented Dec 8, 2025

@erhankur @dobairoland I believe I addressed all the issues, thank you very much for the check. If you agree, this is ready for merge.

Copy link
Collaborator

@erhankur erhankur left a comment

Choose a reason for hiding this comment

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

Thanks. LGTM now.

@Dzarda7 Dzarda7 merged commit c9c8e1e into master Dec 9, 2025
28 checks passed
@Dzarda7 Dzarda7 deleted the feat/add_basic_flashing_support branch December 9, 2025 08:34
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.

4 participants