Skip to content

[PR 496 rework] Disable USART IRQ in uart_write and uart_debug_write #502

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 8 commits into from
Apr 19, 2019
Merged

[PR 496 rework] Disable USART IRQ in uart_write and uart_debug_write #502

merged 8 commits into from
Apr 19, 2019

Conversation

fpistm
Copy link
Member

@fpistm fpistm commented Apr 17, 2019

This PR is a rework of #496 and fixes issues #494 and #467

It hardens and enhances the original PR.
I've tested several config and it works with Arduino Serial monitor.
Note that if the same U(S)ART is used for Serial and printf() some bytes could be lost due to IRQ disabling.

@ppescher and @benwaffle could you review and test it on your side

@fpistm fpistm added enhancement New feature or request help wanted 🙏 Extra attention is needed fix 🩹 Bug fix labels Apr 17, 2019
@fpistm fpistm added this to the 1.6.0 milestone Apr 17, 2019
@fpistm fpistm self-assigned this Apr 17, 2019
@fpistm fpistm mentioned this pull request Apr 17, 2019
Closed
@benwaffle
Copy link
Contributor

benwaffle commented Apr 17, 2019 •
edited
Loading

Note that if the same U(S)ART is used for Serial and printf() some bytes could be lost due to IRQ disabling.

Can you explain under what conditions this would happen?

@fpistm
Copy link
Member Author

fpistm commented Apr 18, 2019 •
edited
Loading

Note that if the same U(S)ART is used for Serial and printf() some bytes could be lost due to IRQ disabling.

Can you explain under what conditions this would happen?

In fact by default, the printf will send on the same U(S)ART used for the generic instance.
In this case, when the first Serial.available is true, then printf is called and disabled the U(S)ART interrupt then in this window the data is simply lost until the interrupt was set and the HAL_UART_ErrorCallback is called and restart receive interrupt.

Defining DEBUG_UART USARTx allow to use an other UART to use for printf, in that case no data is lost.

fpistm and others added 6 commits April 19, 2019 08:12
@fpistm
[UART] Search only once the debug uart
0c2dd3b 
Signed-off-by: Frederic Pillon <[email protected]>
@fpistm
[UART] Reorder serial_t field to avoid padding
b1fec1a 
Signed-off-by: Frederic.Pillon <[email protected]>
@ppescher @fpistm
[UART] Fix serial port full-duplex bug
9dbb03b 
When a receive IT fires while the HAL handle is locked, the receive callback fails to re-enable the IT and the serial port stops receiving anymore. This can happen when transmit IT is enabled outside the IRQ in HardwareSerial::write(), following a buffer full condition that disables transmit IT in HAL_UART_TxCpltCallback().
Solution is to temporarily disable the IRQ while the interrupt enable flags are modified.
@ppescher @fpistm
[UART] Fix error callback (clear flags and restart receive IT)
b9b7a9d
@benwaffle @fpistm
[UART] Disable interrupt in uart_debug_write
a7a1690
@fpistm
[UART] Add private function to get serial_t obj
5c3fb05 
Signed-off-by: Frederic.Pillon <[email protected]>
@fpistm
Copy link
Member Author

fpistm commented Apr 19, 2019

I've added some code enhancement which allow to save space and avoid loop to search the right serial_t obj.

@fpistm
[UART] Simplify code using get_serial_obj()
b00bd29 
Remove of 4 arrays which are no more needed.
Add rx/tx_callback in the serial_t.

Signed-off-by: Frederic.Pillon <[email protected]>
@fpistm
[UART] Reduce serial_t size
9dc4505 
baudrate, databits, stopbits and parity are used only
during init.
So, they are no reason to save them in the structure.
This allow to save space.

Signed-off-by: Frederic.Pillon <[email protected]>
@fpistm fpistm changed the title PR 496 rework [PR 496 rework] Disable USART IRQ in uart_write and uart_debug_write Apr 19, 2019
@fpistm fpistm merged commit 4c29c3a into stm32duino:master Apr 19, 2019
@fpistm fpistm deleted the PR-496-rework branch April 19, 2019 11:54
This was referenced Apr 19, 2019
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@benwaffle benwaffle benwaffle left review comments

Assignees

@fpistm fpistm

Labels
enhancement New feature or request fix 🩹 Bug fix help wanted 🙏 Extra attention is needed
Projects
None yet
Milestone
1.6.0
Development

Successfully merging this pull request may close these issues.
3 participants
@fpistm @benwaffle @ppescher