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

Merged
merged 4 commits into from
Jun 22, 2024

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.

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.

Apologies for the slow review, I've changed some minor details and polished some things. I will restructure and rebase the commits next.

@salkinium salkinium added the ci:hal Triggers the exhaustive HAL compile CI jobs label Jun 22, 2024
@salkinium salkinium merged commit 8c43f84 into modm-io:develop Jun 22, 2024
39 checks passed
@kapacuk kapacuk deleted the uart_freertos branch June 22, 2024 23:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
advanced 🤯 ci:hal Triggers the exhaustive HAL compile CI jobs feature 🚧
Development

Successfully merging this pull request may close these issues.

None yet

3 participants