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

FreeRTOS queues for the uart module #1155

Closed
wants to merge 4 commits into from

Conversation

kapacuk
Copy link
Contributor

@kapacuk kapacuk commented Apr 5, 2024

I suggest a small improvement for the uart module. For projects which are using both FreeRTOS and buffered uart, waiting could be done much more efficiently - instead of a busy loop a task could just sleep. Also, with this change it's now possible to call read and write functions with a timeout.

If you agree with this approach it can be extended to other peripherals (SPI, I2C, etc)

@salkinium
Copy link
Member

Do you think it would be possible to extract the data handling from the UART driver entirely? I don't like the queue size configuration via lbuild, I'd rather configure this in C++ and at the same time also chose which queue implementation I'd like ie. a simple FIFO, or a priotity queue, or a RTOS queue, or a DMA transaction etc. Ideally those queue strategies would also be compatible with other data peripherals.

Is that something you would find interesting to work on? The current trajectory will create a giant mess in the HAL if applied at scale.

@kapacuk
Copy link
Contributor Author

kapacuk commented Apr 6, 2024

Is that something you would find interesting to work on? The current trajectory will create a giant mess in the HAL if applied at scale.

I agree that it would be better to do it at C++ level. I'm interested and I would like to work on it, but I will need some guidance (at least initially) because I don't know the codebase that well.

@rleh
Copy link
Member

rleh commented Apr 6, 2024

As @salkinium has already said, I would prefer a solution too, that split the uart driver into the actual driver that interacts with the hardware and a wrapper that handles the buffering.
This would make it very easy to maintain different implementations for buffering, without the unnecessarily complicated Jinja-C++ code mix and simple unit test options for all implementations. (Our unit tests currently only cover code-branches accessible through lbuild default settings.)

@kapacuk
Copy link
Contributor Author

kapacuk commented Apr 6, 2024

OK, I'll give it a try, will do it on a separate branch.

@kapacuk
Copy link
Contributor Author

kapacuk commented Apr 6, 2024

If we configure buffering in C++, then what should we do with the interrupt handler? Depending on the type of queue chosen by the user, the ISR would have to do different things. I don't think it's possible to create a template which would produce an ISR when it is instantiated.

@rleh
Copy link
Member

rleh commented Apr 6, 2024

The Uart driver could provide a function to register an ISR handler which is executed in the ISR, similar to e.g. the attachConfigurationHandler(…) from our SPI device interface.

using IsrHandler = void(*)();

void registerIsrHandler(IsrHandler handler);

and the wrapper registers a callback for the ISR.

@kapacuk
Copy link
Contributor Author

kapacuk commented Apr 6, 2024

The Uart driver could provide a function to register an ISR handler which is executed in the ISR

That means some runtime overhead, but I guess it's unavoidable. It's not that much anyway.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

3 participants