NetworkUDP - in parsePacket handle previous parsed packet #10239
Conversation
👋 Hello JAndrassy, we appreciate your contribution to this project! Click to see more instructions ...
Review and merge process you can expect ...
|
Test Results 56 files - 83 56 suites - 83 4m 56s ⏱️ - 1h 38m 13s Results for commit 2e50c1c. ± Comparison against base commit cd3d0bf. This pull request removes 9 tests.♻️ This comment has been updated with latest results. |
Memory usage test (comparing PR against master branch)The table below shows the summary of memory usage change (decrease - increase) in bytes and percentage for each target.
Click to expand the detailed deltas report [usage change in BYTES]
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
JAndrassy
force-pushed
the
udp_parsepacket_fix
branch
from
e0978e4 to
2e50c1c
Compare
JAndrassy
changed the title
VojtechBartoska
added
Area: Libraries
| @@ -292,7 +292,10 @@ void NetworkUDP::flush() {} | |||
|
|
|||
| int NetworkUDP::parsePacket() { | |||
| if (rx_buffer) { | |||
| return 0; | |||
| if (rx_buffer->full()) { // packet was not read yet | |||
| return rx_buffer->available(); | |||
There was a problem hiding this comment.
can you show a benefit from returning the previous packet len more than once? If code did not care about it the first time, why should it care after that?
There was a problem hiding this comment.
for the library it is better than destroying it then allocate the next then destroy it ...
There was a problem hiding this comment.
library will not destroy it. It will just return 0, because old packet is still there and new one was not received. The point of returning positive length is to signal that a new packet has been received. This happens only once per packet.
There was a problem hiding this comment.
but the PR is about not returning a zero if the packet was not read until the end
| if (rx_buffer->full()) { // packet was not read yet | ||
| return rx_buffer->available(); | ||
| } | ||
| clear(); // discard the rest of the packet |
There was a problem hiding this comment.
What will happen in you call parsePacket from one thread, but you read it's data from another?
parsePacketnow returns 0 if previous packet was not read. what is the logic of that?this PR proposes to changes it to return the existing packet if it was not read yet and discard the packet if the user read some part of it.