Skip to content

boards: pic32-clicker: check for periph_uart#12744

Closed
basilfx wants to merge 1 commit intoRIOT-OS:masterfrom
basilfx:feature/pic32_clicker_uart
Closed

boards: pic32-clicker: check for periph_uart#12744
basilfx wants to merge 1 commit intoRIOT-OS:masterfrom
basilfx:feature/pic32_clicker_uart

Conversation

@basilfx
Copy link
Member

@basilfx basilfx commented Nov 19, 2019

Contribution description

If there is no dependency on periph_uart, then uart_init is not available. This ensure that this will compile fine if periph_uart is not available.

Testing procedure

Not sure how to test it, but Murdock would fail without this patch in #10741. I also don't have the hardware to test it on real hardware.

Issues/PRs references

#10741

If there is no dependency on periph_uart, then uart_init is not
available. This ensure that this will compile fine if periph_uart
is not available.
@basilfx basilfx added Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) Area: boards Area: Board ports labels Nov 19, 2019

/* intialise UART used for debug (printf) */
#ifdef DEBUG_VIA_UART
#if MODULE_PERIPH_UART && defined(DEBUG_VIA_UART)
Copy link
Contributor

Choose a reason for hiding this comment

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

IIUC, the whole stuff should be removed.

@aabadie
Copy link
Contributor

aabadie commented Nov 19, 2019

@basilfx, the right fix is to do the same as what was done in #12256 but for the pic32-clicker.

@francois-berder
Copy link
Contributor

@basilfx, the right fix is to do the same as what was done in #12256 but for the pic32-clicker.

Indeed that would be better. For now, UART is broken on pic32-clicker.
I was working on refactoring UART (available at https://github.com/francois-berder/RIOT/tree/pic32-uart) but I did not have enough time to complete my work. That would fix the issue you are describing. I do intend to finish this refactoring in the next few weeks and open a PR.

@basilfx
Copy link
Member Author

basilfx commented Nov 21, 2019

@francois-berder @aabadie Would it then be OK to merge this in the mean time? It would otherwise block #10741, and the change itself isn't that big :-)

@francois-berder
Copy link
Contributor

@basilfx OK for me. I will remove these lines anyway in my PR.

@kaspar030
Copy link
Contributor

Ok, then let's go!

@kaspar030 kaspar030 added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Nov 21, 2019
Copy link
Contributor

@kaspar030 kaspar030 left a comment

Choose a reason for hiding this comment

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

ACK.

@aabadie
Copy link
Contributor

aabadie commented Nov 21, 2019

I disagree with the proposed fix here: quick and dirty. It should take 5 minutes to handle this correctly, at least like it's done for the pic32-wifire. If none of you wants to do it, I can open a PR.

"Let's do it right this time" (TM)

@kaspar030
Copy link
Contributor

I disagree with the proposed fix here: quick and dirty. It should take 5 minutes to handle this correctly, at least like it's done for the pic32-wifire. If none of you wants to do it, I can open a PR.

I'd say feel free, but I assumed that @francois-berder is already working on that:

@basilfx OK for me. I will remove these lines anyway in my PR.

So yes, this is a dirty quick fix so we can move on with #10741, with a proper fix in the queue.

@aabadie
Copy link
Contributor

aabadie commented Nov 21, 2019

I do intend to finish this refactoring in the next few weeks and open a PR.

If you don't mind @francois-berder, I'll spend these 5 minutes now and will avoid us to wait a few weeks. Hope this is fine.

@aabadie
Copy link
Contributor

aabadie commented Nov 21, 2019

Please consider #12768 instead of this PR.

@basilfx basilfx deleted the feature/pic32_clicker_uart branch October 20, 2020 19:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area: boards Area: Board ports CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants