Driver design rules in RIOT

Hi,

I wrote a series of sensor drivers for esp-open-rtos/ESP-IDF, for example for different ST sensors, the BME680, the CCS811 or the SHT3x. I would like to port some of these drivers to RIOT OS. All the drivers are quite complex as they try to cover all the features of the sensors.

I have seen that most drivers in RIOT for similar sensors usually only implement an interface to with init, read, power-up and power-down. So I'm wondering now, what the design rules for sensor drivers in RIOT are?

- Should a driver be as complete as possible, which of cource produces more code, or should it be kept simple to produce small code? One option would be to use the pseudomodule approach to enable additional features. On some platforms unused code is not linked into the binary.

- Should a driver support at least data-ready interrupts (if possible at all) to realize event-driven data retrieval?

- Should a driver always return normalized/converted data, or rather return the raw data and the application needs to convert them? The conversion is sometimes quite complex. I saw both approaches of them for similar sensors.

The design rules that are clear to me are:

- Drivers have to provide an interface for polling with init and read that is compatible with SAUL.

- Output are always 16 bit integers.

What else?

Regards Gunar

Hi everyone,

for documentation purposes a quote from a private mail that I wrote Gunar earlier today (at that point I was not aware of this email…):

Maybe one think that could (or even should?!) be considered: try to make very specialized features optional (e.g. via "sub-moduling" them). With this I mean thinks like e.g. event detection, interrupt mode vs. polling etc.). I know choosing those features is definitely not a black&white kind of task, but when keeping this on a coarse level I think it should be rather straight forward. The goal with this would be to allow people to use a simple (e.g. polling) driver configuration (-> low ROM/RAM usage), but also allow developers to chose the full fledged feature version of the driver, trading ROM/RAM usage against features.

As said, don't aim for a fine grained configuration (e.g. submodules for every small feature), but rather go with something like `USEMODULE += l3gd20h` for the simple polling configuration, and something like `USEMODULE += l3gd20h_full` for all the additional features.

To map this reply to the actual questions see inline comments below.

As always, the usage of a driver depends heavily on the application use case and can differ in many ways. Sometimes a simple but small driver is preferable (get me any simple temperature sensor, but I don’t have RAM/ROM to spare), in other cases some specialized features of a device are even the key driver for selecting that specific device. In the 2nd case, it is surely of the essence to have a driver that supports all those little specials. So as stated above: IMO having a coarse modularity inside the driver should be the way to go, this way we can cover both sides. If the driver comes with a ‘full/extra’ configuration, this is part of it anyway, right? In the simples ‘basic’ configuration I don’t think this needs to be part of I would say. My opinion is quite clear: I am always in favor of returning normalized/converted data. In 90% of the cases the conversion is not expensive, so just do it. In those rare cases, where the conversions is actually relatively expensive, we can always fall back by providing additional xx_read_raw() or similar functions, that allow to access the data before conversion. that is a nice to have, but not a MUST. Not quite true. The SAUL interface is build around 16-bit integers, and at least when reading/writing data via SAUL the data needs to be converted. But the driver specific interface can always use other data types/lengths. If it is however easily possible to use int16, one should use it. All this information should go into the ‘device driver guide’ (). This guide needs however still work - and I will not have the time to do it. So it would be nice if other people can help here :slight_smile: Cheers, Hauke

Hi Hauke,

many thanks for your comprehensive and clearifying answers. Most of them met my thoughts about driver design.

- Should a driver support at least data-ready interrupts (if possible at all) to realize event-driven data retrieval?

If the driver comes with a 'full/extra' configuration, this is part of it anyway, right? In the simples 'basic' configuration I don't think this needs to be part of I would say.

Agreed.

- Should a driver always return normalized/converted data, or rather return the raw data and the application needs to convert them? The conversion is sometimes quite complex. I saw both approaches of them for similar sensors.

My opinion is quite clear: I am always in favor of returning normalized/converted data. In 90% of the cases the conversion is not expensive, so just do it. In those rare cases, where the conversions is actually relatively expensive, we can always fall back by providing additional `xx_read_raw()` or similar functions, that allow to access the data before conversion.

Agreed. All the drivers I wrote til now, either return normalized/converted and raw data with same read function or offer an additional read_raw function.

The design rules that are clear to me are:

- Drivers have to provide an interface for polling with init and read that is compatible with SAUL.

that is a nice to have, but not a MUST.

Ok.

- Output are always 16 bit integers.

Not quite true. The SAUL interface is build around 16-bit integers, and at least when reading/writing data via SAUL the data needs to be converted. But the driver specific interface can always use other data types/lengths. If it is however easily possible to use int16, one should use it.

Ok.

What else?

All this information should go into the 'device driver guide' (https://github.com/RIOT-OS/RIOT/wiki/Guide:-Writing-a-device-driver-in-RIOT). This guide needs however still work - and I will not have the time to do it. So it would be nice if other people can help here :slight_smile:

Agreed.

Regards Gunar

Hi there,

based on this exchange, is there matter for a wiki page on this? (Or for alternative documentation, e.g. reviving the concept of RDM [1] ?)

Best

Emmanuel

[1] https://github.com/RIOT-OS/RIOT/pull/6191

Hi Gunar,

I'm not very experienced on the driver development side, but enough as a user to see some issues.

- Should a driver be as complete as possible, which of cource produces more code, or should it be kept simple to produce small code? One option would be to use the pseudomodule approach to enable additional features.

Part of keeping it small is omitting conversion code (see answer below).

How often does it happen that one runs out of flash space? I'm asking because I honestly don't know. I do know that it's probably easier for the user to remove stuff if he runs out of flash than to read the device manual and add the missing functions if the driver is incomplete.

On some platforms unused code is not linked into the binary.

Unused functions, where the linker can determine the function is not used. If you have a big function for configuring device modes, but you never call it with certain parameters and a bit chunk goes unused, it may not be optimized away (I'm not sure if LTO changes this).

- Should a driver support at least data-ready interrupts (if possible at all) to realize event-driven data retrieval?

Yes. Totally yes. Polling is dumb:

* Goes against low power goals. * The data is not polled with a clock that is synchronized with the sensor clock (if the sensor has an ADC), meaning unpredictable jitter.

- Should a driver always return normalized/converted data, or rather return the raw data and the application needs to convert them? The conversion is sometimes quite complex. I saw both approaches of them for similar sensors.

RAW data.

* Conversion usually results in loss of precision, especially if one limits the word length to something like 16 bits (see answer below). * Doing conversion "right" (in an unbiased way) is non trivial. You cannot just go around truncating digits. * Is is beyond the scope of the driver, which should handle device communication/configurations only. * If the converted value is not needed, the conversion cannot be undone. * In SAUL, conversion to and from the base-10 floating point format used is really painful.

I think the measurement should be raw, and there should be a way to query the conversion constant. This way the user can choose, and there are not unnecessary computations done.

In control applications, for example, the conversion is totally not necessary, as the conversion constants can be folded into the control system constants.

The design rules that are clear to me are:

- Drivers have to provide an interface for polling with init and read that is compatible with SAUL.

Yes. It makes all interfaces consistent. That being said, it is sad that there is no unified way for configuring and for interrupt driven measurements.

- Output are always 16 bit integers.

I think it is a bad idea to limit output to 16 bits. ADCs meant for scales, for example, usually have 24 bit [1]. Other applications also demand higher that 16 bits. Keep in mind that 16 bits is equivalent to 4,8 decimal digits, take 1 bit for the sign and you are left with 4,5.

[1] http://www.analog.com/en/products/ad7799.html

What else?

Maybe off topic, but I think we need a IO layer (think SAUL, but more complete) so that the user does not have to directly interact with drivers. I would answer many of your questions, as in that case there would be a well defined interface that device drivers would have to expose. It is an OS, after all.

Regards,

Juan I Carrano.

Hi,

long done :slight_smile:

See Cheers, Hauke

How about adding some information about how to handle multiple threads, when to use mutexes, and how to deal with interrupts? :slight_smile: And especially patterns for being nice from other threads and power consumption point of view…

–Pekka

Cool, that's exactly what I was looking for.

Unfortunaly, it seem not to be reachable from the top level wiki page or I missed it. The only way to find is to use a search engine.

Hi Pekka,

that would indeed be something very desirable. Would you mind maybe just dumping this as TODO at the end of the wiki page? Then this is ‘documented’ and is not forgotten in the depth of the devel list :slight_smile:

Thanks and cheers, Hauke

Cool, that's exactly what I was looking for.

Unfortunaly, it seem not to be reachable from the top level wiki page or I missed it. The only way to find is to use a search engine.

Maybe it could be moved in a specific section of the doxygen documentation.

Hej,

Hi Gunar,

I'm not very experienced on the driver development side, but enough as a user to see some issues.

- Should a driver be as complete as possible, which of cource produces more code, or should it be kept simple to produce small code? One option would be to use the pseudomodule approach to enable additional features.

Part of keeping it small is omitting conversion code (see answer below).

How often does it happen that one runs out of flash space? I'm asking because I honestly don't know. I do know that it's probably easier for the user to remove stuff if he runs out of flash than to read the device manual and add the missing functions if the driver is incomplete.

We have to differentiate two dimensions here: 1. code size, as in "How often does it happen that one runs out of flash space?" -> IMHO we should not even be asking this question, but always expect that people do. Having a low ROM fingerprint is one of the most important differentiators of RIOT! 2. feature richness (or poorness): in an ideal world, all drivers would support everything. But as manpower is limited, there is a good reason for having only basic implementations for many drivers, as we can not force contributors to only provide drivers that are feature-complete... So I think the current approach: merged 'baseline' drivers and 'complete' them by demand works fine.

On some platforms unused code is not linked into the binary.

Unused functions, where the linker can determine the function is not used. If you have a big function for configuring device modes, but you never call it with certain parameters and a bit chunk goes unused, it may not be optimized away (I'm not sure if LTO changes this).

yes, thats why my preferred method would be to use submodules for certain things. But as always, use common sense: having e.g. a read_raw() and a read_converted() function that uses the former, than there is no need to 'sub-module' the read_converted(), as it will only be compiled in in case it is used. But for other features, there are often blocks of code in functions that are used in any case (e.g. initialization), which are easily made configurable using submodules.

- Should a driver support at least data-ready interrupts (if possible at all) to realize event-driven data retrieval?

Yes. Totally yes. Polling is dumb:

I'd say 'dumb' depends very much on the use case. In general I agree that IRQ based approaches definitely are to be preferred, but there are also very valid use cases where polling is be desirable... So not having interrupts in an initial driver is no blocker!

* Goes against low power goals. * The data is not polled with a clock that is synchronized with the sensor clock (if the sensor has an ADC), meaning unpredictable jitter.

- Should a driver always return normalized/converted data, or rather return the raw data and the application needs to convert them? The conversion is sometimes quite complex. I saw both approaches of them for similar sensors.

RAW data.

* Conversion usually results in loss of precision, especially if one limits the word length to something like 16 bits (see answer below). * Doing conversion "right" (in an unbiased way) is non trivial. You cannot just go around truncating digits. * Is is beyond the scope of the driver, which should handle device communication/configurations only. * If the converted value is not needed, the conversion cannot be undone. * In SAUL, conversion to and from the base-10 floating point format used is really painful.

I think the measurement should be raw, and there should be a way to query the conversion constant. This way the user can choose, and there are not unnecessary computations done.

In control applications, for example, the conversion is totally not necessary, as the conversion constants can be folded into the control system constants.

Again, not black and white, but very much depending on the use case. If the conversion is short and its overhead is negligible, I see no need for additional _raw() functions. For all other cases we already provide differentiated APIs (or at least we should).

The design rules that are clear to me are:

- Drivers have to provide an interface for polling with init and read that is compatible with SAUL.

Yes. It makes all interfaces consistent. That being said, it is sad that there is no unified way for configuring and for interrupt driven measurements.

I think our 'dual' approach (driver specific interface and SAUL on top of that) is the way to go. We know, how hard it is to map all the tiny device specific modes into a generic interface (see e.g. periph_), so having something device specific as base is very powerful. SAUL was never meant to map all these specific modes etc, but rather provide something slim but generic, for the most obvious device functions.

Further, SAUL is still more or less a 'prove-of-concept' and in no means complete. Extending it with something that is able to handle events has been on the TODO list for a long time, but nobody ever got to propose something...

- Output are always 16 bit integers.

I think it is a bad idea to limit output to 16 bits. ADCs meant for scales, for example, usually have 24 bit [1]. Other applications also demand higher that 16 bits. Keep in mind that 16 bits is equivalent to 4,8 decimal digits, take 1 bit for the sign and you are left with 4,5.

We don't in general - just SAUL does it. And SAUL is also not designed to handle RAW values, but converted values with a accuracy sufficient for 90+% of typical use cases. If one needs more accuracy, use the driver interface directly.

[1] http://www.analog.com/en/products/ad7799.html

What else?

Maybe off topic, but I think we need a IO layer (think SAUL, but more complete) so that the user does not have to directly interact with drivers. I would answer many of your questions, as in that case there would be a well defined interface that device drivers would have to expose. It is an OS, after all.

Feel free to propose something :slight_smile:

Cheers, Hauke

problems of wiki, where information kills itself ;). Done: https://github.com/RIOT-OS/RIOT/wiki#general-hints

Cheers   matthias

Hi Juan,

thank you for your very comprehensive answer, which gives another view on the use of drivers from a developer who is obviously interested in very accurate results :slight_smile:

Obviously, there are two different groups of application developers that should be supported by a driver interface:

1. Application developers who just want to retrieve sensor data as easy as possible without any special requirements for accuracy and without having detailed knowledge about the sensor, for example the temperature in tenths of degree Celsius.

2. Application developers with deep knowledge about the sensor, the physics and data processing, who want to get raw data from sensor with best accuracy to process them in the application or to use them directly, for example in control applications.

Even though you are right, that a driver in general should realize only the access to the the device, the conversion from raw data is quite complex for some sensors and requires deep knowledge of the sensor and its data sheet. Therefore, there are good reasons why a driver should hide the complexity of processing raw data and provide converted data.

If each sensor driver provides the two functions "read" and "read_raw" both groups of application developers are be satisfied. The "read" function calls "read_raw" implicitly. If the "read" function is not used, it doesn't produce processing overhead.

The parameters of "read_raw" function should of course correspond to the data format of raw data.

Regards Gunar

Hi Juan!

Just a small comment:

Maybe off topic, but I think we need a IO layer (think SAUL, but more complete) so that the user does not have to directly interact with drivers. I would answer many of your questions, as in that case there would be a well defined interface that device drivers would have to expose. It is an OS, after all.

It is an *embedded* OS, after all. Speaking from experience: Trying to design a too generalized driver API is at best difficult, mostly dangerous. ("API Generalization leads to bloat. Bloats to leads to memory consumption. Memory consumptions leads to suffering.")

Maybe you can elaborate a little bit more on the "more complete" part?

Cheers, Oleg