Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sys/stdio_uart: add stdio_clear_stdin #19837

Merged
merged 1 commit into from
Nov 27, 2024

Conversation

HendrikVE
Copy link
Contributor

Contribution description

This PR introduces a new pseudomodule. It is currently only implemented for stdio_uart.

The use case is to clear the input buffer so we can throw away the remaining input (like \n for example) after a call of scanf for the next call of scanf.

Testing procedure

Have an application with multiple calls of scanf and use stdio_flush_rx at some places and don't use it on others.

@github-actions github-actions bot added Area: build system Area: Build system Area: sys Area: System labels Jul 19, 2023
@kaspar030
Copy link
Contributor

bikeshedding: you'd "flush" output, "drain" input?

@HendrikVE
Copy link
Contributor Author

bikeshedding: you'd "flush" output, "drain" input?

I think I'd rather go with a simple "clear" then instead, maybe "stdio_clear_input"?

@MrKevinWeiss
Copy link
Contributor

Should we get this in for the release?

@benpicco benpicco added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Nov 10, 2023
@riot-ci
Copy link

riot-ci commented Nov 10, 2023

Murdock results

✔️ PASSED

6f9edfe sys/stdio_uart: add stdio_clear_stdin

Success Failures Total Runtime
10242 0 10246 15m:08s

Artifacts

@benpicco
Copy link
Contributor

benpicco commented Apr 5, 2024

Needs a rebase, you can move this to sys/stdio/stdio.c now.

@fabian18
Copy link
Contributor

What about ethos? stdio.c is not being compiled when using stdio_ethos.

stdio.inc.mk

STDIO_LEGACY_MODULES = \
  ethos_stdio \
  stdio_ethos \
  stdio_native  # requires #19002 \
  #

# select stdio_uart if no other stdio module is slected
ifeq (,$(filter $(STDIO_MODULES),$(USEMODULE)))
  USEMODULE += stdio_uart
endif

ifeq (,$(filter $(STDIO_LEGACY_MODULES),$(USEMODULE)))
  USEMODULE += stdio
endif

undefined reference to 'stdio_flush_rx' collect2: error: ld returned 1 exit status

@benpicco
Copy link
Contributor

True, ethos would need a custom implementation.
Alternatively it could be ported to the new interface which would require something like #18066

@maribu
Copy link
Member

maribu commented Nov 21, 2024

bikeshedding: you'd "flush" output, "drain" input?

I think I'd rather go with a simple "clear" then instead, maybe "stdio_clear_input"?

@HendrikVE: Ping?

@HendrikVE HendrikVE changed the title sys/stdio_uart: add pseudomodule stdio_flush_rx sys/stdio_uart: add pseudomodule stdio_clear_rx Nov 22, 2024
@HendrikVE
Copy link
Contributor Author

It's called stdio_clear_rx now. I tested it with stdio_uart (works) and stdio_rtt (does not work):

Not using stdio_clear_rx

2024-11-22 18:55:23,799 # main(): This is RIOT! (Version: 2025.01-devel-177-g307c2-stdio_uart_add_flush_rx)
2024-11-22 18:55:23,801 # Welcome to RIOT!
2024-11-22 18:55:23,802 # Enter a character:
h
2024-11-22 18:55:28,159 # You entered: 'h'
2024-11-22 18:55:28,161 # Enter a character:
2024-11-22 18:55:28,162 # You entered: '
2024-11-22 18:55:28,163 # '
2024-11-22 18:55:28,164 # Enter a character:
help
2024-11-22 18:55:29,583 # You entered: 'h'
2024-11-22 18:55:29,585 # Enter a character:
2024-11-22 18:55:29,587 # You entered: 'e'
2024-11-22 18:55:29,588 # Enter a character:
2024-11-22 18:55:29,590 # You entered: 'l'
2024-11-22 18:55:29,591 # Enter a character:
2024-11-22 18:55:29,593 # You entered: 'p'
2024-11-22 18:55:29,595 # Enter a character:
2024-11-22 18:55:29,596 # You entered: '
2024-11-22 18:55:29,596 # '
2024-11-22 18:55:29,598 # Enter a character:

Using stdio_clear_rx

2024-11-22 18:56:31,336 # main(): This is RIOT! (Version: 2025.01-devel-177-g307c2-stdio_uart_add_flush_rx)
2024-11-22 18:56:31,338 # Welcome to RIOT!
2024-11-22 18:56:31,339 # Enter a character:
h
2024-11-22 18:56:32,593 # You entered: 'h'
2024-11-22 18:56:32,595 # Enter a character:
help
2024-11-22 18:56:33,969 # You entered: 'h'
2024-11-22 18:56:33,971 # Enter a character:

Minimal program that I used:

USEMODULE += stdio_clear_rx

#include <stdio.h>
#include "stdio_base.h"

int main(void)
{
    char input_line[2];
    char input;

    while (true) {
        puts("Enter a character:");
        stdio_clear_rx();
        if (fgets(input_line, sizeof(input_line), stdin) != NULL) {
            if (sscanf(input_line, "%c", &input) != 1) {
                puts("Error on parsing user input!");
            }
            else {
                printf("You entered: '%c'\n", input);
            }
        }
        else {
            puts("Error on reading user input!");
        }
    }

    return 0;
}

@HendrikVE
Copy link
Contributor Author

A tiny edit makes sure that stdio_clear_rx is useful when declared by pulling in stdin:

ifneq (,$(filter stdio_clear_rx,$(USEMODULE)))
  USEMODULE += stdin
endif

sys/stdio/stdio.c Outdated Show resolved Hide resolved
sys/stdio/stdio.c Outdated Show resolved Hide resolved
@HendrikVE HendrikVE force-pushed the stdio_uart_add_flush_rx branch 2 times, most recently from 059bb79 to aa13546 Compare November 26, 2024 17:05
@github-actions github-actions bot removed the Area: build system Area: Build system label Nov 26, 2024
@maribu
Copy link
Member

maribu commented Nov 26, 2024

The PR and commit title needs an update now

@HendrikVE HendrikVE changed the title sys/stdio_uart: add pseudomodule stdio_clear_rx sys/stdio_uart: add stdio_clear_rx to clear stdin Nov 27, 2024
@HendrikVE HendrikVE changed the title sys/stdio_uart: add stdio_clear_rx to clear stdin sys/stdio_uart: add stdio_clear_rx for clearing stdin Nov 27, 2024
@HendrikVE HendrikVE changed the title sys/stdio_uart: add stdio_clear_rx for clearing stdin sys/stdio_uart: add stdio_clear_stdin Nov 27, 2024
@HendrikVE
Copy link
Contributor Author

The PR and commit title needs an update now

Did so and also renamed to stdio_clear_stdin

@maribu maribu added this pull request to the merge queue Nov 27, 2024
Merged via the queue into RIOT-OS:master with commit 67a22db Nov 27, 2024
26 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: sys Area: System CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants