pkg/tflite-micro: add kconfig support

[aabadie]

Contribution description

This PR is an attempt to add Kconfig support to the tflite-micro package. Kconfig support is also added to other packages required by tflite-micro: flatbuffers and ruy.

Unfortunately, I can't make it work because it seems external modules are not handled properly (or is it a bug ? or I'm missing something).

Building and running tests/pkg_flatbuffers works (no external modules required).

Building tests/pkg_tflite-micro fails with the following error:

TEST_KCONFIG=1 make -C tests/pkg_tflite-micro/
make: Entering directory '/work/riot/RIOT/tests/pkg_tflite-micro'
=== [ATTENTION] Testing Kconfig dependency modelling ===
=== [ATTENTION] Testing Kconfig dependency modelling ===
Building application "tests_pkg_tflite-micro" for "native" with MCU "native".

"make" -C /work/riot/RIOT/pkg/flatbuffers/ 
"make" -C /work/riot/RIOT/pkg/gemmlowp/ 
"make" -C /work/riot/RIOT/pkg/ruy/ 
"make" -C /work/riot/RIOT/boards/common/init
"make" -C /work/riot/RIOT/boards/native
"make" -C /work/riot/RIOT/boards/native/drivers
"make" -C /work/riot/RIOT/core
"make" -C /work/riot/RIOT/core/lib
"make" -C /work/riot/RIOT/cpu/native
"make" -C /work/riot/RIOT/cpu/native/periph
"make" -C /work/riot/RIOT/cpu/native/stdio_native
"make" -C /work/riot/RIOT/drivers
"make" -C /work/riot/RIOT/drivers/periph_common
"make" -C /work/riot/RIOT/sys
"make" -C /work/riot/RIOT/sys/auto_init
"make" -C /work/riot/RIOT/sys/test_utils/interactive_sync
"make" -C /work/riot/RIOT/sys/test_utils/print_stack_usage
"make" -C /work/riot/RIOT/tests/pkg_tflite-micro/external_modules/mnist
/work/riot/RIOT/tests/pkg_tflite-micro/external_modules/mnist/main_functions.cc:26:10: fatal error: tensorflow/lite/micro/all_ops_resolver.h: No such file or directory
   26 | #include "tensorflow/lite/micro/all_ops_resolver.h"
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
compilation terminated.

I don't know what could be missing here.

Testing procedure

• Green Murdock

Issues/PRs references

Follow-up of #17908
Waiting for #17992

[fjmolinas]

external modules or packages?

[aabadie]

The test application uses the package and an external module.

RIOT/tests/pkg_tflite-micro/Makefile

Line 6 in a427d63

USEPKG += tflite-micro

RIOT/tests/pkg_tflite-micro/Makefile

Lines 11 to 12 in a427d63

	USEMODULE += mnist
	EXTERNAL_MODULE_DIRS += external_modules

[fjmolinas]

Can you try with

diff --git a/pkg/tflite-micro/Kconfig b/pkg/tflite-micro/Kconfig
index 461596636a..589cc1aea0 100644
--- a/pkg/tflite-micro/Kconfig
+++ b/pkg/tflite-micro/Kconfig
@@ -5,7 +5,7 @@
 # directory for more details.
 #
 
-config PACKAGE_TFLITE_MICRO
+config PACKAGE_TFLITE-MICRO
     bool "TFlite Micro package"
     depends on TEST_KCONFIG
 
diff --git a/tests/kconfig/external_modules/external_module_1/include/external_module_1.h b/tests/kconfig/external_modules/external_module_1/include/external_module_1.h
index a60d12ee83..f37fae5f7b 100644
--- a/tests/kconfig/external_modules/external_module_1/include/external_module_1.h
+++ b/tests/kconfig/external_modules/external_module_1/include/external_module_1.h
@@ -27,6 +27,8 @@
 extern "C" {
 #endif
 
+#include "kernel_defines.h"
+
 #ifndef CONFIG_EXTERNAL_MODULE_1_MESSAGE
 #define CONFIG_EXTERNAL_MODULE_1_MESSAGE    "this should not show"
 #endif
diff --git a/tests/pkg_tflite-micro/app.config.test b/tests/pkg_tflite-micro/app.config.test
index efc88a476a..4798ff9937 100644
--- a/tests/pkg_tflite-micro/app.config.test
+++ b/tests/pkg_tflite-micro/app.config.test
@@ -1,2 +1,2 @@
-CONFIG_PACKAGE_TFLITE_MICRO=y
+CONFIG_PACKAGE_TFLITE-MICRO=y
 CONFIG_MODULE_MNIST=y

[aabadie]

Maybe I should add a Makefile.include in the external module but I don't see why that would be necessary. (Although I see some mismatch here)

[aabadie]

Ok, thanks @fjmolinas. It should be good now (with the right module names). Hope Murdock won't find any module mismatch with and without Kconfig...

[fjmolinas]

Ok, thanks @fjmolinas. It should be good now (with the right module names). Hope Murdock won't find any module mismatch with and without Kconfig...

If you want you could also rename the pkg to avoid this confusion, I had the same issue with uwb-core at some point... (haven't renamed yet though)

[aabadie]

Hope Murdock won't find any module mismatch with and without Kconfig...

Of course, there were. I could fix the problem related to cpp in abef5d0: cpp modules still don't have Kconfig support, I'll open a more complete PR for that and rebase this one on top later.
I don't know how to properly handle the disabled cortexm_fpu module in the application config. Murdock genkconfig.py fails with the following error:

tests/pkg_tflite-micro/app.config.test:3: warning: attempt to assign the value 'n' to the undefined symbol MODULE_CORTEXM_FPU

Any idea ?

[leandrolanzieri]

Hope Murdock won't find any module mismatch with and without Kconfig...

Of course, there were. I could fix the problem related to cpp in abef5d0: cpp modules still don't have Kconfig support, I'll open a more complete PR for that and rebase this one on top later. I don't know how to properly handle the disabled cortexm_fpu module in the application config. Murdock genkconfig.py fails with the following error:

tests/pkg_tflite-micro/app.config.test:3: warning: attempt to assign the value 'n' to the undefined symbol MODULE_CORTEXM_FPU

Any idea ?

So, the issue here is that we are trying to set the value to a symbol that is not always defined (only for cortexm). IMO the ideal solution would be to have a generic symbol/module to control FPU independent of the platform (MODULE_FPU or so), but maybe we can have that on a separate PR. I'd say that for now we can try and override the default in the application Kconfig file, but this also requires a change in the source order in the root Kconfig: the application should have priority against cpu and boards.

diff --git a/Kconfig b/Kconfig
index 935a7f61f6..4067f4da34 100644
--- a/Kconfig
+++ b/Kconfig
@@ -18,6 +18,9 @@ rsource "kconfigs/Kconfig.errors"
 # For now, get used modules as macros from this file (see kconfig.mk)
 osource "$(KCONFIG_GENERATED_DEPENDENCIES)"
 
+# The application may declare new symbols as well
+osource "$(APPDIR)/Kconfig"
+
 # Load first board configurations, which might override CPU's
 orsource "$(BOARDDIR)/Kconfig"
 orsource "$(RIOTCPU)/$(CPU)/Kconfig"
@@ -25,8 +28,6 @@ orsource "$(RIOTCPU)/$(CPU)/Kconfig"
 rsource "$(RIOTBOARD)/Kconfig"
 rsource "$(RIOTCPU)/Kconfig"
 
-# The application may declare new symbols as well
-osource "$(APPDIR)/Kconfig"
 
 rsource "core/Kconfig"
 rsource "drivers/Kconfig"
diff --git a/tests/pkg_tflite-micro/app.config.test b/tests/pkg_tflite-micro/app.config.test
index 7b362a4748..4798ff9937 100644
--- a/tests/pkg_tflite-micro/app.config.test
+++ b/tests/pkg_tflite-micro/app.config.test
@@ -1,3 +1,2 @@
 CONFIG_PACKAGE_TFLITE-MICRO=y
 CONFIG_MODULE_MNIST=y
-CONFIG_MODULE_CORTEXM_FPU=n
leandro@solaria ~/W/RIOT (pr/pkg/tflite-micro-kconfig)> git add .
leandro@solaria ~/W/RIOT (pr/pkg/tflite-micro-kconfig)> git diff --staged
diff --git a/Kconfig b/Kconfig
index 935a7f61f6..4067f4da34 100644
--- a/Kconfig
+++ b/Kconfig
@@ -18,6 +18,9 @@ rsource "kconfigs/Kconfig.errors"
 # For now, get used modules as macros from this file (see kconfig.mk)
 osource "$(KCONFIG_GENERATED_DEPENDENCIES)"
 
+# The application may declare new symbols as well
+osource "$(APPDIR)/Kconfig"
+
 # Load first board configurations, which might override CPU's
 orsource "$(BOARDDIR)/Kconfig"
 orsource "$(RIOTCPU)/$(CPU)/Kconfig"
@@ -25,8 +28,6 @@ orsource "$(RIOTCPU)/$(CPU)/Kconfig"
 rsource "$(RIOTBOARD)/Kconfig"
 rsource "$(RIOTCPU)/Kconfig"
 
-# The application may declare new symbols as well
-osource "$(APPDIR)/Kconfig"
 
 rsource "core/Kconfig"
 rsource "drivers/Kconfig"
diff --git a/tests/pkg_tflite-micro/Kconfig b/tests/pkg_tflite-micro/Kconfig
new file mode 100644
index 0000000000..10d137681d
--- /dev/null
+++ b/tests/pkg_tflite-micro/Kconfig
@@ -0,0 +1,7 @@
+
+config MODULE_CORTEXM_FPU
+    bool
+    default n
+    depends on TEST_KCONFIG
+    depends on CPU_CORE_CORTEX_M
+    depends on HAS_CORTEXM_FPU
diff --git a/tests/pkg_tflite-micro/app.config.test b/tests/pkg_tflite-micro/app.config.test
index 7b362a4748..4798ff9937 100644
--- a/tests/pkg_tflite-micro/app.config.test
+++ b/tests/pkg_tflite-micro/app.config.test
@@ -1,3 +1,2 @@
 CONFIG_PACKAGE_TFLITE-MICRO=y
 CONFIG_MODULE_MNIST=y
-CONFIG_MODULE_CORTEXM_FPU=n

[aabadie]

Regarding the cortexm_fpu problem, I could fix it in 8a10e50. Not sure if the fix is appropriate but the idea is to always define it as false by default. The symbol is defined at cpu level so it's defined for all cpus even if it doesn't make senses for non cortexm.

[leandrolanzieri]

Regarding the cortexm_fpu problem, I could fix it in 8a10e50. Not sure if the fix is appropriate but the idea is to always define it as false by default. The symbol is defined at cpu level so it's defined for all cpus even if it doesn't make senses for non cortexm.

Yes, I noticed that. I'm not sure about that fix, mainly because it is defined for all CPUs even when it would not make sense. That's why I thought that having a CPU-independent symbol for FPU enabling, which would be in this direction but would scale with more CPUs.

[aabadie]

Looking at your proposed solution, I don't think it makes much sense. Why is it the package responsibility to declare a cortexm_fpu symbol ?

Ah, it's declared in the application Kconfig file.

[aabadie]

@leandrolanzieri, I applied your suggestion and put the main Kconfig change in its own commit. I tested on a STM32F7 (with FPU) and it worked which means the FPU is correctly disabled. Also tested on native where it still works.

[aabadie]

All green here :)

[leandrolanzieri]

CI is happy and changes look good. Thanks for tackling the application kconfig order as well. ACK