[riot-notifications] [RIOT-OS/RIOT] pkg/uwb*: add Kconfig dependency modelling (#16780)

Leandro Lanzieri notifications at github.com
Fri Aug 27 10:27:52 CEST 2021


@leandrolanzieri commented on this pull request.

Some initial comments

> @@ -56,4 +56,14 @@ config HAS_VDD_LC_FILTER_REG1
         Indicates that a board is equipped with an external LC filter circuit
         attached to the CPUs voltage regulator stage 1.
 
+if TEST_KCONFIG
+
+config MODULE_CPU_COMMON
+    bool "Link nrf5x common code"

Not sure that this needs a prompt, as the user would not disable this one:
```suggestion
    bool
```

> +    depends on HAS_VDD_LC_FILTER_REG0
+    default y if HAS_VDD_LC_FILTER_REG0

```suggestion
    depends on HAS_VDD_LC_FILTER_REG0
    default y
```

> +    bool
+    default y
+    help
+        nrf52 common peripheral code.
+
+config MODULE_VDD_LC_FILTER_REG0
+    bool
+    depends on HAS_VDD_LC_FILTER_REG0
+    default y if HAS_VDD_LC_FILTER_REG0
+    help
+        Use the LC filter attached to the CPUs voltage regulator
+
+config MODULE_VDD_LC_FILTER_REG1
+    bool
+    depends on HAS_VDD_LC_FILTER_REG1
+    default y if HAS_VDD_LC_FILTER_REG1

```suggestion
    default y
```

> +config MODULE_MYNEWT-CORE
+    bool "Apache MyNewy mynewt-core RIOT implementation"
+
+config MODULE_AUTO_INIT_MYNEWT-CORE
+    bool "Auto-initialize the mynewt-core package"
+    default y
+    depends on MODULE_AUTO_INIT
+
+config MODULE_MYNEWT-CORE_OS
+    bool "mynewt-core kernel module"
+
+config MODULE_MYNEWT-CORE_UTIL
+    bool "mynewt-core utilities modules"

Are all these modules optional? Meaning the user should be able to opt-out? or is it contrib? Also, maybe we could make `PACKAGE_MYNEWT-CORE` a `menuconfig` and make all these depend on it? would it be correct from the dependency point of view?

> +config MODULE_MYNEWT-CORE_NRF5X_HAL
+    bool "mynewt-core nrf52 and nrf51 timer hal"

Should this depend on the platform?

> +    select MODULE_FMT
+    depends on HAS_PERIPH_GPIO
+    select MODULE_PERIPH_GPIO
+    depends on HAS_PERIPH_GPIO_IRQ
+    select MODULE_PERIPH_GPIO_IRQ
+    depends on HAS_PERIPH_SPI
+    select MODULE_PERIPH_SPI
+    depends on !HAS_ARCH_NATIVE
+    depends on !HAS_ARCH_AVR8
+
+config MODULE_AUTO_INIT_UWB-CORE
+    bool "Auto-initialize the usb-core package"
+    default y
+    depends on MODULE_AUTO_INIT
+
+config MODULE_UWB-CORE_CONTRIB

Same here, I think contrib and others may not be configurable at all, right? In that case select is fine and no prompt is needed.

> @@ -0,0 +1,26 @@
+# Copyright (c) 2021 INRIA
+#
+# This file is subject to the terms and conditions of the GNU Lesser
+# General Public License v2.1. See the file LICENSE in the top level
+# directory for more details.
+#
+
+config PACKAGE_UWB-DW1000
+    bool "Decawave dw1000 driver package"
+    select MODULE_UWB-DW1000_HAL
+    imply PACKAGE_UWB-CORE

Would this still work without `PACKAGE_UWB-CORE` ?

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/RIOT-OS/RIOT/pull/16780#pullrequestreview-740262156
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.riot-os.org/pipermail/notifications/attachments/20210827/291856ea/attachment.htm>


More information about the notifications mailing list