Vajnar commented on code in PR #16491:
URL: https://github.com/apache/nuttx/pull/16491#discussion_r2137255677
##########
arch/risc-v/src/common/espressif/esp_pcnt.c:
##########
Review Comment:
Hi @tmedicci, for the first question, yes that is the case. The probability
is low, but the window of opportunity for obtaining wrong result is non-zero.
The specific test, how to demonstrate the window of opportunity was proposed by
@matiamic and is as follows:
1. Modify the original `esp_pcnt_unit_get_count()` function as follows:
```
flags = spin_lock_irqsave(&priv->lock);
for (volatile int i = 0; i < 53333; i++); // short busy wait
*ret = pcnt_ll_get_count(ctx.dev, priv->unit_id) +
priv->accum_value;
spin_unlock_irqrestore(&priv->lock, flags);
```
2. Run the example `qe` application and rotate the quadrature encoder and
the following sooner or later happens:
```
qe_main: 705. 1960
qe_main: 706. 1967
qe_main: 707. 1973
qe_main: 708. 1980
qe_main: 709. 1986
qe_main: 710. 1993
qe_main: 711. 1000 // !!!
qe_main: 712. 2006
qe_main: 713. 2012
qe_main: 714. 2018
qe_main: 715. 2025
qe_main: 716. 2031
qe_main: 717. 2037
```
For second question, 1) I don't think that's an issue. I did not use
`CAPIOC_HANDLER` function during testing. For 2) it depends on use case, in my
case that was not an issue.
Setting the thresholds higher would certainly help, but is not a solution on
it's own. The window of opportunity for obtaining wrong result would still be
there.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
