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

feat(mdns): support allocating mDNS task from SPIRAM (IDFGH-14509) #746

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

zwx1995esp
Copy link
Contributor

Description

This PR supports allocating mDNS task from PSRAM.

Related

Testing


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.

@zwx1995esp zwx1995esp force-pushed the support/allocate_memory_from_psram branch from 064ea37 to 6ddcdae Compare January 24, 2025 10:46
Copy link
Collaborator

@david-cermak david-cermak left a comment

Choose a reason for hiding this comment

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

LGTM in general.

@david-cermak david-cermak self-assigned this Jan 24, 2025
@david-cermak
Copy link
Collaborator

LGTM in general.

seems like xTaskCreatePinnedToCoreWithCaps() is not available on v5.0.x

@espressif-bot espressif-bot added the Status: Opened Issue is new label Jan 24, 2025
@github-actions github-actions bot changed the title feat(mdns): support allocating mDNS task from SPIRAM feat(mdns): support allocating mDNS task from SPIRAM (IDFGH-14509) Jan 24, 2025
@zwx1995esp zwx1995esp force-pushed the support/allocate_memory_from_psram branch 4 times, most recently from 0da7722 to 03b7edb Compare January 26, 2025 05:29
components/mdns/mdns.c Fixed Show fixed Hide fixed
@zwx1995esp zwx1995esp force-pushed the support/allocate_memory_from_psram branch from 03b7edb to e75fa55 Compare January 26, 2025 05:38
@zwx1995esp zwx1995esp force-pushed the support/allocate_memory_from_psram branch from e75fa55 to e33ba99 Compare January 26, 2025 06:01
@zwx1995esp
Copy link
Contributor Author

Hi, @david-cermak Might need your help: there are still two CI failed, one for target esp32, I can not reproduce this issue on my local env. And another one is for linux target, it seems that we do not support some FreeRTOS APIs for linux target. Do you have some suggestions for solving these?

@david-cermak
Copy link
Collaborator

Might need your help: there are still two CI failed, one for target esp32, I can not reproduce this issue on my local env. And another one is for linux target, it seems that we do not support some FreeRTOS APIs for linux target. Do you have some suggestions for solving these?

The tests actually flagged a pretry nasty bug while trying to delete the task from itself, including the stack size.
IDF people create a minimal task for that:

https://github.com/espressif/esp-idf/blob/c5865270b50529cd32353f588d8a917d89f3dba4/components/freertos/esp_additions/idf_additions.c#L136-L156

But, here we don't have to be that strict IMO, fixing in #756

Also, should address this

https://github.com/espressif/esp-idf/blob/c5865270b50529cd32353f588d8a917d89f3dba4/components/freertos/esp_additions/idf_additions.c#L113

but since the unit tests pass, it's probably not too serious of a leak

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants