Skip to content

esp_memory_resource a PMR memory resource #24

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

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

Conversation

euripedesrocha
Copy link
Collaborator

Simple memory resource class enabling the usage of C++17 PMR facilities with IDF heap allocation capabilities.

This enables users to set the heap memory destination for C++ containers.

@euripedesrocha euripedesrocha requested a review from 0xjakob July 31, 2023 11:50
@euripedesrocha euripedesrocha self-assigned this Jul 31, 2023
@CLAassistant
Copy link

CLAassistant commented Jul 31, 2023

CLA assistant check
All committers have signed the CLA.

Copy link
Contributor

@0xjakob 0xjakob left a comment

Choose a reason for hiding this comment

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

Nice work! Code structure looks good to me. I left some minor comments. Should we add a README to the example?

esp_log_level_set("*", ESP_LOG_INFO);
esp_log_level_set(idf::memory::resource::tag, ESP_LOG_VERBOSE);
using enum idf::memory::capabilities;
auto memory_source = dma;
Copy link
Contributor

Choose a reason for hiding this comment

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

nitpick: either not using auto or the using declaration might make the type clearer when reading the code. But changing it is not a must.

return capabilities{std::to_underlying(lhs) | std::to_underlying(rhs)};
}

class resource : public std::pmr::memory_resource {
Copy link
Contributor

Choose a reason for hiding this comment

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

For my taste, a simple comment what this class does and guiding users to the documentation of PMR would be helpful.

@euripedesrocha euripedesrocha marked this pull request as ready for review August 3, 2023 14:16
@euripedesrocha euripedesrocha force-pushed the memory_resource branch 4 times, most recently from 66c4c4d to 18b5623 Compare October 26, 2023 13:51
Simple memory resource class enabling the usage of C++17 PMR facilities
with IDF heap allocation capabilities.
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants