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

BufferedUart template and its specialisation for FreeRTOS queues #1156

Open
wants to merge 40 commits into
base: develop
Choose a base branch
from

Conversation

kapacuk
Copy link
Contributor

@kapacuk kapacuk commented Apr 19, 2024

As discussed in PR#1155, here is my attempt to create a wrapper around the UartHal driver which handles the buffering. Not sure if it's what you had in mind, I'm open to suggestions on how to improve it.

I kept the original buffering driver with minor tweaks and created a new templated class BufferedUart. It duplicates some code from the old driver, but I think that's a small price to pay for backward compatibility.

Copy link
Member

@salkinium salkinium left a comment

Choose a reason for hiding this comment

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

Very nice, I'm ok with breaking changes so don't hesitate to generalize the buffers even further.

src/modm/platform/uart/stm32/uart.cpp.in Outdated Show resolved Hide resolved
src/modm/platform/uart/stm32/uart.hpp.in Outdated Show resolved Hide resolved
src/modm/platform/uart/stm32/uart.hpp.in Outdated Show resolved Hide resolved
src/modm/platform/uart/stm32/uart_hal.hpp.in Outdated Show resolved Hide resolved
src/modm/platform/uart/stm32/module.lb Outdated Show resolved Hide resolved
src/modm/platform/uart/stm32/freertos.hpp Outdated Show resolved Hide resolved
src/modm/platform/uart/stm32/freertos.hpp Outdated Show resolved Hide resolved
examples/black_pill_f401/uart_freertos/main.cpp Outdated Show resolved Hide resolved
ext/aws/module.lb Outdated Show resolved Hide resolved
src/modm/platform/uart/stm32/uart.hpp.in Outdated Show resolved Hide resolved
src/modm/platform/uart/stm32/uart_hal.hpp.in Show resolved Hide resolved
@kapacuk
Copy link
Contributor Author

kapacuk commented Apr 29, 2024

Sorry it took me so long, that was a bigger change than I expected.. I think I'm done, it's ready for a review.

@kapacuk kapacuk requested a review from salkinium April 29, 2024 21:39
@salkinium
Copy link
Member

Sorry it took me so long

No pressure! I'm very happy with this work, thank you for taking this on! I will review and test this in depth on the weekend.

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.

None yet

3 participants