Cross platform HAL

[jlecoeur]

One big milestone for MAVRIC will be the moment the hal will be truly platform independent (ie able to run on avr32, linux and stm32).

Proposed folder organization

First I think we should separate the chip drivers, which are not platform specific, from the peripheral drivers, which is the real hal.

+
├── hal
│   ├── common
│   │   ├── [...]
│   │   ├── High level peripheral interface (.h files), ex: uart.h, spi.h...
│   │   └── Common features based on lower level peripherals (.h and .c files), ex: SD over SPI
│   ├── avr32
│   │   ├── [...]
│   │   └──Low level peripheral implementation for avr32 (.h and .c files)
│   ├── stm32
│   │   ├── [...]
│   │   └──Low level peripheral implementation for stm32 (.h and .c files)
│   └── linux
│       ├── [...]
│       └── Low level peripheral implementation for linux (.h and .c files)
└──drivers
   ├── [...]
   ├── Chip drivers, using the hal interfaces from hal/common (.h and .c files)
   └── example: bmp085.c uses hal/common/i2c.h (but not hal/avr32/i2c.h)

Then I see two possibilities to link the low-level and high level peripheral files

First solution

Using function pointers. A high level interface is created in boardsupport.h, and initialized using a low-level function in boardsupport.c

High level

hal/common/uart.h:

typedef uart_t
{
   uint8_t (*write)(uint8_t);
   uint8_t (*read)(uint8_t);
}

Low level

hal/avr32/avr32_uart.c:

avr32_uart_init(uart_t* uart, avr32_uart_conf_t config)
{
    // interface functions
    uart->write = &avr32_uart_write;
    uart->read = &avr32_uart_write;
}

Second solution

Depending on the files which are compiled, a low level implementation is chosen

High level

hal/common/uart.c:

uint8_t uart_write(uint8_t byte)
{
    lld_uart_write(byte);   // lld for low level driver
}

uint8_t uart_read(uint8_t byte)
{
    lld_uart_read(byte);   // lld for low level driver
}

Low level

hal/avr32/lld_uart.c:

uint8_t lld_uart_write(uint8_t byte)
{
    avr32_uart_write(byte);
}

uint8_t lld_uart_read(uint8_t byte)
{
    avr32_uart_read(byte);
}

The first one seems easier to handle. The good point is that you do not need to add code in the LIbrary to add a new uart, just instantiate and initialize a new object in the project folder. However, we have to ensure that all pointers are initialized by the low level drivers. The compiler cannot help here, if a feature is not implemented on a specific hardware, then it is possible to have a NULL pointer for the corresponding function.

The second solution is safer in the sense that it will not compile if the low level driver does not implement one of the functions. However by looking at the code, it is impossible to tell which function is called: there will be several implementations with the same function name. Also, it needs a way to choose the uart: either separate function (uart1_read(), uart2_read(), ...), or a parameter uart_read(uint8_t uart_id, uint8_t byte), in both case more code is needed for more peripheral.

Peripheral initialization

In both cases, it is not clear where to put the initialization.

• in the high level files
  • clean and common interface
  • more work
• in the low level drivers
  • faster
  • easier to extend
  • messier

@lis-epfl/mavric-lis What do you think?

[fschill]

Sounds good! Some suggestions from what we've been doing for our AUV where we have plenty of different boards with different purposes (all AVR32 though):

• in hal we have hal/boards and hal/drivers. hal/boards has the boardsupport files (auv_powerboard, auv_navigationboard, ...). Each project just includes one of them and calls the initialisation.
• all the configuration for low-level drivers is now in the boardsupport .h file. This is done by a static inline function that returns the config struct that is then passed to the driver initialisation in the board initialise function. It's a clean method, and doesn't have dependencies to config files not in the library. E.g.:

static inline usart_config_t backplaneboard_debug_uart1_conf(void) {
    usart_config_t usart_conf =
            {   .mode=UART_IN_OUT,
        .uart_device.uart=(avr32_usart_t*)&AVR32_USART1,
        .uart_device.IRQ=AVR32_USART1_IRQ,
[...]
    };
    return usart_conf;
};

Init functions expect a pointer to the "data" object that will be passed in every call, and a pass-by-value of the "xxx_config" returned by the conf method, which it then copies into its data object or uses for initialisation. The only dependency is now to central_data which is board specific - but it's also project specific so it's not a big issue. Maybe all board-related parts of central_data could be packed into a sub_struct.

• I think it should be doable to have a boardsupport as described above for different processors (it works fine for the hal_emu on linux - the only difference I have now in emulation is the hal-emu instead of hal folder and a different main file which doesn't do much).
• a lot of drivers can be abstracted away with the stream interface - I have now drivers for uart, spi and can, as well as UDP in linux emulation (also simulating internal CAN bus via UDP with multiple binaries), using bytestream_t interfaces which works well (added some more functions for starting transmission, etc. - will try to share some time soon). Although function pointers can be dangerous I think in C it's the only way to achieve true abstraction and interchangeability.
• I created and "error_handler" module which can be configured to send errors and debug messages either via plain-text or via Mavlink status. Will try to post as soon as I have time after the field trip. Also added a basic printf to print_utils which is very handy.

[jlecoeur]

Thanks for the input, indeed moving boardsupports to the Library is the next step. I hope the field tests are going well!

Similar to the first solution, what do you guys think of this? https://gist.github.com/jlecoeur/e32fa5e47e464a26239f#file-test_uart-c-L123 This is basically polymorphism in C, and it enforces the initialization of function pointers