Low level C driver API

Question

Recently I had my first experience writing a wrapper around FTD2XX driver for a desktop tool that communicates with an embedded device. The library provided some common device specific functionality and encapsulated low-level MPSEE and board specific configurations from the user.

While writing this library I found myself choosing between two types of APIs that the library provides

1. Function X takes in specific arguments that it needs and nothing more like the following

   eSTATUS spi_init(FT_HANDLE ftdi_handle, eFREQUENCY clock_freq)

2. Function X takes in a control block structure that encapsulates some specific state of the device (previously configured or otherwise)

   eSTATUS spi_init(sDEVICE_CBLK cblk, eFREQUENCY clock_freq) // where sDEVICE_CBLK is a struct containing a handle as well as other device-related fields

The function does the same thing in both cases, but API looks fairly different.

Looking around at various examples I see both approaches as well as mixed ones. My thinking is that approach 1 is more self documenting and "lightweight", in that you only supply what is needed, but approach 2 is a lot easier to refactor in the future (in case of adding a field to a struct) but then it somehow seems less "aesthetically pleasing" to me, in that it's nowhere near as self documenting

What I found is that some functions may get away with approach 1, while others require device state to operate correctly (such as GPIO directions), but then I end up with heterogeneous API which I would like to avoid if possible.

What I strive for is an API that is intuitive and easy to use and easy to refactor if needed. ABI backwards compatibility isn't a requirement in my case

It seems to me that approach #2 is far more practical and clean - nearly every function will have the same API, refactoring becomes easier and it makes sense that such library provides a control block structure for user to initialize and manage through provided API.

Is there a preferred approach in such situations? I understand this is very open-ended so I would appreciate any thoughts on the matter

Answer 1

Approach 1 is simpler. Approach 2 adds complexity for the sole purpose of, maybe, making it easier to change something in the future. This is questionable because:

1. Most software gets written and then never gets significantly updated. Its a waste of time to add extra code and complexity that may never be used.

2. Even if the software will gets revised in the future, it could be in an unanticipated direction that cannot use the current approach 2. Maybe multiple FTD2XX chips will be added. Perhaps a system power management function is added which then requires an extra system power struct parameter.

3. New revisions of a product often use new chips, sometimes from different vendors. New chips will likely have a different Api. Approach 2 will also fail here.

Keep it simple is a good guideline.

Answer 2

If the driver functions need the instance structure DEVICE_CBLK, but it is not specified in spi_init as an argument, or as a return value, that means it is a global variable somewhere inside the driver. This means that the driver can only support a single instance of the device. It might be taking multiple FT_HANDLE arguments, but that may not be usable for talking to multiple devices, if they need separate state information in the DEVICE_CBLK.

Either of these allows the maximum flexibility in multiple instantiation:

// allocate and return a new DEVICE_CBLK, used for all other calls
DEVICE_CBLK *spi_new(FT_HANDLE ftdi_handle, FREQUENCY clock_freq,
                     STATUS *pStatus);

// alternative: initialize client-allocated control block
STATUS spi_init(DEVICE_CBLK *cblk, FREQUENCY clock_freq);

spi_new pairs with spi_destroy; spi_init pairs with spi_cleanup.

Supporting multiple instantiation via an instance structure that is passed around is very easy to achieve, with only a small amount of start-up effort which starts paying for itself almost immediately.

Suppose you are sure that you don't need it though. You can still do this:

// in header file typedef struct DEVICE_CBLK DEVICE_CBLK;

In the implementation:

DEVICE_CBLK *spi_new(FT_HANDLE ftdi_handle, FREQUENCY clock_freq,
                    STATUS *pStatus)
{
  spi_clock_freq = clock_freq;
  if (spi_open) {
    *pStatus = ERROR_DEVICE_BUSY;
    return NULL;
  }
  spi_open = 1;
  return (DEVICE_CBLK *) ftdi_handle;
}

void spi_destroy(DEVICE_CBLK *cblk)
{
  // nothing to free here; FT_HANDLE is not our responsibility
  if (spi_open) {
    // cleanup
    spi_open = 0;
  }
}

In other words, we lied; there is no device block. We didn't allocate anything in spi_new; we just converted the FT_HANDLE pointer to DEVICE_CBLK *. All the other constructor parameters went into global variables.

Our functions still take a DEVICE_CBLK *, but convert it to a FT_HANDLE for calling the lower level SPI routines that need a handle.

But we have enough of an abstraction to take place to go to a real DEVICE_CBLK in the future without changing the client code, or even recompiling it.