Refactor tcpsocket tests to use sigio and not to wait #9794
Conversation
| continue; | ||
| } else if (recvd < 0) { | ||
| TEST_FAIL(); | ||
| int recvd = sock.recv(&(tcp_global::rx_buffer[bytes2recv_total - bytes2recv]), bytes2recv); |
There was a problem hiding this comment.
I don't think you should remove the while() loop.
You are supposed to loop UNTIl you get the WOULD_BLOCK.
Only then you are guaranteed to receive sigio() event.
There was a problem hiding this comment.
Not sure I understand correctly... The tcpsocket_echotest_nonblock_receive() function is called from sigio handler. I thought it is supposed to trigger on the event to be sure the data is there waiting to be read?
Perhaps I am missing something about how the sigio works?
There was a problem hiding this comment.
For example, if the driver received 2kB of data.. sends sigio() event to notify that there is new data to be read, and you read 1kB. It does not send another event, because there was not change in the state.. the device is still readable.
There was a problem hiding this comment.
sigio() notifies that there is change of state.
readable -> not readable or
not readable -> readable or
connected -> disconnected or
connecting -> connected or
new connections acceptable -> no new connections
etc...
There was a problem hiding this comment.
Now I see. Then I will also fix the tlssocket echo tests as I made the same mistake there some time ago.
There was a problem hiding this comment.
Brough the loop back into the receiver both for tcp and tls socket tests. Thanks for pointing this out!
ciarmcom
requested review from
AriParkkila,
KariHaapalehto,
mtomczykmobica,
SeppoTakalo,
tymoteuszblochmobica,
VeijoPesonen and
a team
|
@michalpasztamobica, thank you for your changes. |
michalpasztamobica
force-pushed
the
greentea_tcpsocket_sigio
branch
from
63e2792 to
b9ece1b
Compare
michalpasztamobica
force-pushed
the
greentea_tcpsocket_sigio
branch
from
b9ece1b to
700f0e6
Compare
|
@VeijoPesonen , thanks for your remarks. I just pushed improved code. |
michalpasztamobica
force-pushed
the
greentea_tcpsocket_sigio
branch
from
700f0e6 to
999273f
Compare
cmonr
requested review from
SeppoTakalo,
OPpuolitaival and
VeijoPesonen
and removed request for
SeppoTakalo and
OPpuolitaival
|
CI started while waiting on one or two more people to complete reviews |
Test run: SUCCESSSummary: 10 of 10 test jobs passed |
Description
As previously done for TLSSockets - we don't want the receiver thread to do a busy wait.
I decided not to touch the UDPSocket echo tests as they do packet counting (and accept some missing packets ratio), so the design there would have to be different, but if you think it's reasonable to rewrite that test to, I can do it as well.
Pull request type
Reviewers
@SeppoTakalo
@AriParkkila
@VeijoPesonen
@KariHaapalehto
@mtomczykmobica
@tymoteuszblochmobica