What is the best way to store serial SPI data into a buffer?

[Kalcifer]

I've been having issues with storing data received over SPI. I was originally storing it into an array but that ended up being a pain to properly deal with in C.
SPI generally sends data separated into segments of 1 Byte, but sometimes the slave will send multiple Bytes per one command so I need to store that data before processing.
I was initially storing each received byte into its own element of an array, but that seems overly complicated. There must be a more elegant solution with a proper input buffer in C.
I was thinking something with <stdio.h> but I'm really not sure.
does anyone have any suggestions?

EDIT: I should add that I was having troubles dealing with the array of data locally in the code. I could probably get it working just fine having a global buffer variable set but I didn't think that was the proper way to go.

[Rick Law]

How about a ring-buffer?

Say allocate a 16 byte string and two pointers - receivePointer and processPointer.

Since it is 16 (2^n), handling the pointers are easy
 receivePointer = (receivePointer+1) & 0x0f; // not using ++ since I am not putting it back until after the & 0x0f

You can figure the same technique for the processPointer.  when processPointer==receivePointer, you got nothing new in the buffer; but of course you still have to take into account situation of having received >=16 bytes since last processed.

[brucehoult]

stdio.h is completely irrelevant.

A "buffer" and an "array" are exactly the same thing, and the correct and only way to do it.

What kind of SPI hardware does your chip have and how are you using it?

Are you manipulating the clock and MOSI and SS and reading MISO by using digital IO pins yourself? Or do you have SPI hardware that does that automatically and puts the result in a buffer? How big? One byte? More? If there is a hardware buffer, how are you deciding when there is data in it?

Your question is far too vague, but in general the short answer is that an array in C is the only way to do thing. You can choose how you allocate that array (global variable, local variable, alloca, malloc) but you need it.

[ejeffrey]

I've been having issues with storing data received over SPI. I was originally storing it into an array but that ended up being a pain to properly deal with in C.
SPI generally sends data separated into segments of 1 Byte, but sometimes the slave will send multiple Bytes per one command so I need to store that data before processing.
I was initially storing each received byte into its own element of an array, but that seems overly complicated. There must be a more elegant solution with a proper input buffer in C.
I was thinking something with <stdio.h> but I'm really not sure.
does anyone have any suggestions?

A char array is the normal way to handle byte sequences in C.  Can you be more explicit about what you are doing and what you don't like about it?

stdio provides stream wrappers around file like objects.  In some cases you might want to wrap a SPI peripheral in a stdio stream but it is not particularly common.  Normally SPI is transaction oriented (delimited by nCS) so a stream wrapper doesn't make a lot of sense.

[ledtester]

Having a real-life example would help to direct this discussion.
 
I have an idea of what you might be referring to, and if I interpret your predicament correctly I see two possible approaches.

The first is to operate SPI "synchronously" -- i.e. you issue calls to explicitly send and receive data. You then deconstruct the data received from the slave device as you read it.

An example of this is the code for 'Example using transactions' on this page:

https://www.pjrc.com/teensy/td_libs_SPI.html

The data received from the "slaveA" device is deconstructed (parsed) with these three calls:

Code: [Select]

  stat = SPI.transfer(0);
  val1 = SPI.transfer(0);
  val2 = SPI.transfer(0);

The library call SPI.transfer() is the basic building block of this kind of synchronous SPI operation. Using it you can construct more complex transfer/parsing functions like transfer a 16-bit or 32-bit word or even transfer a whole data structure.

On the other hand, if you can't read the data from the slave device in a piecemeal fashion (like the transfer is automated using interrupts or DMA), then you can still alleviate the work of parsing of the response by overlaying a struct over the receive buffer.

For instance, if you define a struct like:

Code: [Select]

typedef struct __attribute__((__packed__)) {
    uint8_t stat;
    uint8_t val1;
    uint8_t val2;
} Response_t

and the received response in a byte buffer pointed to by the variable rbuf, then code like:

Code: [Select]

    Response_t* r = (Response_t *) rbuf;

will allow you to access the fields of the response with the expressions:

Code: [Select]

  r->stat
  r->val1
  r->val2

If your compiler is smart enough, and if, say, the value of rbuf is a constant, these accesses could be very efficient.

The tricky part is making sure that your C compiler will define the structure exactly the way the message is laid out in memory, and this is often done by specifying that the structure should be "packed".

[ajb]

The data received from the "slaveA" device is deconstructed (parsed) with these three calls:

Code: [Select]

  stat = SPI.transfer(0);
  val1 = SPI.transfer(0);
  val2 = SPI.transfer(0);

The library call SPI.transfer() is the basic building block of this kind of synchronous SPI operation. Using it you can construct more complex transfer/parsing functions like transfer a 16-bit or 32-bit word or even transfer a whole data structure.

This sort of implies that SPI.transfer() does all of the transaction and blocks while it completes.  You CAN do that, but it's generally much more efficient if you can set up an entire transaction of n bytes and then run it to completion before doing anything with the results.  So build up your outbound message (if necessary) in a local buffer, decide the number of bytes that need to be transacted in each direction, then let DMA or an ISR handle the rest while the MCU goes off and does something else for a bit.  This is an especially good idea when dealing with modern high(er) performance MCUs.  Even aside from the reduced overhead by using DMA, it's silly to have a 200MHz core sitting there watching a flag for hundreds of cycles while your 8MHz SPI bus dawdles along.  Furthermore, it means that the equivalent 'SPI.transfer()' function doesn't need to be tailored to each type or transaction it needs to handle.  You can have just one implementation that takes a void pointer and a size parameter, and maybe a pointer to a result buffer, and just let it rip.

If you have more complex requirements or access patterns you can implement a buffer that can hold multiple transaction descriptors, sort of like how pbufs work in LWIP. 

On the other hand, if you can't read the data from the slave device in a piecemeal fashion (like the transfer is automated using interrupts or DMA), then you can still alleviate the work of parsing of the response by overlaying a struct over the receive buffer.

I would generally caution against this.  It can work okay, especially if it's only ever on one platform and if all of the members are exactly 8 bits, but you have to be cognizant of the way that structs align and pad values on your target platform and the platform endianness to avoid issues.  As an alternative, consider just defining (or enuming) offset values and using those, or if you're dealing with a more complex protocol, you can create simple insert/extract functions or macros.  This strategy is portable across platforms with any alignment/packing strategies or endianness, and is more flexible in how you write the code around it.

In general, programming this sort of thing is a balancing act between writing code that is expressive and simple and using patterns that match what the underlying hardware expects.  Simple, undemanding applications can get away with a lot of abstractions that won't fly in more demanding or complex situations.  The "right" answer will always depend on the application, so without any details from the OP it's really impossible to craft a specific recommendation.  But it's usually best to just use byte (unsigned char) arrays.

[snarkysparky]

Code: [Select]


void SERCOM7_1_Handler( void )   // handler 1 is TX complete. In slave mode this is when SS goes high
{
unsigned int temp;

temp = REG_SERCOM7_SPI_DATA;

REG_SERCOM7_SPI_INTFLAG = SERCOM_SPI_INTFLAG_TXC;  //   clear txc interrupt flag
// ReturnCounter++;
// if(ReturnCounter == 2 )Debug16BitWordOut(temp>>16);
Sercom7RxBuff[Sercom7_RXBuffPtr] = temp;

Sercom7_RXBuffPtr++;

if(Sercom7_RXBuffPtr > SERCOM_7_RX_BUFFLEN - 1)Sercom7_RXBuffPtr = 0;

}


[Kalcifer]

stdio.h is completely irrelevant.

Ah sorry, I was under the impression for some reason that you could use some of the stdio functions to be able to scan the bit stream as one big chunk of data, but I see now as "ejeffrey" pointed out that stdio is more for handling file bit streams not just any serial data.

A "buffer" and an "array" are exactly the same thing, and the correct and only way to do it.

The reason I brought up an array earlier is that the way I have things written is with a transfer() function which transfers how ever many bytes of data and individually stores them into an array. The problems arose when I tried working with the array outside of the function. You can't return the array so I was curious if there was another  sort of "buffer object" that would allow me to deal with this stuff easier.

What kind of SPI hardware does your chip have and how are you using it?

Are you manipulating the clock and MOSI and SS and reading MISO by using digital IO pins yourself? Or do you have SPI hardware that does that automatically and puts the result in a buffer? How big? One byte? More? If there is a hardware buffer, how are you deciding when there is data in it?

I'm currently using an ATTiny84A chip. It uses AVR's Universal Serial Interface (USI) for serial communication. It's SPI (Two/Three wire interface is what they call it, I suppose to avoid legal trouble?) hardware isn't anything to write home about; It's not much better than just bit banging the data, but it gives you an 8 bit shift buffer and dedicated DO/MOSI, DI/MISO, and clock lines. The clock line is a little tedious, you can either control it with a timer, externally, or through software by toggling a specific clock bit, which is what I am doing (one could also just literally toggle it through software and it'd make little to no difference.). The chip doesn't offer a dedicated SS line but it doesn't matter, the GPIO works fine for me. I'm more or less just quickly throwing stuff together for testing purposes, so nothing is fleshed out or proper but at the moment I am just counting how many clock cycles I'm sending and using that to tell when the cycle is finished. For that purpose there is also an internal 4 bit counter inside of the chip that will flag you when the transfer is complete which ill end up using eventually but i didn't bother at the moment.

Your question is far too vague, but in general the short answer is that an array in C is the only way to do thing. You can choose how you allocate that array (global variable, local variable, alloca, malloc) but you need it.

I do apologize for the vagueness, I completely agree that I should have been a bit more thorough/in depth with my question. I suppose the allocation is what I have been having trouble with. It would eat up a decent amount of space if I allocate it globally, and I've always read that it's very bad practice unless absolutely necessary to declare a global variable. Now that you mention malloc() however, that could possibly work pretty well, then I could use free() to deallocate it after I use it. Like a global variable with benefits!

A char array is the normal way to handle byte sequences in C.  Can you be more explicit about what you are doing and what you don't like about it?

Alright, noted! What I was having problems with was dealing with the array and functions. Functions don't seem to play overly well with arrays in C. It was starting to become tedious for me, so I figured that there had to be a way to do things far simpler than the way that I was handling them.

stdio provides stream wrappers around file like objects.  In some cases you might want to wrap a SPI peripheral in a stdio stream but it is not particularly common.  Normally SPI is transaction oriented (delimited by nCS) so a stream wrapper doesn't make a lot of sense.

Ah interesting, I was not aware of that. I was under the impression that the functions in stdio were more versatile; I thought that they could be used for any bit stream. My mistake.
What do you mean by "delimited by nCS"?

[brucehoult]

Your question is far too vague, but in general the short answer is that an array in C is the only way to do thing. You can choose how you allocate that array (global variable, local variable, alloca, malloc) but you need it.

I do apologize for the vagueness, I completely agree that I should have been a bit more thorough/in depth with my question. I suppose the allocation is what I have been having trouble with. It would eat up a decent amount of space if I allocate it globally, and I've always read that it's very bad practice unless absolutely necessary to declare a global variable. Now that you mention malloc() however, that could possibly work pretty well, then I could use free() to deallocate it after I use it. Like a global variable with benefits!

If you know the size of buffer needed (or at least a reasonable maximum for it) then the usual thing is to allocate the array as a local variable in the calling function (that is, on the stack), and then pass a pointer to it into the SPI function to read/write.

If you don't know the size when you write the program, but do know the size at the moment you're about to call the SPI function then alloca() works and is generally better than malloc() and free() because it allocates the variable sized array on the stack and then frees is when the function returns.

Malloc() and free() are generally a very bad idea on something with as little RAM as the 512 bytes on an ATTiny84A (or ATTiny85 that I have used a lot). It's far too easy in a long-running program for repeated malloc() and free() to cause fragmentation so that after a while there is no one chunk of free space big enough for a malloc() even though there is enough free space in total.

It's ok to malloc() something early in the program and then use it forever, as a pseudo-global.

If you want to use dynamic allocation on a very small system then it's better to use a "handle" library instead of a "malloc" library. But I don't know of any. I've been thinking of writing one for Arduino/AVR/FE-310, but have never get a round tuit.

A "handle" library is one where the library knows where all the pointers to memory objects are, and can copy the objects to different places in memory and update the pointers, in order to collect all the free space into one place.

Microsoft BASIC on 8 bit micros did this for string variables -- or at least AppleSoft on the Apple ][ did. (sadly it used an N^3 (I think) algorithm to do the compaction -- one of the first machine code things I ever wrote was a linear-time string heap compactor you could call from AppleSoft periodically before the built in one got triggered)

The original MacOS also used a handle-based heap extensively to allow it to run sophisticate GUI software with something like a 40 KB heap size.

[Doctorandus_P]

TL;DR
My first go to would be a circular buffer.

Maybe add a few little tricks if your data comes in "packets".
For simplicity: assume a buffer size of 100 bytes, and a packet length of upto 7 bytes.

* Then allocate memory of 100+7 bytes in total.
* Restart at the beginning only at the start of a new packet.


============
Alternative (but similar) method:
* Then allocate memory of 100+7 bytes in total.
* When a packet is read from the buffer, and the index of last byte of the packet was >= 100, then move the remaining bytes to the start of the buffer.


The idea is that you can always access the "packet" itself as a continuous array / list / string.

[ledtester]

The reason I brought up an array earlier is that the way I have things written is with a transfer() function which transfers how ever many bytes of data and individually stores them into an array. The problems arose when I tried working with the array outside of the function.
...
I'm currently using an ATTiny84A chip.
...

Given the limited resources of that chip you should store the received bytes in a global array.

The other standard way of "returning" an array in C in to have the caller specify a pointer to the array for the results.

Code: [Select]


int do_spi_stuff(uint8_t *buffer) {
  // do your spi stuff and put the bytes in buffer, e.g.:
  for (uint8_t i = 0; i < 10; ++i) {
    buffer[i] = SPI.transfer(0);
  }
  return 1; // can also return an error condition here
}

uint8_t buf1[10]; // two different SPI buffers
uint8_t buf2[10];

int main() {
  ...
  do_spi_stuff(buf1);
  // process buf1
  ...
  do_spi_stuff(buf2);
  // process buf2
  ...
}

Some more examples of having the caller pass pointers for the return of results from a C function:

https://codeforwin.org/2017/12/return-multiple-value-from-function-c-programming.html

[rstofer]

The circular buffer itself does not need to be global as long as you expose 3 functions:

void queueInit() -- initialize queue pointers on startup
void enqueue(unsiigned char byte) -- this  grabs a byte from the SPI gadget and stuffs it in the queue.   It also manages a digital signal to stop the sender if the queue gets full (flow control).  Overflow would be bad...
unsigned char dequeue(void) -- get the next byte from the queue, returns 0 on queue empty.  In my application, 0 is not a valid data value.  There must be some error handling strategy and this is mine.

In my application, the SPI gadget generates an interrupt for every received byte and the interrupt handler calls the enqueue() function.

Within the source file for these functions is the queue declaration:

static unsigned char queue[QSIZE] and, since I am using one of the LPC1768 buffers, it is 16k bytes.

'static' keeps it local to the file.

All of this stuff, and just this stuff, is in the file queue.c.  The three functions are exposed in queue.h.