Check when mismatched sequencing occurs#205
Check when mismatched sequencing occurs#205corbin-phipps merged 5 commits intofeature/dataStreamingfrom
Conversation
|
|
||
| grpc::Status | ||
| DataStreamReader::Await(uint32_t* numberOfDataBlocksReceived, DataStreamOperationStatus* operationStatus) | ||
| DataStreamReader::Await(uint32_t* numberOfDataBlocksReceived, DataStreamOperationStatus* operationStatus, std::span<uint32_t> lostDataBlockSequenceNumbers) |
There was a problem hiding this comment.
I'm not sure I understand the usage of this variable. The std::span is being passed as a value copy, but it seems like this is intended to be used as an output variable instead. If that's true, this should be a reference instead (std::span<uint32_t>&). Have I misunderstood something here?
There was a problem hiding this comment.
You're right, should definitely be a reference
|
|
||
| *numberOfDataBlocksReceived = m_numberOfDataBlocksReceived; | ||
| *operationStatus = m_readData.status(); | ||
| lostDataBlockSequenceNumbers = std::span<uint32_t>(std::data(m_lostDataBlockSequenceNumbers), std::size(m_lostDataBlockSequenceNumbers)); |
There was a problem hiding this comment.
This is modifying the value passed to the function, then not used. Was lostDataBlockSequenceNumbers meant to be a reference such that this update's the caller's arg?
There was a problem hiding this comment.
Yes, should be a reference. Will update
| REQUIRE(status.ok()); | ||
| REQUIRE(numberOfDataBlocksReceived == fixedNumberOfDataBlocksToStream); | ||
| REQUIRE(operationStatus.code() == DataStreamOperationStatusCodeSucceeded); | ||
| REQUIRE(lostDataBlockSequenceNumbers.empty()); |
There was a problem hiding this comment.
This will always be true since a copy of lostDataBlockSequenceNumbers is passed in.
2 participants
Type
Side Effects
Goals
Fixes #186
Technical Details
OnReadDonecallbacks and store a list of the unreceived sequence numbers.std::spanparameter of the sequence numbers in the client testAwaitfunctions.Test Results
All tests pass.
Reviewer Focus
None.
Future Work
None.
Checklist
allcompiles cleanly.