[GitHub] [incubator-nuttx] MTres19 commented on a change in pull request #2641: can: Cleanup usage of soft fifo semaphore

[GitBox <gi...@apache.org>]

MTres19 commented on a change in pull request #2641:
URL: https://github.com/apache/incubator-nuttx/pull/2641#discussion_r774280480



##########
File path: drivers/can/can.c
##########
@@ -609,24 +609,24 @@ static ssize_t can_read(FAR struct file *filep, FAR char *buffer,
 
       fifo = &reader->fifo;
 
-      while (fifo->rx_head == fifo->rx_tail)
+      if (filep->f_oflags & O_NONBLOCK)

Review comment:
       Good point; I did it this way so that `rx_sem` would have clearly defined semantics; i.e. 1 = ready to read, 0 = not ready to read. Right now, if I only replaced the `nxsem_trywait()` here, a subsequent call to `can_read()` would return the error "RX FIFO sem posted but FIFO is empty."
   
   Obviously those semantics duplicate the information you get from checking if head == tail. It's just what I thought of at the time. Alternatively, we could only post the semaphore in `can_receive()` if at least one thread is waiting on it (i.e. sem_get_value yields a number < 0). Then `rx_sem` would truly be used exclusively for waiting and not duplicate any information. Maybe that would be better? I'd need to put my CAN setup back together of course to test it, in that case.




-- 
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