Additions I made you may want to know about

[jnz86]

Just a small heads up in case you think any of these are a good idea and worth adding in to the repo.

Nothing I wrote here was good. In fact, it was all quite bad. For the most part I just used your existing functions with small logic around them. This wasn't as efficient as writing them "whole" but got the job done.

• size_t lwrb_overwrite(lwrb_t* buff, const void* data, size_t btw) - Functions like _write() but does not stop if it runs out of room will always return the total data stored. I needed a function that would take a rb that didn't have enough space for the incoming data, remove old data, and fit the new packet in. My use case for this was storing a log in a rolling buffer, it would be nice to not lose data, but it's more important to me in this case to lose some old data than new. One issue with this (and appeared somewhere else) is because this is an overwrite function, if I sent more than the size of the buffer, I wanted to store the latest data not the earliest. So if the size was 100, and I wrote 200, I wanted the second half of my data to be stored, not the first.

• size_t lwrb_copy(lwrb_t* dest, lwrb_t* src) - Copies one ring buffer to another. I needed to dump a rb for uart to a tokenizing assembly buffer. The issue here was that you can run into size limitations if one buffer is larger than the other. It also didn't make sense for my application to do this byte by byte, but I also couldn't be sure I had enough stack to allocate the buffer size on entry so I looked at packets of 128 bytes or smaller if there was less data.

• size_t lwrb_skip_up_to_offset(lwrb_t* buff, size_t len) - Your _peek has an offset. I needed the same thing for skip. I knew in one application I wanted to get rid of everything but the last n bytes. In my case, again I was tokenizing / parsing, and looking for a deliminator of a few bytes. If I couldn't find three byte delim, I didn't want to clear the last two bytes in the buffer, because those might turn into a complete deliminator soon. (ie: 01 02 03 wasn't found but 01 02 was and an 03 would be coming soon).

• uint8_t lwrb_find_seq(lwrb_t* buff, const void* const TOKEN, const size_t LEN_TOKEN, size_t* len) - I wanted to search through the ring buffer for a sequence. This was more difficult than it seemed it should have been the complexity is that just like ring buffers you don't know if your sequence is wrapped or not. In the end, I was in a hurry and made a linear copy of the whole buffer and quickly searched it. This returned true/false for found, and stored the position in at a de-referenced len. That might not be how you like to do it, but I prefer returning statuses and did not want to deal with a -1 for "not found". I also noticed you didn't use bool, although I normally would (no issue for C23!).

Nothing to do unless you want to. I would certainly find your take on these probably better than mine with the time I put in. Thought you might want to see how someone was using your code.

[MaJerle]

Thanks for this use cases, nice to see people find the lib helpful!

Are you in a position to add them to library and update the docs to be in-line with the rest? Especially I like overwrite function. I use such functionality in my smart home project to keep track of the last 30 temperatures, that are displayed on the chart.

Today I do it like:

if (lwrb_get_free(...) < sizeof(float)) {
    lwrb_skip(..., sizeof(float));
}
lwrb_write(..., &f, sizeof(float));

[jnz86]

It's pretty bad code I have. That's 95% of what I'm doing. The only difference is I am tracking for an offset if the size of data stored is larger than the size of the buffer. Normally that isn't an issue because the buffer isn't supposed to overwrite, and the user will have to deal with 200 bytes going up but only 100 length being returned.

This is hacky and I wrote it quickly. Honestly, I'm not sure if it's worth combining get_free(), skip() and write() into one more efficient function, or leaving them as separate (and tested) components.

size_t lwrb_overwrite(lwrb_t* buff, const void* data, size_t btw)
{
    size_t size_usable = buff->size - 1;
    size_t free        = lwrb_get_free(buff);
    size_t offset      = 0;

    if (!BUF_IS_VALID(buff) || data == NULL)
    {
        return 0;
    }

    // Make as much space as needed or available (if btw>size, clears whole buffer, not as good a reset())
    if (free < btw)
        lwrb_skip(buff, btw - free);

    // If we won't be able to fit everything, store latest, not earliest data
    if (size_usable < btw)
    {
        offset = btw - size_usable;
        btw    = size_usable;
    }

    return (lwrb_write(buff, data + offset, btw));
}

Between figuring out your documentation method and doing non-hacky versions, I could make more progress on the latter. Other than ARM, I don't have enough experience with optimization of platforms to make a decision on where to combine the functions or not. I could help, but I can't just go at it myself.

[MaJerle]

Let me create a new feature-overwrite branch and we can work there - pull requests are welcome and I will also contribute with tests. Let me know if that's OK for you..

[jnz86]

Sounds good! Top of my head change for efficency is that if you're going to write larger than the size of the buffer using an offset (different of what you would write and the size you can write), that a _reset() is faster and better than skipping at all data first.

You would only use skip if you knew you were going to retain some data that already exists.

(also, truth be told, the one I found the hardest is find_sequence() which probably applies to very few people besides me. I think when I go back to that one, I'll use the linear read to check for a wrapping sequence. At that point you only need a heap array the size of your sequence. Although it is obviously more steps)

[MaJerle]

Might be better idea then to put these functions in separate file, keeping them as "optional" for the build.

Branch done: https://github.com/MaJerle/lwrb/tree/develop-feature-overwrite

[jnz86]

I've seen you do it other places, is this somewhere you would use _ex ?

[MaJerle]

_ex or extended is being used in other libs to let user passing custom (let's say) group parameter.
For instance in case of printf, you can have different printf instances, each with its own print character implementation (UART, debug, sdcard, ...).
You can use lwprintf_..._ex and explicitly pass the parameter, or simply use lwprintf_... to use default one.

I suggest you add new features:

• Declaration in lwrb.h file
• Definition in lwrb_ex.c file, or something similar.

[jnz86]

Makes sense. Can do. I'm still waiting for you to add _ex to shell! ;)

If you want to move this conversation somewhere else let me know. Otherwise...

• An immediate thought I had with _overwrite is that it's a read op and a write op at the same time. At least in documentation it would need to be declared as both. I think this is true, because there is at least a chance you are moving the read pointer.

• Which brings me to thread-safety and multiple consumer/producer. You have a lock/unlock at the tx_start kickoff. And the documentation notes that thread safety is only for single producer/consumer. Did you consider putting mutex callbacks in the lwrb object itself? It's almost certainly out of scope and undesirable. But I figured I would ask if you considered a cb_lock_w etc in the object that the write/skip/adv/reset operations would call themselves if the pointer wasn't null. It would be adding 16 bytes per object and only appeal to people not using specifically as a pipe. As I write it, you're probably correct to not implement this as a feature, and let the user manage their own. In my case, I'm writing my TX and RX data to a log, which is multiple producer and need a mutex.

• I'll knock a draft of the _overwrite out today. It won't be very pretty and might need some work.

I'll do that soon, hvala.

[MaJerle]

Post this to shell, if you need a feature.

These features for lwrb are extended and user has to take care of that. No mutex is planned and no thread safety since we modify both pointers.

[jnz86]

#19 submitted

Oh, also, if there are going to be two .c files, it might make sense to move the macros to the header?

[jnz86]

I wrote a better copy() than I had roughed some tests in, do you want that as well?

[MaJerle]

Sure

[jnz86]

Updated.

[MaJerle]

Makes sense. Can do. I'm still waiting for you to add _ex to shell! ;)

MaJerle/lwshell@b52c415 It is a start at least

[jnz86]

T, fixed a bug. Not sure if you want to leave this open or how you want to proceed.

[MaJerle]

I am far behind my backlog - too many things - sorry. will work on that!