Skip to content

cpu/native/periph/uart: uart_poweroff() closes the file descriptor + extended baud-rate support on Linux#20365

Merged
benpicco merged 1 commit intoRIOT-OS:masterfrom
derMihai:mir/native_uart_poweroff
Feb 10, 2024
Merged

cpu/native/periph/uart: uart_poweroff() closes the file descriptor + extended baud-rate support on Linux#20365
benpicco merged 1 commit intoRIOT-OS:masterfrom
derMihai:mir/native_uart_poweroff

Conversation

@derMihai
Copy link
Copy Markdown
Contributor

@derMihai derMihai commented Feb 9, 2024 •
edited
Loading

Contribution description

uart_poweroff() now closes the underlying file descriptor

This is needed if for re-initializing the UART device. An example use-case is baud-rate probing.

On Linux, extended baud-rate support

On Linux, the extended baud-rates are defined in bits/termios-baud.h as follows:

/* Extra output baud rates (not in POSIX).  */
#define  B57600    0010001
#define  B115200   0010002
#define  B230400   0010003
#define  B460800   0010004
...

As such, I'm not sure if they are available on other platforms too, so I enabled them for Linux only.

Testing procedure

The following should be a valid use-case:

   static uint32_t const baud_rates[] = {
            230400,
            460800,
            921600,
            3000000,
    };
    int res;
    for (unsigned i = 0; i < ARRAY_SIZE(baud_rates); i++) {
        uart_poweroff(UART_DEV(0));
        res = uart_init(UART_DEV(0), baud_rates[i], device_rx_handler, dev);
        assert(res == 0);

        res = device_probing_procedure(UART_DEV(0));
        if (res == 0) {
            /* device responded */
            break;
        }
    }

@github-actions github-actions bot added Platform: native Platform: This PR/issue effects the native platform Area: cpu Area: CPU/MCU ports labels Feb 9, 2024
benpicco
benpicco reviewed Feb 9, 2024
View reviewed changes
@derMihai derMihai force-pushed the mir/native_uart_poweroff branch from 437df91 to 795415d Compare February 9, 2024 11:56
@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 Feb 9, 2024
@riot-ci
Copy link
Copy Markdown

riot-ci commented Feb 9, 2024

Murdock results

✔️ PASSED

795415d cpu/native/periph/uart: uart_poweroff() closes the file descriptor

Success Failures Total Runtime
10014 0 10016 09m:38s

Artifacts

@derMihai
Copy link
Copy Markdown
Contributor Author

derMihai commented Feb 9, 2024

On a second glance, it looks that some baudrates already included (e.g. 115200) are not POSIX either, so I think I should enable them all uncoditionally.

@benpicco benpicco added this pull request to the merge queue Feb 9, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Feb 9, 2024
@benpicco benpicco added this pull request to the merge queue Feb 10, 2024
fabian18
fabian18 reviewed Feb 10, 2024
View reviewed changes
cpu/native/async_read.c
if (res < 0) {
err(EXIT_FAILURE, "native_async_read_remove_handler(): fcntl(F_GETFL)");
}
unsigned flags = (unsigned)res & !O_ASYNC;
Copy link
Copy Markdown
Contributor

@fabian18 fabian18 Feb 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should that be ~O_ASYNC?

Copy link
Copy Markdown
Member

@maribu maribu Feb 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@derMihai and @benpicco ⬆️

Copy link
Copy Markdown
Contributor Author

@derMihai derMihai Feb 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ouch, yes, ~O_ASYNC. Why was this merged?

Copy link
Copy Markdown
Contributor

@benpicco benpicco Feb 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't notice it 🙈

Copy link
Copy Markdown
Contributor Author

@derMihai derMihai Feb 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Haha ok I'll fix is

Copy link
Copy Markdown
Member

@maribu maribu Feb 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't worry, shit happens 😉

Merged via the queue into RIOT-OS:master with commit 9501f64 Feb 10, 2024
@benpicco benpicco mentioned this pull request Feb 10, 2024
Closed
@derMihai derMihai mentioned this pull request Feb 10, 2024
Merged
@MrKevinWeiss MrKevinWeiss added this to the Release 2024.04 milestone Apr 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@benpicco benpicco benpicco approved these changes

@maribu maribu maribu left review comments

@fabian18 fabian18 fabian18 left review comments

Assignees

No one assigned

Labels

Area: cpu Area: CPU/MCU ports CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Platform: native Platform: This PR/issue effects the native platform

Projects

None yet

Milestone

Release 2024.04

Development

Successfully merging this pull request may close these issues.

6 participants

@derMihai @riot-ci @benpicco @maribu @fabian18 @MrKevinWeiss