lwpkt icon indicating copy to clipboard operation
lwpkt copied to clipboard

Decoupling lwpkt from lwrb for More Flexibility

Open SONGMENGSEN opened this issue 6 months ago • 2 comments

I believe lwrb is a great module, and I've been using it extensively. It works wonderfully! However, I think lwpkt should be an independent module, and its usage should not necessarily require lwrb.

Instead of relying on lwrb for the ring buffers, I suggest we could replace the following:

lwrb_t* tx_rb;  /*!< TX ringbuffer */
lwrb_t* rx_rb;  /*!< RX ringbuffer */

With something like:

size_t(*read)(char*, size_t);                
size_t(*write)(char*, size_t);

This would allow users to define their own read and write functions, making the module more independent. Users could still choose to use the lwrb module if they wish, but the lwpkt module would no longer be tightly coupled to it.

SONGMENGSEN avatar Oct 26 '25 16:10 SONGMENGSEN

At this point in time I am not sure if and how good idea it is. The intent is clear to me. We'd need to do few things:

  • Make sure the backward compatibility is kept with lwrb, by means of some options that allow one to enable the custom implementation. The lwrb remains default config
  • The functions would need to be at least read, write and available_size to ensure lwpkt can get and send the byte, plus to send the bytes only if full packet could possibly fit

I'm open for a pull request if one wants to do it. I am a bit tight with time these days.

MaJerle avatar Oct 26 '25 17:10 MaJerle

Here's my work in progress on decoupling it for use in Arduino:

lwrb.h

#pragma once
#include <Arduino.h>
typedef struct {
    HardwareSerial *serial;
    bool native_direction;
} lwrb_t;

#define lwrb_write(rb,b,n) ((size_t)((rb)->native_direction ? (rb)->serial->write((uint8_t *)(b),n) : (abort(), 0)))
#define lwrb_read(rb,b,n) ((size_t)((rb)->native_direction ? (rb)->serial->readBytes((uint8_t *)(b),n) : (abort(), 0)))
#define lwrb_get_free(rb) ((size_t)((rb)->native_direction ? (rb)->serial->availableForWrite() : (abort(), 0)))
#define lwrb_get_full(rb) ((size_t)((rb)->native_direction ? (rb)->serial->available() : (abort(), 0)))

lwpkt.cpp

#include "lwpkt/lwpkt.c"

And it seems to only need a small modification to work:

--- C:\Users\otac0n\AppData\Local\Temp\git-blob-a32600\lwpkt.c	2025-12-17 14:07:54.000000000 -0800
+++ C:\Users\otac0n\Projects\MkII\lib\lwpkt\lwpkt\src\lwpkt\lwpkt.c	2025-12-17 14:02:14.000000000 -0800
@@ -182,13 +182,13 @@
  * \param[in]       inp: Input data in byte format
  * \param[in]       len: Number of bytes to process
  * \return          Current CRC calculated value after all bytes or `0` on error input data
  */
 static uint32_t
 prv_crc_in(lwpkt_t* pkt, lwpkt_crc_t* crcobj, const void* inp, const size_t len) {
-    const uint8_t* p_data = inp;
+    const uint8_t* p_data = (uint8_t*)inp;
 
     if (crcobj == NULL || p_data == NULL || len == 0) {
         return 0;
     }
 
     for (size_t i = 0; i < len; ++i, ++p_data) {

This works!

Image

otac0n avatar Dec 17 '25 23:12 otac0n