[riot-notifications] [RIOT-OS/RIOT] sys/stdio_ethos: replace USE_ETHOS_FOR_STDIO by stdio_ethos pseudomodule (#11668)

Gaëtan Harter notifications at github.com
Fri Jun 14 16:39:27 CEST 2019


cladmi requested changes on this pull request.

It would be nice to have it implemented without https://github.com/RIOT-OS/RIOT/pull/11598 as this one could be done by adding `stdio_uart_rx` dependency alone.

Some minor remarks inside.

-----

Further steps for other pull requests.

>From the implementation, it looks like `stdio_ethos` should be moved to its own module like `stdio_rtt`. But it is another task that requires many changes.

Also the code is doing implicit circular dependency, the code in `drivers/ethos` for stdio references variables defined in `stdio_uart`…
```
drivers/ethos/ethos.c:extern isrpipe_t stdio_uart_isrpipe;
```

>      cb = NULL;
     arg = NULL;
 #endif
 
-#ifndef USE_ETHOS_FOR_STDIO
-    uart_init(STDIO_UART_DEV, STDIO_UART_BAUDRATE, cb, arg);
+#ifdef MODULE_STDIO_ETHOS
+uart_init(ETHOS_UART, ETHOS_BAUDRATE, cb, arg);

No need to remove the spaces.

> @@ -33,7 +33,7 @@
 #include "net/eui64.h"
 #include "net/ethernet.h"
 
-#ifdef USE_ETHOS_FOR_STDIO
+#ifdef MODULE_STDIO_ETHOS

It would be nice to have a warning if `USE_ETHOS_FOR_STDIO` is still set saying "Deprecated: you must now use `USEMODULE+=stdio_ethos`"

> @@ -398,6 +398,15 @@ ifneq (,$(filter stdio_rtt,$(USEMODULE)))
 endif
 
 ifneq (,$(filter shell,$(USEMODULE)))
+  USEMODULE += stdin
+endif
+
+ifneq (,$(filter stdio_ethos,$(USEMODULE)))

`stdio_ethos` needs `stdio_uart` dependency as it is implemented in it.
It was missing from before too.

If implemented without `stdin` requiring `stdio_uart_rx` would be do the trick.

-- 
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/11668#pullrequestreview-249926299
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.riot-os.org/pipermail/notifications/attachments/20190614/02ce75a2/attachment-0001.html>


More information about the notifications mailing list