You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2022/02/26 18:55:20 UTC
[GitHub] [incubator-nuttx] xiaoxiang781216 opened a new pull request #5624: joystick: Fix the event lose between the invocation of poll
xiaoxiang781216 opened a new pull request #5624:
URL: https://github.com/apache/incubator-nuttx/pull/5624
## Summary
- input/ajoystick: Fix the event lose between the invocation of poll
- input/djoystick: Fix the event lose between the invocation of poll
## Impact
joystick driver
## Testing
Pass CI
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a change in pull request #5624: joystick/buttons: Fix the event lose between the invocation of poll
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on a change in pull request #5624:
URL: https://github.com/apache/incubator-nuttx/pull/5624#discussion_r816944734
##########
File path: drivers/input/ajoystick.c
##########
@@ -722,6 +720,19 @@ static int ajoy_poll(FAR struct file *filep, FAR struct pollfd *fds,
opriv->ao_fds[i] = fds;
fds->priv = &opriv->ao_fds[i];
+
+ /* Report if the event is pending */
+
+ if (opriv->ao_pollpending)
+ {
+ fds->revents |= (fds->events & POLLIN);
+ if (fds->revents != 0)
+ {
+ iinfo("Report events: %02x\n", fds->revents);
+ nxsem_post(fds->sem);
Review comment:
Ok. But in compare to `touchscreen_upper.c` case we still consume `opriv->ao_fds[i] = fds;`. I think this should be fixed. `opriv->ao_fds[i] = fds;` should be consumed only `if (fds->revents == 0)`.
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a change in pull request #5624: joystick/buttons: Fix the event lose between the invocation of poll
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on a change in pull request #5624:
URL: https://github.com/apache/incubator-nuttx/pull/5624#discussion_r817030722
##########
File path: drivers/input/ajoystick.c
##########
@@ -722,6 +720,19 @@ static int ajoy_poll(FAR struct file *filep, FAR struct pollfd *fds,
opriv->ao_fds[i] = fds;
fds->priv = &opriv->ao_fds[i];
+
+ /* Report if the event is pending */
+
+ if (opriv->ao_pollpending)
+ {
+ fds->revents |= (fds->events & POLLIN);
+ if (fds->revents != 0)
+ {
+ iinfo("Report events: %02x\n", fds->revents);
+ nxsem_post(fds->sem);
Review comment:
> No, the protocol require that fds must save into ao_fds if it return true regardless the semaphore post or not. You can see touchscreen_upper.c do the same thing:
> https://github.com/apache/incubator-nuttx/blob/master/drivers/input/touchscreen_upper.c#L319-L324
> poll will call ajoy_poll with setup == false immediately after return.
Thank you, I missed that.
But what I see in the touchscreen example is that fd sem count is checked in prior to sem post and all poll waiters are notified. But in current changes it seems like behavior is different, isn't it?
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a change in pull request #5624: joystick/buttons: Fix the event lose between the invocation of poll
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on a change in pull request #5624:
URL: https://github.com/apache/incubator-nuttx/pull/5624#discussion_r817603333
##########
File path: drivers/input/ajoystick.c
##########
@@ -722,6 +720,19 @@ static int ajoy_poll(FAR struct file *filep, FAR struct pollfd *fds,
opriv->ao_fds[i] = fds;
fds->priv = &opriv->ao_fds[i];
+
+ /* Report if the event is pending */
+
+ if (opriv->ao_pollpending)
+ {
+ fds->revents |= (fds->events & POLLIN);
+ if (fds->revents != 0)
+ {
+ iinfo("Report events: %02x\n", fds->revents);
+ nxsem_post(fds->sem);
Review comment:
What about notification logic? This is still not answered. In `touch_poll` we have:
```
for (i = 0; i < CONFIG_INPUT_TOUCHSCREEN_NPOLLWAITERS; i++)
{
if (NULL == upper->fds[i])
{
upper->fds[i] = fds;
fds->priv = &upper->fds[i];
break;
}
}
...
if (eventset)
{
touch_notify(upper, eventset);
}
```
and `touch_notify` iterates `for (i = 0; i < CONFIG_INPUT_TOUCHSCREEN_NPOLLWAITERS; i++)` again,
With this logic we just notify until the first free entry met.
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on pull request #5624: joystick/buttons: Fix the event lose between the invocation of poll
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on pull request #5624:
URL: https://github.com/apache/incubator-nuttx/pull/5624#issuecomment-1053606572
Please update the PR description with a use case that you are trying to fix. The changes seems to be not straight forward to me. I need to explanation to understand what is doing on in the system
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #5624: joystick/buttons: Fix the event lose between the invocation of poll
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #5624:
URL: https://github.com/apache/incubator-nuttx/pull/5624#discussion_r817006877
##########
File path: drivers/input/ajoystick.c
##########
@@ -722,6 +720,19 @@ static int ajoy_poll(FAR struct file *filep, FAR struct pollfd *fds,
opriv->ao_fds[i] = fds;
fds->priv = &opriv->ao_fds[i];
+
+ /* Report if the event is pending */
+
+ if (opriv->ao_pollpending)
+ {
+ fds->revents |= (fds->events & POLLIN);
+ if (fds->revents != 0)
+ {
+ iinfo("Report events: %02x\n", fds->revents);
+ nxsem_post(fds->sem);
Review comment:
No, the protocol require that fds must save into ao_fds if it return true regardless the semaphore post or not. You can see touchscreen_upper.c do the same thing:
https://github.com/apache/incubator-nuttx/blob/master/drivers/input/touchscreen_upper.c#L319-L324
poll will call ajoy_poll with setup == false immediately after return.
##########
File path: drivers/input/button_upper.c
##########
@@ -759,6 +767,19 @@ static int btn_poll(FAR struct file *filep, FAR struct pollfd *fds,
opriv->bo_fds[i] = fds;
fds->priv = &opriv->bo_fds[i];
+
+ /* Report if the event is pending */
+
+ if (opriv->bo_pending)
+ {
+ fds->revents |= (fds->events & POLLIN);
+ if (fds->revents != 0)
+ {
+ iinfo("Report events: %02x\n", fds->revents);
+ nxsem_post(fds->sem);
Review comment:
ditto
##########
File path: drivers/input/djoystick.c
##########
@@ -713,6 +711,17 @@ static int djoy_poll(FAR struct file *filep, FAR struct pollfd *fds,
opriv->do_fds[i] = fds;
fds->priv = &opriv->do_fds[i];
+
+ if (opriv->do_pollpending)
+ {
+ fds->revents |= (fds->events & POLLIN);
Review comment:
ditto
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #5624: joystick/buttons: Fix the event lose between the invocation of poll
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #5624:
URL: https://github.com/apache/incubator-nuttx/pull/5624#discussion_r816878380
##########
File path: drivers/input/button_upper.c
##########
@@ -97,6 +97,7 @@ struct btn_open_s
*/
FAR struct pollfd *bo_fds[CONFIG_INPUT_BUTTONS_NPOLLWAITERS];
+ bool bo_pending;
Review comment:
Done.
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 edited a comment on pull request #5624: joystick/buttons: Fix the event lose between the invocation of poll
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 edited a comment on pull request #5624:
URL: https://github.com/apache/incubator-nuttx/pull/5624#issuecomment-1053616123
> Please update the PR description with a use case that you are trying to fix. The changes seems to be not straight forward to me. I need to explanation to understand what is doing on in the system
@pkarashchenko the case is simple, you can see that xxx_sample could wake up poller if the interrupt happen while the caller is waiting inside select/poll function. But, the event will lose if the caller is busy to do other thing and then can't sleep in select/poll while the interrupt happen. This PR try to fix this issue by adding a pending flag:
1. Set the pending flag in the interrupt handle
2. Check the pending flag in poll setup
3. Clear the pending flag after the caller read the new state
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a change in pull request #5624: joystick/buttons: Fix the event lose between the invocation of poll
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on a change in pull request #5624:
URL: https://github.com/apache/incubator-nuttx/pull/5624#discussion_r816945656
##########
File path: drivers/input/button_upper.c
##########
@@ -759,6 +767,19 @@ static int btn_poll(FAR struct file *filep, FAR struct pollfd *fds,
opriv->bo_fds[i] = fds;
fds->priv = &opriv->bo_fds[i];
+
+ /* Report if the event is pending */
+
+ if (opriv->bo_pending)
+ {
+ fds->revents |= (fds->events & POLLIN);
+ if (fds->revents != 0)
+ {
+ iinfo("Report events: %02x\n", fds->revents);
+ nxsem_post(fds->sem);
Review comment:
same as in `drivers/input/ajoystick.c`
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a change in pull request #5624: joystick/buttons: Fix the event lose between the invocation of poll
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on a change in pull request #5624:
URL: https://github.com/apache/incubator-nuttx/pull/5624#discussion_r817695866
##########
File path: drivers/input/ajoystick.c
##########
@@ -722,6 +720,19 @@ static int ajoy_poll(FAR struct file *filep, FAR struct pollfd *fds,
opriv->ao_fds[i] = fds;
fds->priv = &opriv->ao_fds[i];
+
+ /* Report if the event is pending */
+
+ if (opriv->ao_pollpending)
+ {
+ fds->revents |= (fds->events & POLLIN);
+ if (fds->revents != 0)
+ {
+ iinfo("Report events: %02x\n", fds->revents);
+ nxsem_post(fds->sem);
Review comment:
Got it! LGTM!
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on pull request #5624: joystick/buttons: Fix the event lose between the invocation of poll
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on pull request #5624:
URL: https://github.com/apache/incubator-nuttx/pull/5624#issuecomment-1053618571
> > Please update the PR description with a use case that you are trying to fix. The changes seems to be not straight forward to me. I need to explanation to understand what is doing on in the system
>
> @pkarashchenko the case is simple, you can see that xxx_sample could wake up poller if the interrupt happen while the caller is waiting inside select/poll function. But, the event will lose if the caller is busy to do other thing and then can't sleep in select/poll while the interrupt happen. This PR try to fix this issue by adding a pending flag:
>
> 1. Set the pending flag in the interrupt handle
>
> 2. Check the pending flag in poll setup
>
> 3. Clear the pending flag after the caller read the new state
Ok. So you used a flag instead of posting a semaphore from interrupt and resetting it during `read`. Now it is not clear. Thank you
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #5624: joystick/buttons: Fix the event lose between the invocation of poll
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #5624:
URL: https://github.com/apache/incubator-nuttx/pull/5624#discussion_r817035988
##########
File path: drivers/input/ajoystick.c
##########
@@ -722,6 +720,19 @@ static int ajoy_poll(FAR struct file *filep, FAR struct pollfd *fds,
opriv->ao_fds[i] = fds;
fds->priv = &opriv->ao_fds[i];
+
+ /* Report if the event is pending */
+
+ if (opriv->ao_pollpending)
+ {
+ fds->revents |= (fds->events & POLLIN);
+ if (fds->revents != 0)
+ {
+ iinfo("Report events: %02x\n", fds->revents);
+ nxsem_post(fds->sem);
Review comment:
Since the semaphore used by poll always initialize before xxx_poll, so there is no real difference here. But for the semaphore used to wake up xxx_read, post without check may wake up read thread unexpectedly.
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #5624: joystick/buttons: Fix the event lose between the invocation of poll
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #5624:
URL: https://github.com/apache/incubator-nuttx/pull/5624#discussion_r816869186
##########
File path: drivers/input/ajoystick.c
##########
@@ -194,19 +194,8 @@ static void ajoy_enable(FAR struct ajoy_upperhalf_s *priv)
for (opriv = priv->au_open; opriv; opriv = opriv->ao_flink)
{
- /* Are there any poll waiters? */
-
- for (i = 0; i < CONFIG_INPUT_AJOYSTICK_NPOLLWAITERS; i++)
- {
- if (opriv->ao_fds[i])
- {
- /* Yes.. OR in the poll event buttons */
-
- press |= opriv->ao_pollevents.ap_press;
- release |= opriv->ao_pollevents.ap_release;
- break;
- }
- }
+ press |= opriv->ao_pollevents.ap_press;
Review comment:
Yes, if the user config the monitor setting through AJOYIOC_POLLEVENTS. Otherwise, the event happen before user call poll/select will lose.
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 edited a comment on pull request #5624: joystick/buttons: Fix the event lose between the invocation of poll
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 edited a comment on pull request #5624:
URL: https://github.com/apache/incubator-nuttx/pull/5624#issuecomment-1053616123
> Please update the PR description with a use case that you are trying to fix. The changes seems to be not straight forward to me. I need to explanation to understand what is doing on in the system
@pkarashchenko the case is simple, you can see that xxx_sample could wake up poller if the interrupt happen while the caller is waiting inside select/poll function. But, the event will lose if the caller is busy to do other thing and then can't sleep in select/poll while the interrupt happen. This PR try to fix this issue.
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #5624: joystick: Fix the event lose between the invocation of poll
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #5624:
URL: https://github.com/apache/incubator-nuttx/pull/5624#issuecomment-1053350689
> @xiaoxiang781216 please do real test. It will avoid we receive complaint from users saying something is not working :-)
We test with button_upper.c, joystick driver apply with the similar change.
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko merged pull request #5624: joystick/buttons: Fix the event lose between the invocation of poll
Posted by GitBox <gi...@apache.org>.
pkarashchenko merged pull request #5624:
URL: https://github.com/apache/incubator-nuttx/pull/5624
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] acassis commented on pull request #5624: joystick: Fix the event lose between the invocation of poll
Posted by GitBox <gi...@apache.org>.
acassis commented on pull request #5624:
URL: https://github.com/apache/incubator-nuttx/pull/5624#issuecomment-1052595635
@xiaoxiang781216 please do real test. It will avoid we receive complaint from users saying something is not working :-)
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #5624: joystick/buttons: Fix the event lose between the invocation of poll
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #5624:
URL: https://github.com/apache/incubator-nuttx/pull/5624#issuecomment-1053616123
> Please update the PR description with a use case that you are trying to fix. The changes seems to be not straight forward to me. I need to explanation to understand what is doing on in the system
The case is simple, you can see that xxx_sample could wake up poller if the interrupt happen while the caller is waiting inside select/poll function. But, the event will lose if the caller is busy to do other thing and then can't sleep in select/poll while the interrupt happen. This PR try to fix this issue.
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #5624: joystick/buttons: Fix the event lose between the invocation of poll
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #5624:
URL: https://github.com/apache/incubator-nuttx/pull/5624#discussion_r816872225
##########
File path: drivers/input/ajoystick.c
##########
@@ -722,6 +720,19 @@ static int ajoy_poll(FAR struct file *filep, FAR struct pollfd *fds,
opriv->ao_fds[i] = fds;
fds->priv = &opriv->ao_fds[i];
+
+ /* Report if the event is pending */
+
+ if (opriv->ao_pollpending)
+ {
+ fds->revents |= (fds->events & POLLIN);
+ if (fds->revents != 0)
+ {
+ iinfo("Report events: %02x\n", fds->revents);
+ nxsem_post(fds->sem);
Review comment:
Notify the caller there is event pending for read to avoid the caller block in poll/select. This is a normal action in any xxx_poll(e.g. https://github.com/apache/incubator-nuttx/blob/master/drivers/input/touchscreen_upper.c#L334-L342).
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #5624: joystick/buttons: Fix the event lose between the invocation of poll
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #5624:
URL: https://github.com/apache/incubator-nuttx/pull/5624#discussion_r817692400
##########
File path: drivers/input/ajoystick.c
##########
@@ -722,6 +720,19 @@ static int ajoy_poll(FAR struct file *filep, FAR struct pollfd *fds,
opriv->ao_fds[i] = fds;
fds->priv = &opriv->ao_fds[i];
+
+ /* Report if the event is pending */
+
+ if (opriv->ao_pollpending)
+ {
+ fds->revents |= (fds->events & POLLIN);
+ if (fds->revents != 0)
+ {
+ iinfo("Report events: %02x\n", fds->revents);
+ nxsem_post(fds->sem);
Review comment:
Do you want to notify the existed entry? it already done at ajoy_sample:
https://github.com/apache/incubator-nuttx/pull/5624/files#diff-5803ad0a8a93175597d14948ffe9a46f4e1ad28b59d28bcb568f56553b1fa8f7R305
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #5624: joystick/buttons: Fix the event lose between the invocation of poll
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #5624:
URL: https://github.com/apache/incubator-nuttx/pull/5624#discussion_r816865704
##########
File path: drivers/input/ajoystick.c
##########
@@ -100,6 +100,7 @@ struct ajoy_open_s
* driver events.
*/
+ bool ao_pollpending;
Review comment:
Input device is different from net device, the broadcast semantics is normally used by the input device, which mean that multiple reader will receive the same event from one hardware device.
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a change in pull request #5624: joystick/buttons: Fix the event lose between the invocation of poll
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on a change in pull request #5624:
URL: https://github.com/apache/incubator-nuttx/pull/5624#discussion_r816945929
##########
File path: drivers/input/djoystick.c
##########
@@ -713,6 +711,17 @@ static int djoy_poll(FAR struct file *filep, FAR struct pollfd *fds,
opriv->do_fds[i] = fds;
fds->priv = &opriv->do_fds[i];
+
+ if (opriv->do_pollpending)
+ {
+ fds->revents |= (fds->events & POLLIN);
Review comment:
same as in `drivers/input/ajoystick.c`
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a change in pull request #5624: joystick/buttons: Fix the event lose between the invocation of poll
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on a change in pull request #5624:
URL: https://github.com/apache/incubator-nuttx/pull/5624#discussion_r816697411
##########
File path: drivers/input/ajoystick.c
##########
@@ -722,6 +720,19 @@ static int ajoy_poll(FAR struct file *filep, FAR struct pollfd *fds,
opriv->ao_fds[i] = fds;
fds->priv = &opriv->ao_fds[i];
+
+ /* Report if the event is pending */
+
+ if (opriv->ao_pollpending)
+ {
+ fds->revents |= (fds->events & POLLIN);
+ if (fds->revents != 0)
+ {
+ iinfo("Report events: %02x\n", fds->revents);
+ nxsem_post(fds->sem);
Review comment:
Why do we need to `nxsem_post(fds->sem);` here?
##########
File path: drivers/input/ajoystick.c
##########
@@ -194,19 +194,8 @@ static void ajoy_enable(FAR struct ajoy_upperhalf_s *priv)
for (opriv = priv->au_open; opriv; opriv = opriv->ao_flink)
{
- /* Are there any poll waiters? */
-
- for (i = 0; i < CONFIG_INPUT_AJOYSTICK_NPOLLWAITERS; i++)
- {
- if (opriv->ao_fds[i])
- {
- /* Yes.. OR in the poll event buttons */
-
- press |= opriv->ao_pollevents.ap_press;
- release |= opriv->ao_pollevents.ap_release;
- break;
- }
- }
+ press |= opriv->ao_pollevents.ap_press;
Review comment:
So now we `|=` even in there are no waiters?
##########
File path: drivers/input/ajoystick.c
##########
@@ -100,6 +100,7 @@ struct ajoy_open_s
* driver events.
*/
+ bool ao_pollpending;
Review comment:
Why this reside here and not in `struct btn_upperhalf_s`? I'm just thinking from the analogy with network sockets. If data arrive and there are many waiters, then only one can read the data and not all. Or I'm missing something?
##########
File path: drivers/input/button_upper.c
##########
@@ -97,6 +97,7 @@ struct btn_open_s
*/
FAR struct pollfd *bo_fds[CONFIG_INPUT_BUTTONS_NPOLLWAITERS];
+ bool bo_pending;
Review comment:
```suggestion
bool bo_pending;
FAR struct pollfd *bo_fds[CONFIG_INPUT_BUTTONS_NPOLLWAITERS];
```
Just for consistency with other files
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org