Add RAM-buffered queue (BufferedQueue, RamRing, SharedRamRing)#123
Open
leftger wants to merge 5 commits intotweedegolf:masterfrom
Open
Add RAM-buffered queue (BufferedQueue, RamRing, SharedRamRing)#123leftger wants to merge 5 commits intotweedegolf:masterfrom
leftger wants to merge 5 commits intotweedegolf:masterfrom
Conversation
Introduces queue::buffer with: - RamRing<N>: fixed-capacity ring buffer for variable-length items - OverflowPolicy: Err or DiscardOldest on full ring - BufferedQueue<S,C,N>: accepts sync enqueues into RAM, drains to flash async - SharedRamRing<N>: ISR-safe variant using embassy-sync (optional embassy feature) FIFO ordering is preserved across the RAM→flash boundary. Power-fail note: items still in RAM at power loss are discarded; drained items carry the usual sequential-storage guarantees.
diondokter
reviewed
Mar 19, 2026
SharedRamRing no longer depends on embassy-sync. The blocking mutex is replaced with critical_section::Mutex (borrow pattern), and the Signal is replaced with a minimal WakeSignal built from an AtomicBool and a stored core::task::Waker — no runtime dependency required. The feature flag is renamed from `embassy` to `critical-section` to reflect the actual dependency.
…Signal" This reverts commit a7b0efb.
Member
|
Oh we've got new clippy lints and it's rightfully pointing out I've been lazy 🙈 I'll improve that when the next breaking change comes around I guess |
- Rename feature `embassy` to `shared-ram-ring` per maintainer request
- Run cargo fmt (import order, line-length wrapping)
- Add #[allow(clippy::result_unit_err)] to push/push_overwriting/enqueue
- Fix include!("mock_flash.rs") → include!("../mock_flash.rs") in doctest
(path is now relative to src/queue/mod.rs, not src/queue.rs)
Author
|
Let me know if there's anything else I need to address. Happy to hopefully contribute to your awesome work! |
diondokter
reviewed
Mar 19, 2026
src/queue/buffer.rs
Outdated
Comment on lines
+302
to
+304
| if !self.ram.is_empty() { | ||
| self.drain_all(data_buffer, allow_overwrite).await?; | ||
| } |
Member
There was a problem hiding this comment.
Why this? Not wrong per se, but we could also peek the storage, then if none, peek the oldest from ram
src/queue/buffer.rs
Outdated
Comment on lines
+285
to
+287
| if !self.ram.is_empty() { | ||
| self.drain_all(data_buffer, allow_overwrite).await?; | ||
| } |
src/queue/buffer.rs
Outdated
Comment on lines
+335
to
+338
| /// Mutable reference to the underlying [`QueueStorage`] for direct access. | ||
| pub fn storage(&mut self) -> &mut QueueStorage<S, C> { | ||
| &mut self.storage | ||
| } |
Member
There was a problem hiding this comment.
This allows people to do weird things that would alter the ordering. Is there a real reason we need this? It's kinda footgunny
src/queue/buffer.rs
Outdated
Comment on lines
+318
to
+323
| /// Byte length of the oldest item in the RAM ring, or `None` if the ring is empty. | ||
| /// | ||
| /// Use this to size the `scratch` buffer passed to [`drain_one`][Self::drain_one]. | ||
| pub fn oldest_ram_item_len(&self) -> Option<usize> { | ||
| self.ram.oldest_len() | ||
| } |
Member
There was a problem hiding this comment.
I see what this is trying to do. But is it actually gonna be used in practice? It can be helpful maybe if you have alloc available...
… oldest_ram_item_len - pop/peek now read flash first (flash items are always older) and fall back to the oldest RAM item if flash is empty, avoiding unnecessary drain-all-to-flash on every read; allow_overwrite removed from both since they no longer write to flash - Remove pub storage() accessor — it allowed callers to bypass ordering invariants (footgunny per reviewer) - Remove oldest_ram_item_len() — not practically useful in no_std without alloc; callers can size scratch buffers statically - Restructure pop_reads_flash_before_ram test to use QueueStorage directly instead of the removed storage() accessor
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This adds a RAM-buffered wrapper around
QueueStoragefor cases where the producer is faster than NOR flash can keep up with.RamRing<N>— compact fixed-capacity ring buffer for variable-length byte items (2-byte LE length prefix per item)OverflowPolicy—ErrorDiscardOldestwhen the ring is fullBufferedQueue<S, C, N>— wrapsQueueStoragewith a RAM ring;enqueueis synchronous and never touches flash,drain_one/drain_allcommit items to flash asynchronouslySharedRamRing<N>— ISR-safe variant behind the optionalembassyfeature; wraps the ring in aCriticalSectionRawMutexand uses an EmbassySignalto wake a drain taskFIFO ordering is preserved across the RAM→flash boundary:
pop/peekdrain any pending RAM items to flash first, then read from flash.Power-fail note: items still in the RAM ring at power loss are discarded; items that have been drained carry the usual sequential-storage guarantees.
Dependency note
SharedRamRingpulls inembassy-sync(gated behind theembassyfeature). Theembassy-synctypes are not part of the public API — the only public surface isSharedRamRing<N>itself. Happy to revisit the dependency (e.g. swap the mutex for directcritical_section::withcalls and hand-roll a minimal signal) if you'd prefer to avoid it.