Skip to content

Conversation

umgefahren
Copy link
Contributor

I found it interesting how using an allocator with the rp-pico would work, so i build this little example, which is derived from the blinking led example.
I couldn't find any constant indicating a possible heap size in the hal, but i may have overlooked it.
I was mainly inspired by this repository: alloc-cortex-m

Feedback is appreciated :)

@9names
Copy link
Member

9names commented Mar 2, 2022

We build all examples as part of CI - this example would need to be somehow not built if the compiler is not nightly.

@jannic
Copy link
Member

jannic commented May 31, 2022

We build all examples as part of CI - this example would need to be somehow not built if the compiler is not nightly.

One way to do it could be this:

alloc-cortex-m = { git = "https://github.com/rust-embedded/alloc-cortex-m", rev = "a1be51edcee5e1301549a1373c0c64b94de0323c", optional = true }

[...]

[features]
default = ["boot2", "rt"]
boot2 = ["rp2040-boot2"]
rt = ["cortex-m-rt","rp2040-hal/rt"]
nightly = ["dep:alloc-cortex-m"]

[[example]]
name = "pico_alloc"
required-features = ["nightly"]

(For this to work, the alloc-cortex-m must be moved from [dev-dependencies] to [dependencies], as dev-dependencies can not be optional.)

With this, cargo build --examples skips pico_alloc, and cargo +nightly build --examples --features=nightly can be used to compile it.

@9names
Copy link
Member

9names commented Jun 1, 2022

That could work.
If you do update this, also switch to a version of alloc-cortex-m that is published on crates.io.

I couldn't find any constant indicating a possible heap size in the hal, but i may have overlooked it.

We also don't specify a stack size. It's "whatever is left" after static allocations.
The size for a heap is program-specific, we have no idea what is appropriate for your program.

The base address is defined in the cortex-m-rt linker script, so you could calculate the limit yourself - assuming you know how large you want the stack to be.

@jannic
Copy link
Member

jannic commented Oct 20, 2023

Hi @umgefahren!

Are you still interested in this pull request?

The good news is: It's now possible to implement this on stable rust. So we no longer need to think about nightly rust.
But in the mean time, the board crates were moved to a different repo, so it no longer makes sense to apply this pull request as-is to the rp-hal repo.

As a proof of concept, I successfully applied your changes to the rp-hal-boards repo: rp-rs/rp-hal-boards@main...jannic-dev-forks:rp-hal-boards:pr306 (There are still some rough edges, clippy complains and some formatting is wrong.)

But then, I wonder if this really should be a rp-pico example. The alloc feature is by no means board specific. So it might be better to have an example in rp2040-hal/examples. That would require some minor changes as no BSP would be available. The easiest way would probably be to start from https://github.com/rp-rs/rp-hal/blob/main/rp2040-hal/examples/blinky.rs.

Your pull request is more than a year old, so I wouldn't be surprised if you were no longer interested. In that case, I'd try to find some time to adapt the example myself. But I don't want to "steal" your idea, in case you still want to work on it.

@umgefahren
Copy link
Contributor Author

Quite honestly I didn't remember I did this PR until this very email. I would have to get back into it, so it would be great if you could make the time.

@jannic jannic force-pushed the feature/AllocExampleRpPico branch 3 times, most recently from 59006c5 to 81b78e5 Compare October 22, 2023 21:11
@jannic
Copy link
Member

jannic commented Oct 22, 2023

Ok, I moved the example to rp2040-hal/examples/alloc.rs and made the necessary changes to make it compile without the rp-pico BSP.

It looks like I was a little bit too quick with my statement about compatibility with stable rust: alloc_error_handler only became stable with rust 1.68, so it doesn't work yet with out MSRV 1.64.

Therefore I'll label the PR as a breaking change, to be merged shortly before releasing v0.10.0

@jannic jannic added the breaking change This pull request contains a SemVer breaking change label Oct 22, 2023
@jannic jannic added this to the 0.10.0 milestone Oct 22, 2023
@jannic jannic force-pushed the feature/AllocExampleRpPico branch from 81b78e5 to fccfdbe Compare February 10, 2024 20:00
@jannic
Copy link
Member

jannic commented Feb 10, 2024

I rebased the pull request to current main and updated it for embedded-hal 1.0.0.
The MSRV of this crate is now sufficient for alloc_error_handler, so this pull request is ready for review & merging.

Copy link
Member

@ithinuel ithinuel left a comment

Choose a reason for hiding this comment

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

Hey there, alloc-cortex-m only supports single core and is deprecated in favor of embedded-alloc.

@jannic
Copy link
Member

jannic commented Feb 10, 2024

Hey there, alloc-cortex-m only supports single core and is deprecated in favor of embedded-alloc.

Good point! Fortunately, porting was trivial, it's basically the same crate under a different name.

@jannic jannic requested a review from ithinuel February 10, 2024 21:38
@jannic jannic removed the breaking change This pull request contains a SemVer breaking change label Feb 10, 2024
@jannic jannic merged commit 099e34f into rp-rs:main Feb 10, 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