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

Document the heap allocation of Displayxxxx:default() #127

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

mangelajo
Copy link
Contributor

@mangelajo mangelajo commented Nov 22, 2022

Some platforms (like ESP32) have a limited stack size, but a bigger heap which can range 2-4MBytes for spiram. In such platforms it's impossible to allocate most of the displays in the stack, but it's possible to get them heap allocated (as long as we are compiling with std).

This commit documents heap allocation of the display buffer.

@mangelajo
Copy link
Contributor Author

ha, fmt is very unhappy, fixing that.

@mangelajo
Copy link
Contributor Author

I am hearing suggestions on the esp-rs matrix:

  • allow passing down a buffer instead to the display, i.e. :
#[link_section = ".ext_ram.bss.frame_buffer"]
static FRAME_BUFFER: StaticCell<[u8; N]> = StaticCell::new();

  • using alloc instead of std::Box
extern crate alloc;
use alloc::boxed::Box;

@mangelajo
Copy link
Contributor Author

ha... last minute changes... it doesn't work now:

error[E0599]: no function or associated item named `new` found for struct `epd_waveshare::epd5in65f::Display5in65f` in the current scope
   --> src/peripherals/display.rs:155:66
    |
155 |             let mut display: Box<Display5in65f> = Display5in65f::new();
    |                                                                  ^^^ function or associated item not found in `epd_waveshare::epd5in65f::Display5in65f`
    |
    = help: items from traits can only be used if the trait is in scope
help: the following trait is implemented but not in scope; perhaps add a `use` for it:
    |
1   | use epd_waveshare::traits::HeapAllocated;

@mangelajo mangelajo force-pushed the add-heap-allocation-support branch 2 times, most recently from 44f812c to 6918e2c Compare November 23, 2022 08:53
@mangelajo
Copy link
Contributor Author

A updated the implementation to only use alloc::boxed when enabled instead of std::boxed

@peckpeck
Copy link
Contributor

peckpeck commented Nov 23, 2022

If you look at #123 you can see that the Default display is #[inline(always)] which means that you can skip the stack allocation of a display by just doing Box::new(DisplayXX::default())

Without needing std in the library :-)

@mangelajo
Copy link
Contributor Author

If you look at #123 you can see that the Default display is #[inline(always)] which means that you can skip the stack allocation of a display by just doing Box::new(DisplayXX::default())

Without needing std in the library :-)

Oh awesome, I like your approach much better then. I will give a try to your PR and report back :)

@caemor
Copy link
Owner

caemor commented Nov 28, 2022

I finally hat time to merge Peckpecks PR, so if his idea already resolves your issue, we could change this pr/make a different one where we add it to the readme as an faq point or similar and maybe to the main doc comment as well.

@caemor caemor assigned mangelajo and unassigned caemor Nov 28, 2022
@mangelajo
Copy link
Contributor Author

I will do that @caemor !

@caemor caemor mentioned this pull request Dec 7, 2022
5 tasks
@mangelajo mangelajo closed this Dec 7, 2022
@mangelajo mangelajo reopened this Dec 7, 2022
@mangelajo
Copy link
Contributor Author

Oh sorry, right it was on me to document this.

@mangelajo mangelajo changed the title Add the heap allocation feature Document the heap allocation of Displayxxxx:default() Dec 8, 2022
@mangelajo
Copy link
Contributor Author

Ok, I converted it to documentation.

@mangelajo mangelajo force-pushed the add-heap-allocation-support branch 2 times, most recently from 51d647a to e4f44d7 Compare December 8, 2022 15:40
src/traits.rs Outdated Show resolved Hide resolved
src/traits.rs Show resolved Hide resolved
mangelajo and others added 3 commits June 13, 2023 00:05
@caemor caemor force-pushed the add-heap-allocation-support branch from df2058e to 42414f4 Compare June 12, 2023 22:05
///
///```rust, no_run
///# use epd_waveshare::epd4in2::Display4in2;
///# use embedded_graphics_core::prelude::*;
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
///# use embedded_graphics_core::prelude::*;
///# use embedded_graphics::prelude::*;

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.

None yet

3 participants