Riot: stdio: a layered / multiheaded approach to I/O

Created on 25 Feb 2020 · 14Comments · Source: RIOT-OS/RIOT

Description

We need to rethink our STDIO model! With new methods for STDIO emerging (stdio_cdc_acm, stdio_nimble, ...) and usage of modules that multiplex STDIOs with other functionality (e.g. ethos or slipdev to multiplex network connections via serial) getting more and more complicated to configure, while only being able to just talk to one specific lower-end implementation of STDIO (stdio_uart in ethos's case) this becomes clearer and clearer. After some brainstorming with @haukepetersen and @kaspar030 I decided to write down our initial ideas to be build upon and maybe to find someone™ to implement it in the end.

Problems with the current model

The current model maps STDIO system calls to the functions stdio_read() and stdio_write(), these can be implemented by a stdio_* module to provide the desired functionality. The problem is that this only allows for exactly one STDIO implementation at a time. This leads to problems if this use case isn't given anymore.

Use cases

There are several use cases that call for a new model.

Mapping a more advanced STDIO on top of a more basic STDIO: e.g. multiplexing STDIO with other functionality such as in ethos.
Having multiple STDIO interfaces: e.g. stdio_cdc_acm as main interface for user interaction but also providing stdio_uart for debugging (in case stdio_cdc_acm is unable to be initialized).
Having no STDIO: This is currently modeled as a module with empty operations called stdio_null. However, maybe we find a way to necessitate it at all.

Model proposal

My initial gut / keep-it-simple feeling would call for a driver-based approach. That mean that a new STDIO implementation would provide a struct with function pointers similar as we did it elsewhere (e.g. in netdev).

typedef struct {
    ssize_t (*read)(void *buffer, size_t max_len);
    ssize_t (*write)(const void *buffer, size_t len);
} stdio_t;

Additionally a module dependent init-function is provided.
A top-level implementation (under the current model that would be the equivalent to stdio_read()/stdio_write()) would then call a stdio_t object that is provided via configuration.

static const stdio_t *_stdio;

void stdio_init(const stdio_t *stdio)
{
    _stdio = stdio;
}

ssize_t stdio_read(void *buffer, size_t max_len)
{
     return _stdio->read(buffer, max_len)
}

ssize_t stdio_write(const void *buffer, size_t len)
{
     return _stdio->write(buffer, max_len)
}

This should IMHO cover at least use case 1 and 2 (please understand the code snippets more as pseudo-code than actual implementation ideas, I don't think it is as easy as I make it seem here):

Initialize the more advanced STDIO with the more basic STDIO and use the advanced STDIO in the top-level implementation, e.g.
C const stdio_t *stdio = stdio_uart_init(uart_config, baudrate); stdio = ethos_init(stdio, ...); stdio_init(stdio);
Provide a multiplexer STDIO to call multiple other STDIOs
C const stdio_t *stdios[] = { uart, cdc_acm, NULL, } const stdio_t *stdio = stdio_multiplex_init(stdios); stdio_init(stdio);

Use case 3 still would be a NOP module, but we could use some compile-time config to make at least stdio_read() and stdio_write() also NOPs in case no other STDIO module is present.

For the more graphically minded people here is a somewhat thrown together UML diagram of what I am thinking of (showing both the cases shown in 1. and 2. as a model)

Untitled Diagram

More considerations

Provide support for different STDIOs to different file descriptors / module-wise STDIO? (https://github.com/RIOT-OS/RIOT/issues/13469#issuecomment-595666277)
Provide a separate module list (e.g. STDIO_MODULES) for STDIO modules (https://github.com/RIOT-OS/RIOT/pull/13650#issuecomment-600674597)
Have there no stdio module if stdio is not needed (https://github.com/RIOT-OS/RIOT/pull/13650#discussion_r394381631)

Useful links

Related issues and PRs:

sys help wanted RFC enhancement new feature

Source

miri64

👍2 ❤1

All 14 comments

Nice sum-up! The ops look pretty much like the vfs_file_ops. Let's see if we can re-use some of that code.

Maybe refactor vfs a bit, so the mounting / actual FS support becomes optional, and it can be used with only the file descriptor handling. That in turn could then become default, dropping the current non-vfs use case.

kaspar030 on 25 Feb 2020

Maybe refactor vfs a bit, so the mounting / actual FS support becomes optional, and it can be used with only the file descriptor handling. That in turn could then become default, dropping the current non-vfs use case.

An alternative could be to factor file descriptor table / handling out of VFS.

kaspar030 on 25 Feb 2020

[…] The ops look pretty much like the vfs_file_ops. Let's see if we can re-use some of that code.

Not quite sure where you see the similarity (except that both are a struct of function pointers...). Compared to my proposal the vfs_file_ops struct is quite large.

https://github.com/RIOT-OS/RIOT/blob/ab333f3fb205d46e6915b16fffe97cf279819850/sys/include/vfs.h#L258-L373

miri64 on 25 Feb 2020

Not quite sure where you see the similarity (except that both are a struct of function pointers...). Compared to my proposal the vfs_file_ops struct is quite large.

Yeah you're right. I just looked at this, which made is seem similar.

https://github.com/RIOT-OS/RIOT/blob/e787224ae94fc362b347ebad773977eedac203e2/sys/vfs/vfs_stdio.c#L53-L56

kaspar030 on 25 Feb 2020

Thought as much, because for a second while writing OP I thought the same ;-). But quickly remembered "No the rest is just initialized with NULL" ^^

miri64 on 25 Feb 2020

Hello, my tech leader advised me to take a look at this issue as I am working on the same problem. I need to have a remote shell on an MCU, which I would like to do with MQTT . Therefore I need to be able to redirect STDIO during runtime. For this I created a "stdio_switcher" which look like solution 2 multiplexer. I defined a new fileops, stdio write/read and vfs_bind for mqtt and made sure to modify allocate_fd() so it doesn't return an error when fd is already in use. I do like the idea of by passing vfs, the ongoing work looks something like this...

````
stdio_sw.c
void stdio_redirection(int opt)
{
switch (opt) {
case SW_DEFAULT:
vfs_bind_stdio();
break;

ifdef MODULE_STDIO_MQTT

    case SW_MQTT_READ:
        vfs_bind_stdio_mqtt(opt);
        break;

    case SW_MQTT_WRITE:
        vfs_bind_stdio_mqtt(opt);
        break;

endif

    default:
        printf("stdio_sw: stdio_redirection() failure!\n");
        return;
}

}

vfs_stdio.c

ifdef MODULE_STDIO_MQTT

void vfs_bind_stdio_mqtt(int opt)
{
int fd;
switch (opt) {
case 1:
/* rebind READ ONLY */
fd = vfs_bind(STDIN_FILENO, O_RDONLY, &_stdio_ops_mqtt, (void *)STDIN_FILENO);
assert(fd >= 0);
(void)fd;
break;

    case 2:
        /* rebind WRITE ONLY */
        fd = vfs_bind(STDOUT_FILENO, O_WRONLY, &_stdio_ops_mqtt, (void *)STDOUT_FILENO);
        assert(fd >= 0);
        fd = vfs_bind(STDERR_FILENO, O_WRONLY, &_stdio_ops_mqtt, (void *)STDERR_FILENO);
        assert(fd >= 0);
        (void)fd;
        break;

    default:
        printf("stdio_mqtt: vfs_bind_stdio_mqtt() no fd have been rebound\n");
        return;
}

}

endif

vfs.c

ifdef MODULE_STDIO_SW

    /* fd is already in use but we still free it before reallocation */
    _free_fd(fd);

else

    /* The desired fd is already in use */
    return -EEXIST;

endif

nisennenmondai on 26 Feb 2020

For this I created a "stdio_switcher" which look like solution 2 multiplexer.

Just to be clear: I did not present any solutions here, just use cases that call for a new model. The new STDIO interface should cover all 3 use cases ideally.

miri64 on 26 Feb 2020

The multiplexer I suggested for 2 could also e.g. be implemented as a linked list (given that some STDIOs might need to be initialized later than others (e.g. stdio_uart can be initialized quite early, stdio_nimble or your MQTT proposal need a network stack running first), this might even be the more desirable solution with having a stdio_multiplexer_add() function also present.

miri64 on 26 Feb 2020

👍1

Great discussion!

I think being able to use only one IO device at the time is limiting. There should be a common abstraction for IO devices, that currently is the stdio abstraction. But one should be able to read/write from/to multiple IO devices at the same time. With the proposed multiplexer solution this is not possible AFAICS.

Maybe it makes sense to have a vfs file available for each of these IO devices?

Exemplary use case for this: I want to use the shell via uart and log output to USB. If both the shell implementation and the logging implementation would use the vfs abstraction this would be possible.

toonst on 6 Mar 2020

Maybe it makes sense to have a vfs file available for each of these IO devices?

We should find a solution independent of vfs, as IMHO we should not enforce that dependency. That said: how does having different IO descriptors (as proposed) differ from having different vfs files that makes the one possible and the other not?

If I understand correctly, what you propose is basically providing different files. With some tweaking of the implementations we could already do that with the current model. However, its usage would be equivalent to setting sys.stdout to a different file in a python script. This however does not change the system IO and IMHO the solution coming out of this discussion should cover the system IO holistically.

miri64 on 6 Mar 2020

I think the problem is that most modules (eg. shell) are using the system IO, while in fact we should not use system IO at all but rather a different abstraction that allows for setting a file descriptor per module.

toonst on 9 Mar 2020

IMHO that is a completely different use case, that, given you are asking for it, maybe should consider in our model as well. First and foremost however we should fix the mess that is our system IO before putting new features on top ;-).

miri64 on 9 Mar 2020

👍1

While discussing DEFAULT_MODULEs with @leandrolanzieri we though we could clean the current state of stdio_% by adding a new STDIO_MODULE variable (I think @miri64 mentioned in another PR something like that as well).

Since we currently to not have support for multiple stdio we could simply use a variable that gets overridden by conflicting applications. That means that boards like hamilton would declare STDIO_MODULE ?= stdio_rtt and the applications like examples/gnrc_border_router could overwrite it by declaring STDIO_MODULE ?= stdio_ethos.

Then in stdio.inc.mk if STDIO_MODULE exists it get converted to a USEMODULE, and if not usual resolution happens (where probably stdio_uart would end up being used??).

We could also check against STDIO_MODULE_OPTIONS or STDIO_MODULES to verify STDIO_MODULE has a valid value.

This would replace the current handling through DEFAULT_MODULE. It would also be an opportunity to state which is the default STDIO_MODULE (I think its stdio_uart but its not explicit).

I wanted to bring it up in this issue in case you think there a reason this would conflict with what is being attempted here.

fjmolinas on 1 Apr 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If you want me to ignore this issue, please mark it with the "State: don't stale" label. Thank you for your contributions.