[NUC472/M453] Fix serial error with sync/async calls interlaced #4241
Conversation
|
Can you please remove the merge commit and rebase ?
I dont see this one in the changeset? |
|
How was this tested? It might be beneficial for bigger changes to add more details into the commit message (for instance the serial fix changes lot of lines, and introducing new internal API, plus explaining what was the bug that this commit is fixing). |
Serial implementation uses different vector handlers for sync/async calls respectively. The issue can be reproduced with the following flow: 1. Register sync mode callback with Serial.attach(). 2. Sync call with Serial.putc()/getc(). 3. Change to async call with Serial.write()/read(). 4. Change back to sync call with Serial.putc()/getc(). Now, vector handller is still for async mode, not for sync mode. To fix it: 1. Introduce internal function serial_enable_interrupt() for both sync/async vector handler enable/disable. Original HAL function serial_irq_set() is reduced to call it for sync mode vector handler enable/disable. 2. Introduce internal function serial_rollback_interrupt() to roll back sync mode vector handler at end of async transfer.
|
@0xc0170 Please ignore the PR comment below. The related commit has been merged into master before.
I add comment for this commit.
|
Can you remove this commit please? |
@0xc0170 I remove the commit. |
| } | ||
|
|
||
| int serial_getc(serial_t *obj) | ||
| { | ||
| // TODO: Fix every byte access requires accompaniness of one interrupt. This degrades performance much. | ||
| // TODO: Fix every byte access requires accompaniment of one interrupt. This has side effect of performance degradation. |
There was a problem hiding this comment.
Why do we still have TODOs in the code?
| @@ -394,11 +365,12 @@ int serial_getc(serial_t *obj) | |||
|
|
|||
| void serial_putc(serial_t *obj, int c) | |||
| { | |||
| // TODO: Fix every byte access requires accompaniness of one interrupt. This degrades performance much. | |||
| // TODO: Fix every byte access requires accompaniment of one interrupt. This has side effect of performance degradation. | |||
|
@theotherjimmy I change the comment for serial_getc/serial_putc by replacing TODO with NOTE to avoid confusion. |
|
Thanks @ccli8 |
|
Restarting travis |
|
/morph test |
Result: SUCCESSYour command has finished executing! Here's what you wrote!
OutputAll builds and test passed! |
Description
This PR includes the following bugfixes for NUMAKER_PFM_NUC472/NUMAKER_PFM_M453 targets.