cnetworkingdrivermicrocontrollerdevice-driver

C Message Encapsulation (SPI/nRF24 Driver): Any good practices?

I'm currently developing a radio (nRF24) module driver on top of a homemade SPI driver. The driver is pretty simple: to send a message via the radio module, you need to send the message via SPI, prefixed with a command:

Sending a RF message

I got it working perfectly, but I'm struggling with the encapsulation of the user payload (i.e adding the NRF24_CMD before the user payload).

As of today, I'm allocating a new char array for the SPI payload each time I'm sending a message. I'm writing the radio module command in the first byte of the array, and then copying the user payload as demonstrated here:

void nrf24_send(char * payload, size_t length){
    char spi_payload[MAX_DATA];
    spi_payload[0] = RF_SEND_CMD;
    for(int i=0; i<length; i++){
        spi_payload[i+1] = payload[i];
    }
    spi_write(spi_payload, length + 1);
}

This generates a lot of pointless memory accesses just to copy the user payload and I feel that there is room for improvement.

What would be an efficient way to encapsulate the user message ?

Is there a good practice in the driver development community for these kinds of problems ?

FYI:

Here are a few ideas I had:

  1. using memcpy instead of the for loop -> Better, the copy of the array would be faster, but there's still the need to copy the payload
  2. tell the user to allocate the buffer and left the first byte empty -> No copy needed, but the user needs to know about the hardware, and I don't want that
  3. create a c struct to hide the complexity (cf below) -> I'm afraid that alignments issues may arise, and since my NRF24_CMD byte needs to be directly followed by the payload, I don't know if I can trust my compiler...

Hiding the complexity with a struct:

struct nrf24_message_t {
   char cmd;
   char payload;
}

Thanks for your help!

Hugo

Solution

  • As the command byte and the payload array's underlying data type are all of type char you don't need to care for alignment issues – most important, there won't be any gaps in between if you define one after the other.

    I'd now allow the user to allocate the memory appropriately and provide some kind of conversion function, e.g.:

    // header:
// TODO: include guards, etc
struct nrf24_message;

struct nrf24_message* nrf24_createFromRaw(size_t size, char* raw);

// to allow the user to write to
size_t nrf24_bufferSize(struct nrf24_message* message);
char* nrf24_buffer(struct nrf24_message* message);

void nrf24_send(struct nrf24_message* message, size_t length);
// still a length parameter:
// maybe a message gets shorter than maximum the buffer size!

    This is the visible user interface while the implementation sees the details of this struct:

    //source:
struct nrf24_message
{
    size_t payloadSize;
    char cmd;
    char payload[]; // flexible array member...
};

struct nrf24_message* nrf24_createFromRaw(size_t size, char* raw)
{
    if(size <= sizeof(size_t) + 1) // < if you want to allow empty message
    {
        // failure! buffer is too short
        return NULL;
    }
    struct message* m = (struct message*)raw;
    m->payloadSize = size - sizeof(size_t) - 1;
    return m;
}

size_t nrf24_bufferSize(struct message* nrf24_message)
{
    return message->payloadSize;
}
char* nrf24_buffer(struct nrf24_message* message)
{
    return message->payload;
}
void nrf24_send(struct nrf24_message* message, size_t length)
{
   spi_write(&message->cmd, length + 1);
}

    Et voilà: Now the user can provide memory while you still have space in front of the data.

    Usage example:

    char raw[256];
struct* nrf24_message = nrf24_createFromRaw(sizeof(raw), raw);
size_t length = snprintf(nrf24_buffer(message), nrf24_bufferSize(message), "...");
nrf24_send(message, length);