tests/driver_ws281x: resolve weird feature dependencies

[miri64]

Contribution description

While browsing through some features I'm interested in, I stumbled upon this weird construct in the Makefile of tests/ws281x. It was commented on in the PR that introduced it but to me no proper reason was given why not just FEATURES_BLACKLIST was used.

Testing procedure

BOARD=native make -C tests/driver_ws281x/ info-boards-supported

has the same output as

make -C tests/driver_ws281x/ info-boards-supported

(in master, all boards are outputted for the first).

Issues/PRs references

None

[maribu]

See inline

[maribu]

This is not going to work. E.g. the GPIO ABC backend will add support for this driver on some ARM boards, but only those with GPIO ABC support.

This cannot be reasonably be solved with the current way of dependency tracking. Maybe KConfig allows to model "depends on: foo OR bar OR baz"

[miri64]

Can't you just in this case do something like removing FEATURES_BLACKLIST += arch_arm and add FEATURES_REQUIRES += periph_gpio_abc for ARM platforms in Makefile.board.dep? And how does the current hack help with the situation?

[maribu]

The current approach couldbe extended by testing for the backends one by one and adding FEATURES_REQUIRED := arch_avr8 if none of the other backend matched.

But better would be to teach the dependency system the logical or.

[miri64]

You did not answer my first question. Isn't this already solvable without hacks and the current build system approach?

[miri64]

This is how the appropriate patch could look like (untested):

diff --git a/tests/driver_ws281x/Makefile b/tests/driver_ws281x/Makefile
index 43323538e9..94aaac3a38 100644
--- a/tests/driver_ws281x/Makefile
+++ b/tests/driver_ws281x/Makefile
@@ -10,7 +10,6 @@ USEMODULE += ws281x
 # Currently the ws281x only supports AVR-based platforms and native
 # (via VT100 terminals).
 # See https://doc.riot-os.org/group__drivers__ws281x.html
-FEATURES_BLACKLIST += arch_arm
 FEATURES_BLACKLIST += arch_esp32
 FEATURES_BLACKLIST += arch_esp8266
 FEATURES_BLACKLIST += arch_mips32r2
diff --git a/tests/driver_ws281x/Makefile.board.dep b/tests/driver_ws281x/Makefile.board.dep
new file mode 100644
index 0000000000..dbc03d05fd
--- /dev/null
+++ b/tests/driver_ws281x/Makefile.board.dep
@@ -0,0 +1,3 @@
+ifneq (,$(filter arch_arm,$(FEATURES_PROVIDED)))
+  FEATURES_REQUIRED += periph_gpio_abc
+endif

[maribu]

There might be ARM boards that don't have GPIO ABC, but SPI or I2S. What really is needed is a logical or. I think that this is actually quite feasible to implement.

It would be used by e.g :

FEEATURES_REQUIRED += arch_native|arch_armv8|periph_spi|periph_i2s|periph_gpio_abc

[fjmolinas]

Yes we currently can't do this in out build-system. Maybe some kind of construct similar to the one done for FEATURES_CONFLICT could be done. It could declare lists of options where at least one has to be provided.

FEATURES_REQUIRED_OPTIONS += arch_native:arch_armv8:periph_spi:periph_i2s:periph_gpio_abc

But this would require more reflection, and I'm not sure o the impact.

[benpicco]

That's indeed a much simpler solution!

But if someone implements an SPI or I2S based backend, how would we handle that?

But the current hack won't handle that either.

[maribu]

I have nothing against this PR. But on long term, a different approach will be needed.

[miri64]

I have nothing against this PR. But on long term, a different approach will be needed.

Ok, then let's set the label for now. Maybe @aabadie and @fjmolinas have some idea about your OR proposal in #13343 (comment)

[benpicco]

I prefer @maribu's solution from #13349 but until it is merged this will make the test less quirky.

[miri64]

Me too, but I saw that we somehow forgot merging this, so I hit the green button.