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

Added support for v5.2 #284

Closed

Conversation

MartinBroers
Copy link

When implementing support for the newest v5.2 release from espressif, I found espressif/esp-idf#13113 this issue, where they mention we need to apply a patch, or the linker is going to run havoc.

Also, updated the pipeline so it looks more similar to the esp-idf-svc pipeline. This MR is required for esp-rs/esp-idf-svc#365, else that one can't build.

@MartinBroers MartinBroers marked this pull request as ready for review February 17, 2024 10:13
@MartinBroers MartinBroers force-pushed the Added-support-for-v5-2 branch 4 times, most recently from 4a30f52 to 3db40f3 Compare February 17, 2024 10:56
MartinBroers pushed a commit to MartinBroers/esp-idf-svc that referenced this pull request Feb 17, 2024
Remove this commit when esp-rs/esp-idf-sys#284
is merged.

Signed-off-by: Martin Broers <[email protected]>
MartinBroers pushed a commit to MartinBroers/esp-idf-svc that referenced this pull request Feb 17, 2024
Remove this commit when esp-rs/esp-idf-sys#284
is merged.

Signed-off-by: Martin Broers <[email protected]>
matrix:
target:
- riscv32imc-esp-espidf
- xtensa-esp32-espidf
- xtensa-esp32s2-espidf
- xtensa-esp32s3-espidf
idf-version:
- v4.4.6
- v5.0
Copy link
Collaborator

Choose a reason for hiding this comment

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

From a practical standpoint, we might remove v5.0 from the CI runs if we add 5.2, to not make it to slow. I think most people either are still on v4.4.6 or on v5.1.2 currently and that will shift more to 5.2.

Copy link
Author

Choose a reason for hiding this comment

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

Fair enough. Removed the pipeline build for v5.0.

@MartinBroers MartinBroers force-pushed the Added-support-for-v5-2 branch from 3db40f3 to a3f1544 Compare February 17, 2024 20:10
@esp-lrh
Copy link

esp-lrh commented Feb 19, 2024

Hi, @MartinBroers @Vollbrecht
The patch in espressif/esp-idf#13113 is a temporary fix, the formal merge request is created and in reviewing, maybe need one or two days to merge and sync to github, backports need additional days.

This issue only occurs when enable bluetooth and wifi in sdkconfig but only initialize wifi, it is recommended to workaround by editing sdkconfig or initializing bluetooth.

@ivmarkov
Copy link
Collaborator

@MartinBroers What I do not understand from the linked issue is, what is the actual problem?

  • Is it something related only to 5.2?
  • Isn't there a workaround without patching? They do seem to mention that there might be a non-patching workaround with sdkconfig.defaults?

@MartinBroers
Copy link
Author

@esp-lrh @ivmarkov
I have tested locally with patching sdkconfig.defaults and can confirm we do not need this patch for building the samples in the esp-idf-svc pipeline.

I will rework the PR on esp-idf-svc to work without requiring this patch.

@MartinBroers What I do not understand from the linked issue is, what is the actual problem?

* Is it something related only to 5.2?

Yes, building the examples in esp-idf-svc does NOT work in v5.1.2

* Isn't there a workaround without patching? They do seem to mention that there might be a non-patching workaround with `sdkconfig.defaults`?

Yes, see my reaction above. We need to patch .github/configs/sdkconfig.defaults to not use bluetooth. Not sure if the sdconfig.defaults was crafted in such a way to explicitly support BT during compilation?

@MartinBroers MartinBroers force-pushed the Added-support-for-v5-2 branch from a3f1544 to e57bc87 Compare February 19, 2024 09:22
@ivmarkov
Copy link
Collaborator

Closing as we found a way to do this without patching esp idf for now.

@ivmarkov ivmarkov closed this Feb 21, 2024
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