[riot-notifications] [RIOT-OS/RIOT] black/white e-Paper/e-Ink display driver (#12509)

Silke Hofstra notifications at github.com
Wed May 19 13:54:31 CEST 2021


@silkeh commented on this pull request.



> @@ -0,0 +1,10 @@
+BOARD ?= nucleo-f411re
+
+include ../Makefile.tests_common
+
+USEMODULE += xtimer
+USEMODULE += epd_bw_spi
+
+INCLUDES += -I$(APPDIR)

> This is a configuration for your personal setup. I don't think it should be put there (I mean in RIOT). 

Sure, but it's the only setup I have to test with.

> Maybe someone with a nucleo-f411re won't wire the same e-ink paper display than yours and maybe not on the same pins.

Which is why the pinout is documented...

> So I would simply remove the epd_bw_spi_params.h include from the application directory (it should remain generic as much as possible).

Sure, but that just means that I have to keep it untracked in my code base. Otherwise I won't be able to actually run this application.

> Note that this would simply forbid the possibility to build this application with a board providing a e-ink paper: the board support won't be able to provide custom defines (different pins, screen size, controller) the usual way.

How would removing the header file lead to this application not working with a board having an e-paper display?

> The x/y display sizes and controller are the same as the default ones, so no need to overwrite them here.

I'd much rather have a complete override instead of a partial one in case defaults change.

> Also note that I would be ok to move the setup specific pin definitions to the default driver params with a comment (use the same one used in the application params.h file, you can even generalize to any Nucleo 64 board I guess, they have a compatible pinout).

Sorry, but I don't think it makes any sense: my personal setup can't be included explicitly, but it's fine to do it implicitly as driver defaults?

-- 
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/12509#discussion_r635166077
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.riot-os.org/pipermail/notifications/attachments/20210519/af6500d6/attachment-0001.htm>


More information about the notifications mailing list