Package: Patch vs Suplimenting with a Module

Development
1

I am trying to pull in a HW vendor’s driver library as a RIOT package. The lib follows the typical pattern of providing an API to interact with the device through the lib and some stubbed out functions that the lib uses to talk to the HW via SPI. The idea is that you are responsible to fill in the stubbed functions with the appropriate code for your MCU. I have successfully compiled the lib and wired it up to RIOT’s SPI peripheral driver.

Now that I have something functional, I am working to organize it all properly. I stuffed the vendor’s code (extracted from the zip they provide) into a git repo. I have added the the package to RIOT such that it downloads the lib, and compiles all but the stubbed c files.

My next task is to provide my implementations of the stubs that call the SPI driver. I can see two approaches and am unsure which is the “right way.” I could make use of RIOT’s package patching facilities to patch up the c files containing the stubs provided by the vendor with my implementations, rather than omit those files. I could also create a module (driver), outside the package, that provides the missing stubs and depends on the package. Then by adding the driver to USEMODULE, the package will also be pulled into the build. This feels more “correct” and maintainable to me in the long run.

I have been looking at how the littlefs2 module works and see that it does something similar to my second approach except that its driver is inside pkg/littlefs2/fs rather than in sys/fs/littlefs2. Is there a compelling reason to hide this module away in this unexpected place?

Should I just patch rather than split the code across a package and a module?

Any other sage advice here?

edit: fix typos and add clarity

2

In regard to how littlefs2 is handled. I see the following lines in /Make.dep

ifneq (,$(filter littlefs2,$(USEMODULE)))
  USEPKG += littlefs2
...
  USEMODULE += littlefs2_fs
...
endif

and this line in /pkg/littlefs2/Makefile.include

DIRS += $(RIOTBASE)/pkg/littlefs2/fs

Which are complications of the module being in this non-standard location (I think).

Also the line USEMODULE += littlefs2_fs does not appear to do anything since the line DIRS += $(RIOTBASE)/pkg/littlefs2/fs already pulls the module in.

3

Hi @enoch247, usually for external code we keep the glue code in the package itself, even the implementation of RIOT API wrapping around that package API. In general pkg glue code, or implementation of some kind of abstraction layer goes into a contrib directory under the package, see for example the lwp package. Or in the example you mention, the code that is under littlefs is the implementation of the vfs api for littlefs.

From what I can deduce from your post you seem to be creating a HAL for that library, IMO it would make sense to have that code under pkg/name_of_pkg/hal or pkg/name_of_pkg/contrib.

BTW have you taken a look at the package documentation as well as the module documentation? It doesn’t address all your questions but gives some extra info.

4

Ok here there are a couple of things, first, a MODULE does not necessarily need to be a compilation unit, there is not necessarily a directory with code associated with it, some of them are used to compile in some specific SRC files, or activate some ifdefs (see pseudomodules).

Now in the specific examples you mention, it is indeed weird, because littlefs2 is pulling in littlefs2_fs no matter what, but normally if there is a compilation unit associated with a module that compilation unit should be pulled in depending on that module being included, so in your example it should be:

ifneq (,$(filter littlefs2_fs,$(USEMODULE)))
  DIRS += $(RIOTBASE)/pkg/littlefs2/fs
endif

Which is what the littlefs package does. So the current status is WRONG, but works because that module is used all the time in any case. But since USEMODULE can also not be linked to source files directly adding something to USEMODULE also means that there will be a define MODULE_FOO if calling USEMODULE += foo, and then some place some code might have e.g.:

if (IS_USED(MODULE_FOO)) {
  auto_init_foo()
}

So there is still a purpose for both. The DIRS variable is there to add directories where MAKE needs to be called, but this is only for distinct compilation units, one compilation unit could hold many modules, see for example ztimer, all those c files are distinct modules (except for core.c, util.c, periodic.c, see ztimer/Makefile and the SUBMODULE section of pseudomodules).

I hope this clears things up a bit, don’t hesitate to ask for more details!