charlesnicholson / nanocobs Goto Github PK

Once #47 is done, update the readme and add examples of the new incremental encoding approaches.

Handy for sending COBS frames over a streaming link where you don't want or need to have the entire frame in RAM. Get a chunk, decode it, do whatever you want with the newly-decoded chunk, flush it, get another chunk, repeat.

Probably just the natural opposite of the incremental encoder.

When running the tests using the code from #39 with the addition of the integer option to -fsanitize, I get this warning:

cobs.c:107:17: runtime error: unsigned integer overflow: 0 - 1 cannot be represented in type 'unsigned int'

We get in variable-length COBS encoded messages. In the README, there's an example with a fixed, 64-byte long buffer:

char buf[64];

// You fill 'buf' with an encoded cobs frame (from uart, etc) that ends with 0x00.
unsigned const length = you_fill_buf_with_data(buf);

cobs_ret_t const result = cobs_decode_inplace(buf, length);
if (result == COBS_RET_SUCCESS) {
  // decoding succeeded, 'buf' bytes 0 and length-1 are COBS_SENTINEL_VALUE.
  // your data is in 'buf[1 ... length-2]'
} else {
  // decoding failed, look to 'result' for details.
}

However, if the buffer buf is variable-length and for example 10000 bytes long, I don't think the last valid byte will be at length-2. Is there an easy way to figure out the length of the decoded message? Maybe decode_inplace can in the len parameter take a pointer to an int, and update it to the decoded length?

See here for an executable version: https://godbolt.org/z/MdT5befeq

Driver Program:

#include "cobs.h"
#include <iostream>

using namespace std;
int main()
{
    char buf[] = {0x2, 0, 0};
    auto ret = cobs_decode_inplace(buf, sizeof(buf));

    cout << "returned COBS_RET_SUCCESS: " << (ret == COBS_RET_SUCCESS) << endl;

    cout << "buffer: " << hex << endl;
    for(int i = 0; i < sizeof(buf); i++)
    {
        cout << (int)(buf[i]) << " ";
    }
    cout << endl;
}

Output:

returned COBS_RET_SUCCESS: 1
buffer: 
5a 0 5a 

Expected Output:
Return COBS_RET_ERR_BAD_PAYLOAD since the input was invalid due to the intermediate 0 byte.

It's the straggler. Enough of the other tests (wiki, paper, encode, decode-inplace) work that I'm pretty sure it's good, but it's bad to leave it untested.

Make cobs_encode_inc look like cobs_decode_inc - each call to cobs_encode_inc should take src + dst pointers, so that you don't need enough RAM to hold a fully-encoded buffer if you don't need that.

Thank you for the extremely free license for this, this definitely makes my life easier. For me, it would be useful if the license information is tracked per-file with standard SPDX headers. https://spdx.dev/learn/handling-license-info/#why. Is this something you would be willing to implement? I can also make a PR if that's easier for you.

// SPDX-License-Identifier: Unlicense OR 0BSD

If the frames you're encoding have a user-supplied payload and a link-layer-supplied header, it costs an extra buffer and copy to make the final frame contiguous- you have to do something wasteful like this:

unsigned char final_buf[128];
header_t const h = {.magic = 0x12, .len = payload_len}; // packed
memcpy(final_buf, &h, sizeof(h));
memcpy(&final_buf[sizeof(h)], payload, payload_len);

If the COBS encoding interface instead exposed a context struct, the api could look like this

cobs_ret_t cobs_encode_inc_begin(void *dst, unsigned dst_max, cobs_encode_ctx_t *out_ctx);
cobs_ret_t cobs_encode_inc(cobs_encode_ctx_t *ctx, void const *src, unsigned src_len);
cobs_ret_t cobs_encode_inc_end(cobs_encode_ctx_t *ctx, unsigned *out_enc_len);

Then the current cobs_encode function could be reimplemented in terms of begin/inc/end.

Users without full header/payload frame buffers would then call

unsigned char buf[128]; // or whatever
unsigned buf_len;

cobs_encode_ctx_t ctx;
cobs_encode_inc_begin(buf, buf_len, &cobs_ctx);
cobs_encode_inc(&ctx, header, sizeof(header));
cobs_encode_inc(&ctx, payload, payload_len);

unsigned enc_len;
cobs_encode_inc_end(&ctx, &enc_len);

phy_tx(buf, enc_len);

cobs_encode_inc and cobs_encode_inc_end need return values for buffer exhaustion.