Embedded IoT: local data storage when no network coverage

Question

I am working on a part of an IoT project where I have been given a task to develop a mechanism to store data locally when there is no network coverage or if the network is down for some reason. Our application is architected such that, every hour we store the feed/sensor data to a bucket i.e.local storage and we have a network task that constantly checks for data in the bucket. If there is data, then it transmits all the data to the server. I have developed this software component and performed some unit tests. So far everything seems to work fine and I don't see any obvious issues with it. However, I believe there may be loopholes that I am overseeing and loads of scope for improvements in terms of the code structure and logic. Hence, I'd like to present you with all my code and seek your expert advice, and review feedbacks. Thank you for your time and patience.

Below is the header file.

/*
* Bucket.h
*
* Created: 17/12/2020 12:57:45 PM
*  Author: Vinay Divakar
*/


#ifndef BUCKET_H_
#define BUCKET_H_

#define BUCKET_SIZE           32        // Range 10-32000 Memory in bytes allocated to bucket for edge storage
#define BUCKET_MAX_FEEDS      32        // Range 2-64 Number of unique feeds the bucket can hold
#define BUCKET_MAX_VALUE_SIZE   64      // Maximum value string length

// Values passed via void pointer so this is needed so functions know how to use the void pointer
typedef enum {
  UINT16,
  STRING
} cvalue_t;

// Used to register feeds to the bucket and for passing sensor data to the bucket
typedef struct {
  const char* key;  // MUST BE CONST, since the feeds reference key will never change.
  cvalue_t type;        // Data type     
  void* value;      // Value
  uint32_t unixTime;    // Timestamp
} cbucket_t;

int8_t BucketRegisterFeed(cbucket_t *feed);
int8_t BucketPut(const cbucket_t *data);
int8_t BucketGet( char *keyOut, char *dataOut, uint32_t *timestampOut);
uint8_t BucketNumbOfRegisteredFeeds(void);
void BucketZeroize(void);

//Debug functions
void DebugPrintRegistrationData(void);
void DebugPrintBucket(void);
void DebugPrintBucketTailToHead(void);

#endif /* BUCKET_H_ */

Below is the source code.

/*
* Bucket.c
*
* Created: 17/12/2020 12:57:31 PM
* Author: Vinay Divakar

* Description: This module is used to accumulate data when the network is down or unable to transmit data due to poor signal quality. It currently supports uint16_t and string data types.  The bucket is designed to maximize the amount of data that can be stored in a given amount of memory for typical use. The feeds must be registered before they can be written to or read from the bucket. 

The BucketPut API writes the feed to the bucket.
E.g. 1: If the BucketPut sees that consecutive feeds written have the same timestamp, then it writes the data as shown below. This is done to save memory.
                        [Slot] [Data] [Slot] [Data] [Slot] [Data]...[Timestamp]
E.g. 2: If the BucketPut sees that consecutive feeds written have different timestamps, then it writes the data as shown below.
                        [Slot] [Data] [Timestamp] [Slot] [Data] [Timestamp] [Slot] [Data] [Timestamp]...
E.g. 3: If the BucketPut sees a mixture of above, then it writes the data as shown below.
                        [Slot] [Data] [Slot] [Data] [Slot] [Data] [Timestamp] [Slot] [Data] [Timestamp] [Slot] [Data] [Slot] [Data] [Timestamp]
The BucketGet API reads the feed in the following format.
                        [Key] [Value] [Timestamp]

* Notes:
1. Module hasn't been tested for the STRING data type. 
*/

/*
*====================
   * Includes
   *====================
   */
#include <stdio.h>
#include <string.h>
#include "atmel_start.h"
#include "Bucket.h"
//#include "INIconfig.h"

/* For debug purposes */
const char* cvaluetypes[] = {"UINT16", "STRING"};

/*
*====================
* static/global vars
*====================
*/
// Stores the registered feed 
cbucket_t *registeredFeed[BUCKET_MAX_FEEDS];
const uint8_t unixTimeSlot = 0xFF;  //virtual time slot
// Bucket memory for edge storage and pointers to keep track or reads and writes
uint8_t cbucketBuf[BUCKET_SIZE];
uint8_t *cBucketBufHead = &cbucketBuf[0];
uint8_t *cBucketBufTail = &cbucketBuf[0];

/*
*====================
* Fxns
*====================
*/
int8_t _RegisteredFeedFreeSlot(void);
void _PrintFeedValue(cbucket_t *feed);

/****************************************************************
 * Function Name    : BucketZeroize
 * Description      : Zeroize the bucket
 * Returns          : None.
 * Params           :None.
 ****************************************************************/
void BucketZeroize(void){
    memset(cbucketBuf, 0, sizeof(cbucketBuf));
}

/****************************************************************
 * Function Name    : BucketRegisterFeed
 * Description      : Links cbucket structures from the main
 application to the bucket (Used to encode/decode
 data in/out of the bucket)
 * Returns          : true on success else negative.
 * Params       @feed :Feed to register
 ****************************************************************/
int8_t BucketRegisterFeed(cbucket_t *feed) {
    int8_t slot = _RegisteredFeedFreeSlot();
    int8_t registrationSucess = -1;
    if (slot != -1) {
        registeredFeed[slot] = feed;
        registrationSucess = 0;
    }
    return registrationSucess;
}

/****************************************************************
 * Function Name    : BucketGetRegisteredFeedSlot
 * Description      : Gets slot index of the registered feed
 * Returns          : !0 on OK, -ve on error
 * Params           @data: points to the feed struct
 ****************************************************************/
int8_t BucketGetRegisteredFeedSlot(const cbucket_t *data){
    int8_t slotIdx = -1;
    /* Check if the feed had been previously registered */
    for(int i = 0 ; i < BUCKET_MAX_FEEDS ; i++) {
        //found it?
        if(data == registeredFeed[i]) {
            //Get the slot index
            slotIdx = i; i = BUCKET_MAX_FEEDS;
        }
    }
    return(slotIdx);
}

/****************************************************************
 * Function Name    : BucketCheckDataAvailable
 * Description      : Checks for data in the bucket
 * Returns          : false on empty else true.
 * Params           None.
 ****************************************************************/
int8_t BucketCheckDataAvailable(void){
    //Bucket is empty?
     if(cBucketBufTail == cBucketBufHead){
        return(false); 
    }
     return(true);
}

/****************************************************************
 * Function Name    : BucketGetTimeStamp
 * Description      : Gets the timestamp for the specific key
 * Returns          : true on OK, false on empty, -1 on error
 * Params           : None.
 ****************************************************************/
int8_t BucketPutGetTimeStamp(uint32_t *timestamp){
    int8_t status = false;
    
    //There's no data left to be read from the bucket i.e. tail = head
    if(BucketCheckDataAvailable() == false){
        return(false);
    }
    
    //Attempt looking for the time slot, 
    uint8_t *cBucketBufHeadTmp = cBucketBufHead;
    //Iterate through the cells while handling wraparounds
    for(int i = 0 ; i < 5 ; i++){
        if(cBucketBufHeadTmp == &cbucketBuf[0]){ //if head points to start of bucket, wrap to end of the bucket
            cBucketBufHeadTmp = &cbucketBuf[BUCKET_SIZE];
        }
        //Step back one address
        cBucketBufHeadTmp--;
    }
    
    //Now, we should be pointing to the virtual time slot i.e. 0xff
    if(*cBucketBufHeadTmp == unixTimeSlot){
        //Check if head points to the end of the bucket, wrap to start of the bucket
        if(cBucketBufHeadTmp == &cbucketBuf[BUCKET_SIZE]){//if the last cell in the bucket is the time slot, skip it by wrapping around to point to the start of the bucket
            cBucketBufHeadTmp = &cbucketBuf[0];
        }else{//Inc address to skip the virtual time slot i.e. 0xFF
            cBucketBufHeadTmp++;    
        }
        
        //load the timestamp
        *timestamp = 0;
        for(int i = 0 ; i < sizeof(uint32_t) ; i++, cBucketBufHeadTmp++){
            //Now if head points to end of bucket, read that value and wrap to start of bucket
            if(cBucketBufHeadTmp == &cbucketBuf[BUCKET_SIZE]){//Handle if head needs to be wrapped around to read the timestamp
                cBucketBufHeadTmp = &cbucketBuf[0]; //point to start of the bucket to continue reading the timestamp
                if(i == 0){
                    *timestamp |= (*cBucketBufHeadTmp & 0xFF);
                }else if(i == 1){
                    *timestamp |= (*cBucketBufHeadTmp & 0xFF) << 8;
                }else if(i == 2){
                    *timestamp |= (*cBucketBufHeadTmp & 0xFF) << 16;
                }else if(i == 3){
                    *timestamp |= (*cBucketBufHeadTmp & 0xFF) << 24;
                }
            }else{//read value as is
                if(i == 0){
                    *timestamp |= (*cBucketBufHeadTmp & 0xFF);
                }else if(i == 1){
                    *timestamp |= (*cBucketBufHeadTmp & 0xFF) << 8;
                }else if(i == 2){
                    *timestamp |= (*cBucketBufHeadTmp & 0xFF) << 16;
                }else if(i == 3){
                    *timestamp |= (*cBucketBufHeadTmp & 0xFF) << 24;
                }
            }//else
        }//for
        //we found it!
        status = true;
    }else{
            //Did not find? likely to be the first write
    }
    return(status);
}

/****************************************************************
 * Function Name    : BucketGetDataSize
 * Description      : Gets the size of the item
 * Returns          : false on error, true on OK
 * Params       @data :points to feed struct
 ****************************************************************/
uint8_t BucketGetDataSize(const cbucket_t *data){
    uint8_t dataSizeOut = 0;
    if(data->type == UINT16) {
        //Total data size  = 2 bytes i.e. 16 bit unsigned 
        dataSizeOut = sizeof(uint16_t);
    }else if(data->type == STRING) {
        //Total data size = length of the string until null terminated. Now get the length of the string by looking upto '\0'
        const uint8_t *bytePtr =  (uint8_t*)data->value;
        for(int i = 0; i  < BUCKET_MAX_VALUE_SIZE; i++, bytePtr++) {
            if(*bytePtr == '\0') {
                i = BUCKET_MAX_VALUE_SIZE;
                //Include the '\0' to be written. This will be used to indicate the end of string while reading
                dataSizeOut++;
                continue;
            }
            dataSizeOut++;
        }
    } else {
        return (false);
    } // invalid data type, discard it!
    return(dataSizeOut);
}

/****************************************************************
 * Function Name    : BucketWriteData
 * Description      : Writes the item to the bucket
 * Returns          : false on error or No. of bytes written.
 * Params       @slotIdx = slot index of the feed.
                @dataSizeIn = size of the item.
                @bucketHeadPtr = ptr to ptr to bucket head.
                @data = ptr to feed struct.
 ****************************************************************/
uint8_t BucketWriteData(int8_t slotIdx, uint8_t dataSizeIn, const cbucket_t *data){
    //Do a sanity check for the right data type
    if(data->type == UINT16){

    } else if(data->type == STRING){
        
    }else{
        return(false);
    }

    if(cBucketBufHead >= &cbucketBuf[BUCKET_SIZE]) 
        cBucketBufHead = &cbucketBuf[0];
    
    //Write the slot index to the bucket
    *cBucketBufHead++ = slotIdx;
    //Point to the value to be read byte by byte, in this case, data type of the value doesn't matter 
    uint8_t *bytePtr =  (uint8_t*)data->value;
    //By now we must have the right data size to be written. Write the item to the bucket
    for(uint8_t i = 0 ; i < dataSizeIn ; i++, cBucketBufHead++) {
        if(cBucketBufHead >= &cbucketBuf[BUCKET_SIZE]) 
            cBucketBufHead = &cbucketBuf[0];
        *cBucketBufHead = bytePtr[i];
    }
    //Write the virtual address of the time slot. This address is used to identify the start of timestamp data i.e. 4 bytes.
    if(cBucketBufHead >= &cbucketBuf[BUCKET_SIZE])
        cBucketBufHead = &cbucketBuf[0];
    *cBucketBufHead++ = unixTimeSlot;
    
    //Write the timestamp to the bucket corresponding to the above slot address 
    uint32_t unixTimeTmp = data->unixTime;
    printf("[BucketWriteData], unixTimeTmp: %lu\r\n", unixTimeTmp);
    for(int i = 0; i < sizeof(data->unixTime); i++, cBucketBufHead++){
        if(cBucketBufHead >= &cbucketBuf[BUCKET_SIZE]) 
            cBucketBufHead = &cbucketBuf[0];
        *cBucketBufHead = unixTimeTmp & 0xFF;
        unixTimeTmp >>= 8;
    }
    return(true);
}

/****************************************************************
 * Function Name    : BucketPut
 * Description      : writes to the bucket
 * Returns          : true on success else negative..
 * Params       @data :points to struct to be written
 ****************************************************************/
int8_t BucketPut(const cbucket_t *data) {
    int8_t slot = 0;
    uint8_t dataSize = 0;
    
    //Find the slot for this feed
    slot = BucketGetRegisteredFeedSlot(data);
    if(slot < 0){ 
        printf("[BucketPut], Error, feed not registered\r\n"); 
        return(-1); 
    } 
        
    //Get th size of the item and handle storing as appropriate 
    dataSize =  BucketGetDataSize(data);
    if(dataSize == false){  
        printf("[BucketPut], Error, invalid feed type\r\n");  
        return(-2); 
    }
    
    //Get the amount space left in the bucket 
    printf("[BucketPut], dataSize = %u\r\n", dataSize);
    int remaining = (cBucketBufTail + BUCKET_SIZE - cBucketBufHead-1) % BUCKET_SIZE;
    printf("bucket size = %d    remaining = %hu \r\n", (int)BUCKET_SIZE, remaining);
       
    //Get the timestamp from the unix time slot i.e. special virtual slot to hold the timestamp 
    uint32_t lastStoredTime = 0;
    if(BucketPutGetTimeStamp(&lastStoredTime) == false){//last stored time slot not found?, likely because there was no data left to read and this is the first write after bucket empty
        //Check if there is enough space in the bucket for storing the current feed data. Total size to account for =  slot size + data size + unix timestamp size + unix time slot size .
        uint8_t totalItemSize = dataSize + sizeof(slot) + sizeof(data->unixTime) + sizeof(unixTimeSlot);
        if(totalItemSize > remaining) { 
            printf("[BucketPut], Error, no space available, space = %hu,    item size = %hu\r\n", remaining, totalItemSize); 
            return(-4); 
        }else{
            printf("[BucketPut], available space = %hu  total item size = %hu\r\n", remaining, totalItemSize);
        }
    }else{//last stored timestamp found, likely because there was some previous data in there
        if(lastStoredTime == data->unixTime){//last stored timestamp matches the current feeds timestamp
            printf("[BucketPut] Last stored timestamp[%lu] = Current feed timestamp[%lu]\r\n", lastStoredTime, data->unixTime);
            uint8_t totalItemSize = dataSize + sizeof(slot);
            if(totalItemSize > remaining) { 
                printf("[BucketPut], Error, no space available, space = %hu,    item size = %hu\r\n", remaining, totalItemSize); 
                return(-5); 
            }else{
                printf("[BucketPut], available space = %hu  total item size = %hu\r\n", remaining, totalItemSize);
            }
            
            //Get total size of the time frame
            uint8_t stepOffset = sizeof(slot) + sizeof(data->unixTime);
            //move the head ptr backwards by size of time slot index + size of timestamp i.e. 5 bytes
            for(int i = 0 ; i < stepOffset ; i++){
                if(cBucketBufHead <= &cbucketBuf[0]){//if head points to start of the bucket, wrap to the end of the bucket and continue writing the value from there.
                    cBucketBufHead = &cbucketBuf[BUCKET_SIZE];
                }
                 cBucketBufHead--; //dec head by one
            }
            //Now, overwrite from here the new data and append with the new feeds timestamp which would be the same as the last stored timestamp. This is done to save memory.
        }else{//last stored timestamp is different from the current feeds timestamp
            printf("[BucketPut] Last stored timestamp[%lu] != Current feed timestamp[%lu]\r\n", lastStoredTime, data->unixTime);
            //Perform a usual write while appending the new timestamp for the current feed.Check if there is enough space in the bucket for storing the current feed data
            //Total size to account for =  slot size + data size + unix timestamp size + unix time slot size .
            uint8_t totalItemSize = dataSize + sizeof(slot) + sizeof(data->unixTime) + sizeof(unixTimeSlot);
            if(totalItemSize > remaining) { 
                printf("[BucketPut], Error, no space available, space = %hu,    item size = %hu\r\n", remaining, totalItemSize); 
                return(-6); 
            }else{
                printf("[BucketPut], available space = %hu  total item size = %hu\r\n", remaining, totalItemSize);
            }   
        }
    }//else
    
    //If we have reached here, means all checks have passed and its safe to write the item to the bucket
    uint8_t status =    BucketWriteData(slot, dataSize, data);
    if(status == false){//check if the passed type is supported
        printf("[BucketPut], Error, invalid data type\r\n");
        return(-4);
    }
    
    return(true);
}

/****************************************************************
 * Function Name    : BucketGetTimestampForFeed
 * Description      : Gets the timestamp for the feed
 * Returns          : false on error else true.
 * Params           @timestamp: feeds timestamp
 ****************************************************************/
int8_t BucketGetTimestampForFeed(uint32_t *timestamp){
    int8_t status = false;
    *timestamp = 0;
    
    if(cBucketBufTail >= &cbucketBuf[BUCKET_SIZE]){//if tail points to end of bucket or beyond
        cBucketBufTail = &cbucketBuf[0];//wrap around to point start of the bucket  and continue reading
    }
    //Check if tail is pointing to the virtual time slot
    if(*cBucketBufTail == unixTimeSlot){//If so, read the timestamp
        printf("[BucketGetTimestampForFeed], tail pointing to time slot, read it.\r\n");
        *cBucketBufTail++ = 0;  //Skip the virtual timeslot and point to the timestamp
        for(int i = 0 ; i < sizeof(uint32_t) ; i++){
            if(cBucketBufTail >= &cbucketBuf[BUCKET_SIZE]){//if tail points to end of bucket or beyond
                cBucketBufTail = &cbucketBuf[0];//wrap around to point start of the bucket and continue reading
            }
            //read the timestamp
            if(i == 0){
                *timestamp |= (*cBucketBufTail & 0xFF);
            }else if(i == 1){
                *timestamp |= (*cBucketBufTail & 0xFF) << 8;
            }else if(i == 2){
                *timestamp |= (*cBucketBufTail & 0xFF) << 16;
            }else if(i == 3){
                *timestamp |= (*cBucketBufTail & 0xFF) << 24;
            }
            *cBucketBufTail++ = 0;
        }//for
        status = true;
    }else{//timeslot not found?
        //means the next byte is the slot index for the next feed which is not what we are looking for.
        //Lets look beyond, it must be there! - if not, then there's something seriously wrong with the code
        uint8_t *cBucketBufTailTmp = cBucketBufTail;
        //Iterate and look for the time slot
        while(cBucketBufTailTmp++ < &cbucketBuf[BUCKET_SIZE]){
            if(cBucketBufTailTmp >= &cbucketBuf[BUCKET_SIZE]){//Check if tail reached or beyond the bucket's end.
                cBucketBufTailTmp = &cbucketBuf[0];//Wrap around to point to the start of the bucket and continue to read.
                printf("[BucketGetTimestampForFeed], reached end of bucket, wrapping around...\r\n");
            }
            //Check if we are pointing to the virtual time slot, we should find it at some point.
            if(*cBucketBufTailTmp == unixTimeSlot){//yes!, found it.
                printf("[BucketGetTimestampForFeed], yes!, time slot has been found.\r\n");             
                cBucketBufTailTmp++;//Skip the virtual timeslot and point to the start of the timestamp
                for(int  i = 0 ; i < sizeof(uint32_t) ; i++, cBucketBufTailTmp++){
                    if(cBucketBufTailTmp >= &cbucketBuf[BUCKET_SIZE]){//Check if tail reached or beyond the bucket's end.
                        cBucketBufTailTmp = &cbucketBuf[0];//Wrap around to point to the start of the bucket and continue to read.
                    }
                    //Read the time stamp
                    if(i == 0){
                        *timestamp |= (*cBucketBufTailTmp & 0xFF);
                    }else if(i == 1){
                        *timestamp |= (*cBucketBufTailTmp & 0xFF) << 8;     
                    }else if(i == 2){
                        *timestamp |= (*cBucketBufTailTmp & 0xFF) << 16;
                    }else if(i == 3){
                        *timestamp |= (*cBucketBufTailTmp & 0xFF) << 24;
                    }
                }//for
                status = true;
                break;
            }//timeslot
        }//while
    }//else
    return(status);
}

/****************************************************************
 * Function Name    : BucketGetReadData
 * Description      : Reads the key and value for that feed/slot.
 * Returns          : false error, true on success.
 * Params           @key: key to be populated(static).
                    @value: value read from the bucket.
 ****************************************************************/
int8_t BucketGetReadData(char *key, char *value){
    int8_t slotIdx;

    //Check if the tail is pointing to the end of the bucket or beyond, wrap around to start of bucket to continue reading
    if(cBucketBufTail >= &cbucketBuf[BUCKET_SIZE])
        cBucketBufTail = &cbucketBuf[0];
        
    //Read the slot index for the feed
    slotIdx = *cBucketBufTail;  //this is an int8_t type, if greater, will lead to undefined behavior.
    *cBucketBufTail++ = 0;
    if(slotIdx > BUCKET_MAX_FEEDS){ 
        printf("[BucketGetReadData], Error, Slot[%u] index is out of bounds\r\n", slotIdx); 
        return(false); 
    }else{
        printf("[BucketGetReadData], Slot[%d] = 0x%x\r\n", slotIdx, (uint32_t)&registeredFeed[slotIdx]->key);
    }
    //Copy the key for the corresponding slot
    strncpy(key, registeredFeed[slotIdx]->key, strlen(registeredFeed[slotIdx]->key));
    //If the feed is of type UINT16_t
    if(registeredFeed[slotIdx]->type == UINT16){
        uint16_t dataU16 = 0;
        for(int i = 0 ; i < sizeof(uint16_t) ; i++){//Read 2 bytes
            if(cBucketBufTail >= &cbucketBuf[BUCKET_SIZE])//Check if we are pointing to the end of the bucket
                cBucketBufTail = &cbucketBuf[0];//Wrap around to point to the start of the bucket to continue reading
            if(i == 0){
                dataU16 |= (*cBucketBufTail & 0xFF);
            }else if(i == 1){
                dataU16 |= (*cBucketBufTail & 0xFF) << 8;   
            }
            *cBucketBufTail++ = 0; 
        }//for
        printf("[BucketGetReadData] dataU16 = %hu [0x%X]\r\n", dataU16, dataU16); 
        sprintf(value, "%hu", dataU16); //convert the u16 into string
    } else if(registeredFeed[slotIdx]->type == STRING){//If the feed is of type string (UNTESTED!)
            uint8_t i = 0; printf("[BucketGetReadData] dataStr = ");
            while(*cBucketBufTail != '\0'){
                if(cBucketBufTail >= &cbucketBuf[BUCKET_SIZE])//Check if we are pointing to the end of the bucket
                    cBucketBufTail = &cbucketBuf[0];//Wrap around to point to the start of the bucket to continue reading
                printf("0x%x ", *cBucketBufTail);
                value[i++] = *cBucketBufTail;
                *cBucketBufTail++ = 0;
            }//copy data until end of string is reached
            //copy the null terminated character and point to start of the next address
            value[i] = *cBucketBufTail;
            *cBucketBufTail++ = 0;
            printf("\r\n");
    }else{
        printf("[BucketGetReadData], Error, invalid read type Slot[%d]\r\n", slotIdx);
        return(false);
    }
    return(true);
}

/****************************************************************
 * Function Name    : BucketGet
 * Description      :Gets the data
 * Returns          : true on success else negative..
 * Params       @keyOut :contains the key
                @dataOut:contains the value
                @timestampOut: contains the timestamp
 ****************************************************************/
int8_t BucketGet( char *keyOut, char *dataOut, uint32_t *timestampOut) {
    //Check if theres anything in the bucket
    printf("<==============================================================================>\r\n");
    if(BucketCheckDataAvailable() == false){
        printf("[BucketGet], AlLERT, Bucket is empty, no more data left to read.\r\n");
        //cBucketBufHead = &cbucketBuf[0]; cBucketBufTail = &cbucketBuf[0];
        return(-1);
    }
    
    //Read the key-value for the corresponding feed/slot
    if(BucketGetReadData(keyOut, dataOut) == false){ 
        printf("[BucketGet], Error, bucket read failed\r\n"); return(-2); 
    }
    //Read the timestamp corresponding to this feed
    if(BucketGetTimestampForFeed(timestampOut) == false){
        printf("[BucketGet], Error, feed timestamp read failed\r\n"); return(-3); 
    }
    //All good, dump the key-value and associated timestamp
    printf("[Bucket Get] timestamp  = %lu [0x%X]\r\n", *timestampOut, *timestampOut);
    printf("[Bucket Get] key    = %s\r\n", keyOut);
    printf("[Bucket Get] value  = %s\r\n", dataOut);
    printf("<==============================================================================>\r\n");
        
    return(true);
}

/****************************************************************
 * Function Name    : _RegisteredFeedFreeSlot
 * Description      :Returns first free slot(index) found, returns
 -1 if no free slots available
 * Returns          : slot on success else negative..
 * Params       None.
 ****************************************************************/
int8_t _RegisteredFeedFreeSlot(void) {
    int8_t slot;
    //Check for slots sequentially, return index of first empty one (null pointer)
    for(slot = 0; slot<BUCKET_MAX_FEEDS; slot++) {
        if(registeredFeed[slot] == 0) return slot;
    }
    //All slots full
    return -1;
}

/****************************************************************
 * Function Name    : BucketNumbOfRegisteredFeeds
 * Description      : Gets the num of registered feeds
 * Returns          : No. of registered feeds
 * Params       None.
 ****************************************************************/
uint8_t BucketNumbOfRegisteredFeeds(void) {
    uint8_t counts = 0;
    // Check for slots sequentially, return index of first empty one (null pointer)
    for(uint8_t slot = 0; slot<BUCKET_MAX_FEEDS; slot++) {
        if(registeredFeed[slot] == 0)
            slot = BUCKET_MAX_FEEDS;
        else
            counts++;
    }
    // All slots full
    return counts;
}

/****************************************************************
 * Function Name    : _PrintFeedValue
 * Description      :Prints feed data value via void pointer
 and corrects for type FLOAT AND DOUBLE DONT PRINT ON WASPMOTE
 BUT NO REASON TO BELIEVE THEY ARE WRONG
 * Returns          : None.
 * Params       @feed: Points to the feed to be printed
 ****************************************************************/
void _PrintFeedValue(cbucket_t *feed) {
    if (feed->type == UINT16) printf("%d",*(uint16_t*)feed->value);
    else if (feed->type == STRING) printf("%s",(char*)feed->value);
    else printf("%s","UNSUPPORTED TYPE");
}

/*
*====================
* Debug Utils
*====================
*/
/****************************************************************
 * Function Name    : _DebugPrintRegistrationData
 * Description      :Prints all registered feeds and their details
 * Returns          : None.
 * Params       None.
 ****************************************************************/
void DebugPrintRegistrationData(void) {
    int8_t slot;

    printf("********************** Current Bucket Registration Data **************************\r\n");
    printf("slot\taddress\tkey\ttype\tvalue\tunixtime\r\n");
    for(slot = 0; slot<BUCKET_MAX_FEEDS; slot++) {
        printf("%d\t", slot);                                        // Print index
        if (registeredFeed[slot] != NULL) {
            printf("0x%x\t",(uint32_t)&registeredFeed[slot]->key);                 // Print structure address
            printf("%s\t",registeredFeed[slot]->key);                  // Print key
            printf("%s\t",cvaluetypes[registeredFeed[slot]->type]);    // Print type
            _PrintFeedValue(registeredFeed[slot]);                     // Print value
            printf("\t%lu\r\n",registeredFeed[slot]->unixTime);          // Print time
        } else printf("--\t--\tEMPTY\t--\t--\r\n");
    }
}

/****************************************************************
 * Function Name    : _DebugPrintBucket
 * Description      :Prints all bucket memory, even if empty
 * Returns          : None.
 * Params       None.
 ****************************************************************/
void DebugPrintBucket(void) {
    uint16_t readIndex = 0;
    printf("\r\n********************* BUCKET START ********************\r\n");
    while(readIndex < BUCKET_SIZE) {
        printf("0x%04X ", readIndex);
        for (uint8_t column = 0; column < 16; column++) {
            if(readIndex < BUCKET_SIZE) printf("%02X ",cbucketBuf[readIndex]);
            readIndex++;
            //delayMicroseconds(78);  // Wait for a byte to send at 115200 baud
            //delay(0.1);
        }
        printf("\r\n");
    }
    printf("********************** BUCKET END *********************\r\n");
}

/****************************************************************
 * Function Name    : _DebugPrintBucket
 * Description      : Prints bucket memory that has data
 * Returns          : None.
 * Params       None.
 ****************************************************************/
void DebugPrintBucketTailToHead(void) {
    uint8_t *cBucketBufHeadTemp = cBucketBufHead;
    uint8_t *cBucketBufTailTemp = cBucketBufTail;
    uint16_t index;
    printf("\n*************** BUCKET START FROM TAIL ****************\n");
    // Label and indent first line
    if ((cBucketBufTailTemp - &cbucketBuf[0]) % 16 != 0) {
        printf("          ");
        for (index = (cBucketBufTailTemp - &cbucketBuf[0]) % 16; index > 0; index--) {
            printf("   ");
        }
    }
    // Print rest of data
    while(cBucketBufTailTemp != cBucketBufHeadTemp) {                                                                  // Increment read address
        if(cBucketBufTailTemp >= &cbucketBuf[BUCKET_SIZE]) cBucketBufTailTemp = &cbucketBuf[0]; // Handle wraparound
        index = cBucketBufTailTemp - &cbucketBuf[0];                                            // Get current index in buffer
        if (index % 16 == 0) printf("\n0x%04X ", cBucketBufTailTemp - &cbucketBuf[0]);          // New line every 0x00n0
        printf("%02X ", *cBucketBufTailTemp);
        cBucketBufTailTemp++;                                                   // Print data in buffer
    }
    printf("\n***************** BUCKET END AT HEAD ******************\n");
}

Below is the test code.

//<===============================Bucket Test================================>
char sensorValStr1[]    = "aaaaaaaaaaaaaa";
char sensorKey1[]       ="sensorRef1";
cbucket_t sensor1       = {sensorKey1, STRING, sensorValStr1,               1613343689};

uint16_t sensorValU16_2 = 0xAAAA;
char sensorKey2[]       ="sensorRef2";
cbucket_t sensor2       = {sensorKey2, UINT16, (uint8_t*)&sensorValU16_2,   1613343689};

uint16_t sensorValU16_3 = 0xBBBB;
char sensorKey3[]       ="sensorRef3";
cbucket_t sensor3       ={sensorKey3, UINT16, (uint8_t*)&sensorValU16_3,    1613343689};

uint16_t sensorValU16_4 = 0xCCCC;
char sensorKey4[]       ="sensorRef4";
cbucket_t sensor4       ={sensorKey4, UINT16, (uint8_t*)&sensorValU16_4,    1613343691};
    
//<===============================Bucket Test================================>

int main(void) {
    atmel_start_init();                 /* Initialize the drivers */
    
    //<=================Bucket Test===================>
    BucketRegisterFeed(&sensor1);
    BucketRegisterFeed(&sensor2);
    BucketRegisterFeed(&sensor3);
    BucketRegisterFeed(&sensor4);
    DebugPrintRegistrationData();

    char keyOutT[32] = {0};
    char valOutT[32] = {0};
    uint32_t timeStmp = 0;

    for(int i = 0 ; i < 3 ; i++){
        //sensor2.unixTime +=1;
        BucketPut(&sensor2);
    }
    DebugPrintBucket();

    BucketGet(keyOutT, valOutT, &timeStmp);
    DebugPrintBucket();
    BucketGet(keyOutT, valOutT, &timeStmp);
    DebugPrintBucket();
    BucketGet(keyOutT, valOutT, &timeStmp);
    DebugPrintBucket();
    BucketGet(keyOutT, valOutT, &timeStmp);
    
    for(int i = 0 ; i < 8 ; i++){
            BucketPut(&sensor3);
    }
    DebugPrintBucket();
    while(BucketGet(keyOutT, valOutT, &timeStmp) == true);
    DebugPrintBucket();
    for(int i = 0 ; i < 8 ; i++){
            BucketPut(&sensor4);
    }
    DebugPrintBucket();
    while(BucketGet(keyOutT, valOutT, &timeStmp) == true);
    //<===================Bucket Test====================>
    
    while (true) {

    }
}
  

Answer 1

We're missing an include of <stdint.h> in the header, and <stdbool.h> in the implementation.

Some parts of the code are hard to read. In particular, I think this comment should be wrapped at a more conventional line length:

/*
 * Description: This module is used to accumulate data when the network is down or unable to transmit data due to poor signal quality. It currently supports uint16_t and string data types.  The bucket is designed to maximize the amount of data that can be stored in a given amount of memory for typical use. The feeds must be registered before they can be written to or read from the bucket.
 */

We should compile with more warnings enabled. With gcc -std=c17 -Wall -Wextra -Wwrite-strings -Wno-parentheses -Wpedantic -Warray-bounds -Wstrict-prototypes -Wconversion, I get warnings about narrowing conversions:

256360.c:151:23: warning: conversion from ‘int’ to ‘int8_t’ {aka ‘signed char’} may change value [-Wconversion]
  151 |             slotIdx = i; i = BUCKET_MAX_FEEDS;
      |                       ^
256360.c:506:28: warning: conversion from ‘int’ to ‘uint16_t’ {aka ‘short unsigned int’} may change value [-Wconversion]
  506 |                 dataU16 |= (*cBucketBufTail & 0xFF);
      |                            ^
256360.c:508:28: warning: conversion from ‘int’ to ‘uint16_t’ {aka ‘short unsigned int’} may change value [-Wconversion]
  508 |                 dataU16 |= (*cBucketBufTail & 0xFF) << 8;
      |                            ^
256360.c:682:22: warning: conversion from ‘long int’ to ‘uint16_t’ {aka ‘short unsigned int’} may change value [-Wconversion]
  682 |         for (index = (cBucketBufTailTemp - &cbucketBuf[0]) % 16; index > 0; index--) {
      |                      ^
256360.c:689:17: warning: conversion from ‘long int’ to ‘uint16_t’ {aka ‘short unsigned int’} may change value [-Wconversion]
  689 |         index = cBucketBufTailTemp - &cbucketBuf[0];                                            // Get current index in buffer
      |                 ^~~~~~~~~~~~~~~~~~

conversions between signed and unsigned:

256360.c:292:25: warning: conversion to ‘uint8_t’ {aka ‘unsigned char’} from ‘int8_t’ {aka ‘signed char’} may change the sign of the result [-Wsign-conversion]
  292 |     *cBucketBufHead++ = slotIdx;
      |                         ^~~~~~~
256360.c:489:15: warning: conversion to ‘int8_t’ {aka ‘signed char’} from ‘uint8_t’ {aka ‘unsigned char’} may change the sign of the result [-Wsign-conversion]
  489 |     slotIdx = *cBucketBufTail;  //this is an int8_t type, if greater, will lead to undefined behavior.
      |               ^
256360.c:520:30: warning: conversion to ‘char’ from ‘uint8_t’ {aka ‘unsigned char’} may change the sign of the result [-Wsign-conversion]
  520 |                 value[i++] = *cBucketBufTail;
      |                              ^
256360.c:524:24: warning: conversion to ‘char’ from ‘uint8_t’ {aka ‘unsigned char’} may change the sign of the result [-Wsign-conversion]
  524 |             value[i] = *cBucketBufTail;
      |                        ^

comparisons between signed and unsigned:

256360.c:207:27: warning: comparison of integer expressions of different signedness: ‘int’ and ‘long unsigned int’ [-Wsign-compare]
  207 |         for(int i = 0 ; i < sizeof(uint32_t) ; i++, cBucketBufHeadTmp++){
      |                           ^
256360.c:309:22: warning: comparison of integer expressions of different signedness: ‘int’ and ‘long unsigned int’ [-Wsign-compare]
  309 |     for(int i = 0; i < sizeof(data->unixTime); i++, cBucketBufHead++){
      |                      ^
256360.c:420:27: warning: comparison of integer expressions of different signedness: ‘int’ and ‘long unsigned int’ [-Wsign-compare]
  420 |         for(int i = 0 ; i < sizeof(uint32_t) ; i++){
      |                           ^
256360.c:451:36: warning: comparison of integer expressions of different signedness: ‘int’ and ‘long unsigned int’ [-Wsign-compare]
  451 |                 for(int  i = 0 ; i < sizeof(uint32_t) ; i++, cBucketBufTailTmp++){
      |                                    ^
256360.c:502:27: warning: comparison of integer expressions of different signedness: ‘int’ and ‘long unsigned int’ [-Wsign-compare]
  502 |         for(int i = 0 ; i < sizeof(uint16_t) ; i++){//Read 2 bytes
      |                           ^

and use of the wrong conversion specifier:

256360.c:360:57: warning: format ‘%lu’ expects argument of type ‘long unsigned int’, but argument 2 has type ‘uint32_t’ {aka ‘unsigned int’} [-Wformat=]
  360 |             printf("[BucketPut] Last stored timestamp[%lu] = Current feed timestamp[%lu]\r\n", lastStoredTime, data->unixTime);
      |                                                       ~~^                                      ~~~~~~~~~~~~~~
      |                                                         |                                      |
      |                                                         long unsigned int                      uint32_t {aka unsigned int}
256360.c:360:87: warning: format ‘%lu’ expects argument of type ‘long unsigned int’, but argument 3 has type ‘uint32_t’ {aka ‘const unsigned int’} [-Wformat=]
  360 |             printf("[BucketPut] Last stored timestamp[%lu] = Current feed timestamp[%lu]\r\n", lastStoredTime, data->unixTime);
      |                                                                                     ~~^                        ~~~~~~~~~~~~~~
      |                                                                                       |                            |
      |                                                                                       long unsigned int            uint32_t {aka const unsigned int}
256360.c:380:57: warning: format ‘%lu’ expects argument of type ‘long unsigned int’, but argument 2 has type ‘uint32_t’ {aka ‘unsigned int’} [-Wformat=]
  380 |             printf("[BucketPut] Last stored timestamp[%lu] != Current feed timestamp[%lu]\r\n", lastStoredTime, data->unixTime);
      |                                                       ~~^                                       ~~~~~~~~~~~~~~
      |                                                         |                                       |
      |                                                         long unsigned int                       uint32_t {aka unsigned int}

256360.c:380:88: warning: format ‘%lu’ expects argument of type ‘long unsigned int’, but argument 3 has type ‘uint32_t’ {aka ‘const unsigned int’} [-Wformat=]
  380 |             printf("[BucketPut] Last stored timestamp[%lu] != Current feed timestamp[%lu]\r\n", lastStoredTime, data->unixTime);
      |                                                                                      ~~^                        ~~~~~~~~~~~~~~
      |                                                                                        |                            |
      |                                                                                        long unsigned int            uint32_t {aka const unsigned int}
256360.c:560:41: warning: format ‘%lu’ expects argument of type ‘long unsigned int’, but argument 2 has type ‘uint32_t’ {aka ‘unsigned int’} [-Wformat=]
  560 |     printf("[Bucket Get] timestamp  = %lu [0x%X]\r\n", *timestampOut, *timestampOut);
      |                                       ~~^              ~~~~~~~~~~~~~
      |                                         |              |
      |                                         |              uint32_t {aka unsigned int}
      |                                         long unsigned int
256360.c:641:25: warning: format ‘%lu’ expects argument of type ‘long unsigned int’, but argument 2 has type ‘uint32_t’ {aka ‘unsigned int’} [-Wformat=]
  641 |             printf("\t%lu\r\n",registeredFeed[slot]->unixTime);          // Print time
      |                       ~~^      ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      |                         |                          |
      |                         long unsigned int          uint32_t {aka unsigned int}
256360.c:690:45: warning: format ‘%X’ expects argument of type ‘unsigned int’, but argument 2 has type ‘long int’ [-Wformat=]
  690 |         if (index % 16 == 0) printf("\n0x%04X ", cBucketBufTailTemp - &cbucketBuf[0]);          // New line every 0x00n0
      |                                          ~~~^    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      |                                             |                       |
      |                                             unsigned int            long int

These are all easy to fix, and just indicate sloppiness. (Remember that the format string for uint32_t is PRIi32, defined in <inttypes.h>).

---

It's good to see tests included, though I did need to remove the call to undefined function atmel_start_init() to enable it to compile. That didn't seem to be required, so probably shouldn't have been in the test suite.

Tests are better if they are self-checking, rather than relying on visual inspection of output - how often are you likely to look closesly enough to spot a regression?

I would like to see some tests of the limit cases (such as creating BUCKET_MAX_FEEDS feeds, and then creating one more, and checking it's properly handled).

I had to remove the unfinished final test (while (true) {}), as that was preventing the tests completing on my system.

There's a couple of empty loops in the test, of this form:

while(BucketGet(keyOutT, valOutT, &timeStmp) == true);

We should probably be actually checking the values, rather than just discarding, but as a general principle, I like to write the body of the loop (;) on its own line, where it's less easily missed than at the end of line like that.

---

Do the debug strings need to be in release builds? I'd rewrite like this:

#ifndef NDEBUG
const char* cvaluetypes[] = {"UINT16", "STRING"};
#endif

Similarly for all the other debug code.

---

There are a lot of functions in the implementation that are not in the public interface - they all ought to be declared with static linkage, to reduce namespace pollution.

---

Instead of manipulating the loop index to finish a loop:

for(int i = 0 ; i < BUCKET_MAX_FEEDS ; i++) {
    //found it?
    if(data == registeredFeed[i]) {
        //Get the slot index
        slotIdx = i; i = BUCKET_MAX_FEEDS;
    }
}

The conventional, well-understood means is to use a break statement:

for(int i = 0 ; i < BUCKET_MAX_FEEDS ; i++) {
    //found it?
    if(data == registeredFeed[i]) {
        //Get the slot index
        slotIdx = i;
        break;
    }
}

In this case, it's probably best to simply return from the function instead.

---

int8_t BucketCheckDataAvailable(void){
    //Bucket is empty?
     if(cBucketBufTail == cBucketBufHead){
        return(false); 
    }
     return(true);
}

The patter if (c) return true; else return false; is over complex - just return the condition directly (and why are we returning a bool as int8_t?):

bool BucketCheckDataAvailable(void)
{
    // Bucket is not empty?
    return cBucketBufTail != cBucketBufHead;
}

Also, no need for redundant comparisons against false:

if(BucketCheckDataAvailable() == false){
    return(false);
}

becomes simply

if (!BucketCheckDataAvailable()) {
    return false;
}

---

What's going on here?

/****************************************************************
 * Function Name    : BucketGetTimeStamp
 * Params           : None.
 ****************************************************************/
int8_t BucketPutGetTimeStamp(uint32_t *timestamp){

That's one reason not to repeat code information in comments!

---

Here's a loop that has a lot of duplication:

    for(int i = 0 ; i < sizeof(uint32_t) ; i++, cBucketBufHeadTmp++){
        //Now if head points to end of bucket, read that value and wrap to start of bucket
        if(cBucketBufHeadTmp == &cbucketBuf[BUCKET_SIZE]){//Handle if head needs to be wrapped around to read the timestamp
            cBucketBufHeadTmp = &cbucketBuf[0]; //point to start of the bucket to continue reading the timestamp
            if(i == 0){
                *timestamp |= (*cBucketBufHeadTmp & 0xFF);
            }else if(i == 1){
                *timestamp |= (*cBucketBufHeadTmp & 0xFF) << 8;
            }else if(i == 2){
                *timestamp |= (*cBucketBufHeadTmp & 0xFF) << 16;
            }else if(i == 3){
                *timestamp |= (*cBucketBufHeadTmp & 0xFF) << 24;
            }
        }else{//read value as is
            if(i == 0){
                *timestamp |= (*cBucketBufHeadTmp & 0xFF);
            }else if(i == 1){
                *timestamp |= (*cBucketBufHeadTmp & 0xFF) << 8;
            }else if(i == 2){
                *timestamp |= (*cBucketBufHeadTmp & 0xFF) << 16;
            }else if(i == 3){
                *timestamp |= (*cBucketBufHeadTmp & 0xFF) << 24;
            }
        }//else
    }//for

The common code can be moved outside the if/else, and the if (i== chain (which looks like a candidate for switch/case) can be eliminated using simple arithmetic:

    for (int i = 0;  i < sizeof (uint32_t);  ++i, ++cBucketBufHeadTmp) {
        if (cBucketBufHeadTmp == &cbucketBuf[BUCKET_SIZE]) {
            // wrap to start of the bucket to continue reading the timestamp
            cBucketBufHeadTmp = &cbucketBuf[0];
        }
        *timestamp |= *cBucketBufHeadTmp << (8 * i);
    }

I also removed the pointless & 0xff (since *cBucketBufHeadTmp is a uint8_t, it has no effect).

Looking at this more broadly, the code to wrap around is repeated quite a lot, and would be better factored out into a function.

---

BucketGetDataSize() has this loop:

    const uint8_t *bytePtr =  (uint8_t*)data->value;
    for(int i = 0; i  < BUCKET_MAX_VALUE_SIZE; i++, bytePtr++) {
        if(*bytePtr == '\0') {
            i = BUCKET_MAX_VALUE_SIZE;
            //Include the '\0' to be written. This will be used to indicate the end of string while reading
            dataSizeOut++;
            continue;
        }
        dataSizeOut++;
    }

dataSizeOut++; continue; is pointless, as that's exactly what would happen if we omitted those two statements. But the whole loop is pointless, since we should just use strlen() and have no need to write a loop here.

Also, I'd prefer to see the testing of data->type using a switch, to make it easier to maintain (a decent compiler will warn if you miss a case when switching on an enum, for example).

---

/*
 * Function Name    : BucketPut
 * Returns          : true on success else negative..
 */

That's confusing, since negative values are all true in C.

I don't see why this function needs to be so chatty (and why are errors printed to stdout instead of stderr as one would expect?).

---

I only skim-read from here on, but saw many repeats of the same items already mentioned. It's probably time to address those comments, and return for another review when that's done.

Answer 2

Here are some things that may help you improve your code.

Use the required #includes

The code uses uint8_t and uint32_t which means that it should #include <stdint.h>. It also uses true and false so it should #include <stdbool.h>. It was not difficult to infer, but it helps reviewers if the code is complete.

Don't make non-portable size assumptions

The code includes a number of dubious casts like this one:

printf("[BucketGetReadData], Slot[%d] = 0x%x\r\n", slotIdx, (uint32_t)&registeredFeed[slotIdx]->key);

That's dubious because it casts a pointer to the key (which is declared as const char*) into a 32-bit quantity. That will absolutely fail on a 64-bit machine like the one I'm currently using. You might say that you only ever intend to run this code on a 32-bit machine, which is fine, but then make these assuptions explicit somewhere in the code such that the validity of the assumption is checked at compile time and an error is emitted if the assumption is not true. In this case, I'd suggest omitting the cast and instead using the %p format specifier which is an implementation-defined character sequence for a pointer. It is often exactly the kind of hexadecimal representation you seek.

Be careful with signed and unsigned

In a number of cases in the code we have something like this:

for(int i = 0 ; i < sizeof(uint32_t) ; i++, cBucketBufHeadTmp++){

The problem is that the code compares an int i to a size_t from sizeof, but size_t is unsigned and int is signed. Instead, i as a size_t type.

Also, the _RegisteredFeedFreeSlot() routine returns -1 if there is no free slot. This implies that the maximum number of slots is 254 because 255 would be encoded exactly the same way as -1 in an int8_t. Worse, because you're using it as an array index into the registeredFeed array, any number greater than 127 will be interpreted as a negative number and absolutely wreak havoc as that would be undefined behavior in this context. (See this question for more on negative numbers for array indices.) This means that the maximum legal value for BUCKET_MAX_FEEDS is 127. The code comment indicates that the maximum value is 64, but that should be enforced, as with the previous suggestion, and generate a compile-time error message if the value is not in range.

Declare non-interface functions and data static

The interface is apparently completely defined in the Bucket.h file which is good. That's exactly the way it should be. However, all of the extra functions in Bucket.c, such as _RegisteredFeedFreeSlot() should be static so they cannot be used outside of that context. However, see the next suggestion.

Don't use leading underscores in names

Anything with a leading underscore is a reserved name in C (and in C++). See this question for details.

Simplify your code

This is a peculiar construction:

int8_t BucketCheckDataAvailable(void){
    //Bucket is empty?
     if(cBucketBufTail == cBucketBufHead){
            return(false); 
    }
     return(true);
}

In addition to the messy and inconsistent indentation, this is a convoluted way to do this. Instead, you could write this:

return cBucketBufTail != cBucketBufHead;

Note that return is a keyword, not a function, and it does not require and should not have parentheses.

Consider restructuring the code

Right now there is a lot of code within multiple functions that deal with the circular buffer wrapping around. I would isolate that to two helper functions like this:

bool CircularBufferWrite(size_t size, const void *data);
bool CircularBufferRead(size_t size, void *data);

These two would be the only functions that actually write to or read from the circular buffer directly, freeing the other code to be much simpler. The bool return value indicates success or failure.

Use consistent formatting

The code as posted has inconsistent indentation which makes it hard to read and understand. Pick a style and apply it consistently. The good part is that the function comments are generally pretty good. The bad is that some of the lines are almost 200 characters long. This impairs readability, even on large screens. I'd suggest a limit of 80 characters.

Consider isolating printf statements

Since this is on an embedded platform, it's unlikely that the printf statements are going to be particularly useful when the device is deployed. For that reason, I'd suggest providing a debug print that compiles to nothing unless it is a debug build. See assert.h for inspiration on how to do that.

Make the tests work for humans

Right now, an expert needs to examine the test output to determine if the results are as intended or not. Rather than making the human work for the machine, make the tests work for the human: have the program evaluate its own test results and print a simple pass/fail message for each. Unit testing is a widely used and very helpful technique when done well. You're already partway there.