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

uart_native_tty: Emulate an interrupt driven uart #68857

Merged
merged 1 commit into from
Feb 27, 2024

Conversation

zagor
Copy link
Contributor

@zagor zagor commented Feb 12, 2024

Emulate SERIAL_SUPPORT_INTERRUPT for UART_NATIVE_TTY, using a thread that
polls the tty and invokes the callback.

This allows interrupt-driven subsystems such as modbus to use a native tty,
which is useful for testing purposes.

@zephyrbot zephyrbot added area: UART Universal Asynchronous Receiver-Transmitter area: native port Host native arch port (native_sim) labels Feb 12, 2024
@zagor zagor force-pushed the native-tty-emulate-irq branch from 4ccdbdd to 052072e Compare February 12, 2024 12:10
Copy link
Member

@aescolar aescolar left a comment

Choose a reason for hiding this comment

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

A few nits, and a more significant comment.

boards/posix/native_sim/doc/index.rst Outdated Show resolved Hide resolved
boards/posix/native_sim/doc/index.rst Show resolved Hide resolved
drivers/serial/uart_native_tty.c Outdated Show resolved Hide resolved
drivers/serial/uart_native_tty.c Show resolved Hide resolved
drivers/serial/uart_native_tty.c Outdated Show resolved Hide resolved
drivers/serial/uart_native_tty.c Outdated Show resolved Hide resolved
drivers/serial/uart_native_tty.c Show resolved Hide resolved
drivers/serial/uart_native_tty.c Show resolved Hide resolved
drivers/serial/uart_native_tty.c Show resolved Hide resolved
Emulate SERIAL_SUPPORT_INTERRUPT for UART_NATIVE_TTY, using a thread that
polls the tty and invokes the callback.

This allows interrupt-driven subsystems such as modbus to use a native tty,
which is useful for testing purposes.

Signed-off-by: Björn Stenberg <[email protected]>
@zagor zagor force-pushed the native-tty-emulate-irq branch from 052072e to 64ea668 Compare February 26, 2024 08:38
@zagor
Copy link
Contributor Author

zagor commented Feb 26, 2024

Rebased and updated this thread-based patch. My schedule filled up faster than I hoped. I propose merging this as a first step and come back at a later time with an event-based version.

@aescolar
Copy link
Member

Thanks @zagor

I propose merging this as a first step and come back at a later time with an event-based version.

That is fair, the other proposal is certainly much more work. I will do another review pass to this one.

@aescolar
Copy link
Member

CC @MarkoSagadin

Copy link
Member

@aescolar aescolar left a comment

Choose a reason for hiding this comment

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

+1 for native side integration. I defer to @dcpleung for the UART API integration

@aescolar aescolar assigned dcpleung and unassigned aescolar Feb 26, 2024
@aescolar aescolar merged commit 8cdcb2c into zephyrproject-rtos:main Feb 27, 2024
23 checks passed
Comment on lines +578 to +579
This driver only supports poll mode and interrupt mode. Async mode is not
supported.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think the first paragraph of this section now also requires updating. It currently says "With this driver an application can use the polling UART API (uart_poll_out, uart_poll_in) to write and read characters to and from a connected serial port device."
cc @aescolar

Copy link
Collaborator

Choose a reason for hiding this comment

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

BTW, would it make sense / be possible to have the same approach for the uart_native_pty driver?

Copy link
Member

Choose a reason for hiding this comment

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

@kartben Sure, I guess nobody though it worthwhile so far

} else {
k_sleep(K_MSEC(1));
}
} else if (data->tx_irq_enabled) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it possible that this should be an 'if' instead of an 'else if'? rx_irq_enabled is only set false while reading the receive buffer, so data could never be transmitted in my case.

Copy link
Member

Choose a reason for hiding this comment

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

It seems like a bug. @ct-fk Would you like to send a PR with the fix?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: native port Host native arch port (native_sim) area: UART Universal Asynchronous Receiver-Transmitter
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants