[NOTES] Re-imagining the HAL API

[Giovanni]

Hi,

Just few notes for posterity. Currently I am working on a SW project requiring something like the HAL and decided to explore new directions, I must say that I am liking the result.

Basically it is about making the various drivers API more consistent and convenient to use (not that it is hard to use right now...). The driver model is based on the following common API.

Code: Select all

msg_txxxStart(drv, drvconfig)
void xxxStop(drv)
void xxxStartOperation(drv, opconfig)
void xxxStopOperation(drv)
msg_t xxxSynchronize(drvm timeout)


Start and stop are similar to the current ones except start may fail. The different thing is:

xxxStartOperation()

It activates the driver to asynchronously perform the operation encoded in "opconfig" structure which is something similar to the current ADC conversion group structure.
This structure contains all the required information for the operation to be performed such as:
- pointers to buffers
- size of transfers
- callback pointers (no more in the driver config, it is per-operation)
- flags like circular mode false/true
- low level settings like ADC channels, if applicable
- etc

The really new one is xxxSynchronize() which waits for the ongong operation to terminate (by itself or using xxxStopOperation()) and returns an operation status. Another thing I did is to make the driver configuration structure also contain DMA/IRQ settings and do not assume those are defined into an mcuconf.h file, this reduces the driver dependencies on a lot of things.

Another change is to de-focus on API classes for simplicity, xxxStartOperation() and xxxStopOperation() would work in any context (X-class always), of course xxxSynchronize() would be limited to threads.

Most I/O drivers could be reduced to this kind of API, the rework would be huge and break a lot of things so currently I am just writing this as note, food for thought.

Giovanni

[RoccoMarco]

Seems good,
the implementation of synchronous function would be as easy as the redefinition of the sequence

xxxStartOperation()
xxxSynchronize()

Could you bring an example of a failing xxxStart?

Also, would the xxxStart's configuration lose all those configurations that are actually related to the operation itself? For example considering the SPI driver as example, the BR, CPOL, CPHA, DS would be part of the drvconfig or part of the opconfig?

[Giovanni]

For example failing to initialize a peripheral or failing to allocate a DMA.

Giovanni

[FXCoder]

Some questions...
1. Would drivers and structures associated (xxxDn structures) remain statically declared or could they be dynamic so they could be instantiated/disposed on the fly?
2. The return from xxxStart(...) would be generalized or specific?
i.e. would msg_t get for example a MSG_ERROR return covering all failures or implement an extend (system defined) error return to give more detail?
3. I'll think on it some more but agree the rework would be huge!
--
Bob

[steved]

Definitely an improvement on the current HAL (which is, as you say, not difficult to use). A few thoughts:

1. (Much as Bob's point 2) Extend the range of msg_t values to give more feedback as to what has gone wrong, and allow the caller scope to recover appropriately. For example, a memory card driver might return:

Code: Select all

Card not inserted
      Read error
      Write error
      [Other errors]

The first three are user-recoverable in that, for a unit with a user interface, the user can be prompted to sort out the memory card.
A wider range of values won't add overhead, since msg_t is a 32-bit value anyway (assuming ARM platforms).
The return code could be conceptually split into "error classes" defined by, say, the top 16 bits, with more specific information in the lower 16 bits.
As with C, bit 31 set could indicate an error; bit 31 clear is a return value.

2. xxxStartOperation() should return an error code - since it could potentially fail. (Obviously not all conditions would be picked up, but certain checks could be done before returning - parameter value consistency, memory card inserted or not, and so on).

3. Is there merit in adding a 'timeout' parameter to xxxStartOperation()? Then zero is the asynchronous call which returns immediately; other values do the xxxSynchronize() call once the operation is started.

4. Could xxxStopOperation() return useful information?

5. Integrating a consistent method of managing a contention-resolution mutex would be useful; at present support depends on the driver. My initial thought is that the mutex would have to be defined in the drvconfig structure. This would add two or three calls:

Code: Select all

void xxxAcquire(drvconfig)
msg_t xxxAcquireTimeout(drvconfig, timeout)
void xxxRelease(drvconfig)

If implemented, this would probably need to be enabled in halconf.h for each peripheral individually



I also agree the rework would be huge, but could possibly be done incrementally (show us how it should be done, and maybe others will pick up specific drivers!).

[Giovanni]

I will answers in details later, good points there.

I just thought that this thing could we written in (very simplified) C++ and become an experiment, few drivers for just one platform, the G4 for example, the problem would be that not all compilers support C++.

It would be nice to be able to have common LLDs between HAL and new HAL, this would require some rework at LLD interface level but would then allow to maintain both.

Giovanni

[Giovanni]

Some questions...
1. Would drivers and structures associated (xxxDn structures) remain statically declared or could they be dynamic so they could be instantiated/disposed on the fly?
2. The return from xxxStart(...) would be generalized or specific?
i.e. would msg_t get for example a MSG_ERROR return covering all failures or implement an extend (system defined) error return to give more detail?
3. I'll think on it some more but agree the rework would be huge!
--
Bob

1) It could be an idea, no static objects and pass the peripheral pointer in configuration (and clock enablers/disablers), more complex to use however.

2) msg_t can define extra error codes specific for the driver, this is already done in other places.

3) Sure.

[Giovanni]

Definitely an improvement on the current HAL (which is, as you say, not difficult to use). A few thoughts:

1. (Much as Bob's point 2) Extend the range of msg_t values to give more feedback as to what has gone wrong, and allow the caller scope to recover appropriately. For example, a memory card driver might return:

Code: Select all

Card not inserted
      Read error
      Write error
      [Other errors]

The first three are user-recoverable in that, for a unit with a user interface, the user can be prompted to sort out the memory card.
A wider range of values won't add overhead, since msg_t is a 32-bit value anyway (assuming ARM platforms).
The return code could be conceptually split into "error classes" defined by, say, the top 16 bits, with more specific information in the lower 16 bits.
As with C, bit 31 set could indicate an error; bit 31 clear is a return value.

2. xxxStartOperation() should return an error code - since it could potentially fail. (Obviously not all conditions would be picked up, but certain checks could be done before returning - parameter value consistency, memory card inserted or not, and so on).

3. Is there merit in adding a 'timeout' parameter to xxxStartOperation()? Then zero is the asynchronous call which returns immediately; other values do the xxxSynchronize() call once the operation is started.

4. Could xxxStopOperation() return useful information?

5. Integrating a consistent method of managing a contention-resolution mutex would be useful; at present support depends on the driver. My initial thought is that the mutex would have to be defined in the drvconfig structure. This would add two or three calls:

Code: Select all

void xxxAcquire(drvconfig)
msg_t xxxAcquireTimeout(drvconfig, timeout)
void xxxRelease(drvconfig)

If implemented, this would probably need to be enabled in halconf.h for each peripheral individually



I also agree the rework would be huge, but could possibly be done incrementally (show us how it should be done, and maybe others will pick up specific drivers!).

1) It can be extended with more error codes.

2) It is a "start" operation, errors should happen during operation. I would not make checks at runtime about parameters etc, code size would go up for no added value.

3) I prefer to keep it simple and do not pass extra parameters, this API would have 2 parameters max, which is very efficient on ARM.

4) It could, the UART driver would require that if you remember its API.

5) The config structure would be const, this is why it is in the driver currently, just a note, mutexes do not support timeouts, there is a reason for that (priority inheritance complexity).

Giovanni

[stertingen]

I will answers in details later, good points there.

I just thought that this thing could we written in (very simplified) C++ and become an experiment, few drivers for just one platform, the G4 for example, the problem would be that not all compilers support C++.

It would be nice to be able to have common LLDs between HAL and new HAL, this would require some rework at LLD interface level but would then allow to maintain both.

Giovanni

I'd love to see a HAL in C++. Just for curiosity: Which compilers would be incompatible to C++?

[Giovanni]

There are compilers that are not even C99 compatible, proprietary compilers for small, non-ARM, devices mainly.

Giovanni