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

Add ConstInitCell #14

Merged
merged 4 commits into from
Apr 26, 2024
Merged

Conversation

jamesmunns
Copy link
Contributor

This adds a "const initialized" variant of StaticCell.

I decided to add a second type, rather than extending StaticCell, because I couldn't see a way to do this without potentially making the value non-zero-initialized.

My other plan was to change the atomicbool into an AtomicU8 with three states:

  • Empty + Untaken
  • Full + Untaken
  • Full + Taken

But then one of the two flavors would have to start with a non-zero init value, which could "infect" the whole type into .data instead of .bss.

@jamesmunns jamesmunns changed the title Add const init cell Add ConstInitCell Apr 25, 2024
@ivmarkov
Copy link

ivmarkov commented Apr 25, 2024

I think I have exactly the use case that needs this.
The main, fat rs-matter structure is now const-initializable, yet it is not thread-safe (contains RefCells and Mutex<NoopRawMutex, ...> stuff), that might or might not become configurable in future. I.e. use CriticalSectionRawMutex or NoopRawMutex depending on build features - yet - the single threaded model would remain the most appealing one.

Yet, I would really like to put it in a static context with static MATTER: SomethingSomething<Matter<'static>> = SomethingSomething::new(Matter::new(...)), as this way (I hope) there would be no allocate-it-first-on-stack-and-then-move-into-rwdata-static` memory blolwups.

(Don't pay attention to the 'a lifetime. It can be 'static too, as the stuff that carries 'a can also be const-initialized this way.)

@jamesmunns
Copy link
Contributor Author

btw @ivmarkov, you might like some of the "Generic over RawMutex" shenanigans I'm doing over in the new postcard-rpc PR, to allow users to pick their mutex:

https://github.com/jamesmunns/postcard-rpc/blob/5b642d959cafc9b3822a056110f7e6e948bf3d02/source/postcard-rpc/src/target_server/mod.rs#L84-L101

It looks like this at the device level:

https://github.com/jamesmunns/postcard-rpc/blob/5b642d959cafc9b3822a056110f7e6e948bf3d02/example/full-setup/firmware/src/main.rs#L34-L37

@ivmarkov
Copy link

@jamesmunns How about naming it StaticConstCell instead?

(I considered StaticConstInitCell too but that's a bit too long for my taste.).

Reasons:

  • Kind of closer in naming to StaticCell and thus hinting that this guy is in a way a variation of the other; also matches closer the crate name itself
  • Both are only meaningful/useful in static contexts

@jamesmunns
Copy link
Contributor Author

@ivmarkov I don't love it! IMO the "const" part only applies at "init", the static itself isn't const. I don't feel that strongly about it if dirbaio prefers that tho.

@jamesmunns jamesmunns requested a review from Dirbaio April 26, 2024 09:51
@Dirbaio
Copy link
Member

Dirbaio commented Apr 26, 2024

ConstStaticCell perhaps?

@Dirbaio Dirbaio merged commit 380b73a into embassy-rs:main Apr 26, 2024
5 checks passed
@jamesmunns jamesmunns deleted the james/add-const-init-cell branch April 26, 2024 20:49
@Dirbaio
Copy link
Member

Dirbaio commented Apr 26, 2024

Released v2.1.0

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.

3 participants