Fix uart async test #30836
Fix uart async test #30836
Conversation
The waiting for RX_RDY takes makes timeout in rx since it was enabled with timeout of 50 ms, and the semaphore waits for 100 ms. Signed-off-by: Shlomi Vaknin <[email protected]>
Currently, this test expects the rx_rdy flag to not occur after transmitting 5 bytes, which is not the case in loopback uart test. Signed-off-by: Shlomi Vaknin <[email protected]>
First, increase the timeout in rx_enable, as it takes much more than 10 ms to send 1500 bytes in 115200 baudrate. Second, after sending the 1000 bytes, we expect to receive first rx_rdy and after that tx_done. Currently, this order makes the rx_rdy to be received twice before taking the rx_rdy the semaphore. Since this semaphore has max count of 1 we have a bug. Signed-off-by: Shlomi Vaknin <[email protected]>
github-actions
bot
added
the
area: Tests
|
|
||
| uart_rx_enable(uart_dev, rx_buf, 10, 50); | ||
| zassert_equal(k_sem_take(&rx_rdy, K_MSEC(100)), -EAGAIN, | ||
| zassert_equal(k_sem_take(&rx_rdy, K_MSEC(10)), -EAGAIN, |
There was a problem hiding this comment.
At least on nrf drivers RX timeout was implemented as break between bytes. It allows to report user when some data was received. It seems like more reasonable than timeouting even without any bytes received. In that case 100ms is an arbitrary value.
However, i see that api is not clear about it.
| zassert_equal(k_sem_take(&tx_done, K_MSEC(100)), 0, "TX_DONE timeout"); | ||
| zassert_equal(k_sem_take(&rx_rdy, K_MSEC(100)), 0, "RX_RDY timeout"); | ||
| zassert_equal(k_sem_take(&rx_rdy, K_MSEC(100)), -EAGAIN, | ||
| zassert_equal(k_sem_take(&rx_rdy, K_MSEC(10)), -EAGAIN, |
There was a problem hiding this comment.
100ms was an arbitrary value.
| zassert_equal(k_sem_take(&tx_done, K_MSEC(100)), 0, "TX_DONE timeout"); | ||
| zassert_not_equal(k_sem_take(&rx_rdy, K_MSEC(1000)), 0, | ||
| "RX_RDY timeout"); | ||
| zassert_equal(k_sem_take(&rx_rdy, K_MSEC(100)), 0, "RX_RDY timeout"); |
There was a problem hiding this comment.
when rx is started with SYS_FOREVER_MS timeout, it shall report RX_RDY event only if full buffer is recieved (100 bytes) so there should be no event after 5 bytes.
| memset(long_tx_buf, 1, sizeof(long_tx_buf)); | ||
|
|
||
| uart_rx_enable(uart_dev, long_rx_buf, sizeof(long_rx_buf), 10); | ||
| uart_rx_enable(uart_dev, long_rx_buf, sizeof(long_rx_buf), 150); |
There was a problem hiding this comment.
RX timeout is a inactive time on RX which leads to RX_RDY event.
|
@shlomow i think that test failures you are seeing are caused by different interpretation of rx timeout. I will make a PR to clarify what rx timeout means. |
|
See Line 56 in cc470a2 |
|
@nordic-krch Thanks a lot for the clarifications! I think that I expected the driver to notify the user for available data as soon as it detects it. Since I am using stm32 the hardware has a feature that it sends interrupt when it finds no activity for one uart frame (typically 10 bit times). This api force some latency on users who want to respond fast to uart reception. I guess this is what I missed. Thanks again! |
|
@shlomow some time ago i was also mentioning that milliseconds timeout is too much in some cases when API was initially discussed: #10820 (comment) |
|
I commented there. Thanks again! I am closing this PR. |
2 participants
This pr fixes multiple bugs in the uart async test.