jonas-schievink / rubble

(going to be a) BLE stack for embedded Rust
BSD Zero Clause License
397 stars 56 forks source link

Use bbqueue from crates.io #71

Closed jonas-schievink closed 4 years ago

jonas-schievink commented 5 years ago

As discovered in https://github.com/jonas-schievink/rubble/pull/64, bbqueue's grant_max doesn't work as documented (https://github.com/jamesmunns/bbqueue/issues/29), so our usage of it is wrong and can lead to packet processing stalling.

This either needs a documentation fix and additional methods in bbqueue, or we should switch to a different queue. Ideally, that queue would also support thumbv6 while still having efficient batch operations.

jonas-schievink commented 5 years ago

8b62569 uses a fork that includes https://github.com/jamesmunns/bbqueue/pull/33. We should move back to the upstream repo or a crates.io release as soon as we can.

jamesmunns commented 4 years ago

@jonas-schievink is this something that would be fixed by https://github.com/jamesmunns/bbqueue/pull/37?

Or do you need one of the proposed grants at https://github.com/jamesmunns/bbqueue/issues/38, maybe grant_largest or grant_largest_max?

jonas-schievink commented 4 years ago

@jamesmunns Does grant_max_remaining now work as documented? If so then that's all I need.

jamesmunns commented 4 years ago

Ah no, good catch! It works as documented at the module level (see https://docs.rs/bbqueue/0.4.0-alpha5/bbqueue/atomic/struct.Producer.html), but it is a little vague at the function level, which I will be fixing. For reference:

I think you actually want grant_largest_max(), which will cause a wraparound if the requested space is NOT available at the end, but IS available at the beginning.

jamesmunns commented 4 years ago

I need to come up with a consistent naming scheme, but in general:

jonas-schievink commented 4 years ago

I think you actually want grant_largest_max(), which will cause a wraparound if the requested space is NOT available at the end, but IS available at the beginning.

The only remaining use of bbqueue is in the logging utilities, which should be fine with getting a small buffer first, and then wrap around and get another buffer. It will only "fail" (it will consider data to be lost) if the function returns Err.

jamesmunns commented 4 years ago

I also plan to offer split grants in the future, which will give you the buf at the start and end, so you can decide at once if there is enough space, but I haven't put any detailed design or dev into that, planning on doing that post 0.4.0.

jonas-schievink commented 4 years ago

Fixed by #100