[RFC] Introduce drivers as package

[dylad]

Description

I've created this issue to centralize discussions about package/drivers. In some case, sensors configuration or computation are quite complicate and some vendors already provide some drivers. The idea here is to benefit from those drivers and integrate them as package. Thus, we will avoid maintenance on low-level stuff but we will still have to maintain an adaption layer to RIOT. @gschorcht already asked some questions :

• Should we create a sub-directory within pkg/ to regroup this kind of drivers ? Or should we just use a prefix/suffix like driver_pkgname/pkgname_driver ?
• Should we let full access to the vendor API through the package AND provides basic functionalities like driverxxx_init(),driverxxx_read() and driverxxx_t struct, driverxxx_params_t ?
• How can we integrate this to the SAUL interface ?

Useful links

See also these PRs

10363

10502

[jcarrano]

I agree that there is no point in copying manufacturer's drivers into RIOT:

• We have a PKG infrastructure structure to specifically designed to handle third party code.
• In my (very partial and biased) opinion most of those manufacturer-supplied drivers tend to be of the lowest quality - bloated lasagna-code.
• By not bundling the code, we free ourselves from any license/copyright liability.

Should we create a sub-directory within pkg/ to regroup this kind of drivers ?

I'd say no. That would complicate the build system only to provide a filesystem-based organization, whose benefits are not clear to me.

Or should we just use a prefix/suffix like driver_pkgname/pkgname_driver ?

That's a matter of convention.

Should we let full access to the vendor API through the package AND provides basic functionalities like driverxxx_init(), driverxxx_read() and driverxxx_t struct, driverxxx_params_t ?

There are two parts to a riot PGK, the third party code itself, and the "contrib". IMO the contrib should be optional if possible (i.e. another package)

How can we integrate this to the SAUL interface ?

See #9105 . It will allow "registry-like" functionality without any central file. For example init fuctions would be placed into a XFA.

[gschorcht]

In my (very partial and biased) opinion most of those manufacturer-supplied drivers tend to be of the lowest quality - bloated lasagna-code.

I completly agree.

[gschorcht]

Should we create a sub-directory within pkg/ to regroup this kind of drivers ?

I'd say no. That would complicate the build system only to provide a filesystem-based organization, whose benefits are not clear to me.

I agree.

Or should we just use a prefix/suffix like driver_pkgname/pkgname_driver ?

I would prefer to use the prefix driver_ for driver packages so that they are at least visually grouped in alphabetical order. We use the same naming scheme for test applications in tests.

There are two parts to a riot PGK, the third party code itself, and the "contrib". IMO the contrib should be optional if possible (i.e. another package)

I don't agree for driver packages. According to the driver design rules, a driver should be configured by a parameter set of type <driver>_params_t and should be operational once it is initialized using this parameter set. IMHO, for vendor driver packages, especially sensor driver packages, there should be a small driver as a wrapper module with at least these 4 elements <driver>_init(), <driver>_read(), <driver>_t, and <driver>_params_t should be placed indrivers`. This small wrapper module can also enable the package so that the application has only to enable the driver module. An example can be found in #10363.

This also makes it easy to use SAUL.

[Citrullin]

By not bundling the code, we free ourselves from any license/copyright liability.

I especially agree on this point. Since you just reference some github repo, RIOT OS itself is separated with its LGPL license. It's the same for application implementations. It's an elegant way to have clear separation, even if you have combine it later in a binary. But from a legal perspective, RIOT has a really great design.

I don't agree for driver packages. According to the driver design rules, a driver should be configured by a parameter set of type _params_t and should be operational once it is initialized using this parameter set. IMHO, for vendor driver packages, especially sensor driver packages, there should be a small driver as a wrapper module with at least these 4 elements _init(), _read(), _t, and _params_t should be placed in drivers`. This small wrapper module can also enable the package so that the application has only to enable the driver module. An example can be found in #10363.

I think that is a nice design for drivers, since you still find the drivers in the driver directory, but the third party code is separated and can be found in pkg. A user need to clone the github repo, therefore, RIOT is not conflicting with any licenses. You could even use proprietary libraries.

[dylad]

I must said that I like the way @gschorcht wrote #10363.

I don't agree for driver packages. According to the driver design rules, a driver should be configured by a parameter set of type _params_t and should be operational once it is initialized using this parameter set. IMHO, for vendor driver packages, especially sensor driver packages, there should be a small driver as a wrapper module with at least these 4 elements _init(), _read(), _t, and _params_t should be placed in drivers`. This small wrapper module can also enable the package so that the application has only to enable the driver module. An example can be found in #10363.

SAUL adaption will also benefits from this. IMHO this is the way to go.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If you want me to ignore this issue, please mark it with the "State: don't stale" label. Thank you for your contributions.

[gschorcht]

I try to summarize it to get an agreement on how to use vendor driver code:

1. Vendor driver code is imported as a package according to http://doc.riot-os.org/group__pkg.html.
2. The package is located in pkg directory and uses the prefix driver_ for its name, e.g., driver_xyz.

3. An associated wrapper module is realized in drivers which implements an interface for such a device according to RIOT's APIs. For example, a sensor driver would consist of the files

   • drivers/include/xyz.h
   • drivers/xyz/include/xyz_params.h
   • drivers/xyz/xyz.c
   • drivers/xyz/xyz_saul.c

   and would implement at least the 4 basic elements

   • xyz_t
   • xyz_params_t
   • xyz_init()
   • xyz_read()

   according to the driver development guide. Using this approach makes it possible to use a vendor driver package in RIOT's standard way including SAUL.

   A network device driver would implement the interface according to the Network Device Driver API.

[aabadie]

There are now a couple of drivers provided from packages (bme680, atwinc15x0). I started to write one following the same approach and I know @fjmolinas is also working on a new one the same way.

I think we can conclude that https://github.com/RIOT-OS/RIOT/issues/10506#issuecomment-520143665 is the design to follow and close this issue. What do you think @gschorcht @fjmolinas @dylad ?

[gschorcht]

There are now a couple of drivers provided from packages (bme680, atwinc15x0). I started to write one following the same approach and I know @fjmolinas is also working on a new one the same way.

I think we can conclude that #10506 (comment) is the design to follow and close this issue. What do you think @gschorcht @fjmolinas @dylad ?

of course :+1: from my side, the proposal came from me :wink:

[dylad]

I think we can conclude that #10506 (comment) is the design to follow and close this issue. What do you think @gschorcht @fjmolinas @dylad ?

I am happy with this !

[aabadie]

So let's close then!