Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace hardcoded assumption with one dynamically calculated #32291

Merged
merged 5 commits into from
Feb 28, 2024
Merged

Replace hardcoded assumption with one dynamically calculated #32291

Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
bf77aba
Replace hardcoded assumption with one dynamically calculated
tehampson Feb 23, 2024
fedd14b
Quick Fix
tehampson Feb 23, 2024
992ce19
Restyled by whitespace
restyled-commits Feb 23, 2024
cd97ab1
Restyled by clang-format
restyled-commits Feb 23, 2024
59f0bbc
Address PR comments
tehampson Feb 26, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 43 additions & 7 deletions src/app/tests/TestCommandInteraction.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -379,6 +379,8 @@ class TestCommandInteraction
static void AddInvokeResponseData(nlTestSuite * apSuite, void * apContext, CommandHandler * apCommandHandler,
bool aNeedStatusCode, CommandId aResponseCommandId = kTestCommandIdWithData,
CommandId aRequestCommandId = kTestCommandIdWithData);
static uint32_t GetAddResponseDataOverheadSizeForPath(nlTestSuite * apSuite, const ConcreteCommandPath & aRequestCommandPath,
bool aIsForcedSizeBufferLargerThan255);
static void FillCurrentInvokeResponseBuffer(nlTestSuite * apSuite, CommandHandler * apCommandHandler,
const ConcreteCommandPath & aRequestCommandPath, uint32_t aSizeToLeaveInBuffer);
static void ValidateCommandHandlerEncodeInvokeResponseMessage(nlTestSuite * apSuite, void * apContext, bool aNeedStatusCode);
Expand Down Expand Up @@ -577,6 +579,36 @@ void TestCommandInteraction::AddInvokeResponseData(nlTestSuite * apSuite, void *
}
}

uint32_t TestCommandInteraction::GetAddResponseDataOverheadSizeForPath(nlTestSuite * apSuite,
const ConcreteCommandPath & aRequestCommandPath,
bool aIsForcedSizeBufferLargerThan255)
{
BasicCommandPathRegistry<4> mBasicCommandPathRegistry;
CommandHandler commandHandler(kCommandHandlerTestOnlyMarker, &mockCommandHandlerDelegate, &mBasicCommandPathRegistry);
commandHandler.mReserveSpaceForMoreChunkMessages = true;
ConcreteCommandPath requestCommandPath1 = { kTestEndpointId, kTestClusterId, kTestCommandIdFillResponseMessage };
ConcreteCommandPath requestCommandPath2 = { kTestEndpointId, kTestClusterId, kTestCommandIdCommandSpecificResponse };
Optional<uint16_t> commandRef;
commandRef.SetValue(1);
mBasicCommandPathRegistry.Add(requestCommandPath1, commandRef);
tehampson marked this conversation as resolved.
Show resolved Hide resolved
commandRef.SetValue(2);
mBasicCommandPathRegistry.Add(requestCommandPath2, commandRef);

CHIP_ERROR err = commandHandler.AllocateBuffer();
NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR);
uint32_t remainingSizeBefore = commandHandler.mInvokeResponseBuilder.GetWriter()->GetRemainingFreeLength();

// When ForcedSizeBuffer exceeds 255, an extra byte is needed for length, affecting the overhead size required by
// AddResponseData.
uint32_t sizeOfForcedSizeBuffer = aIsForcedSizeBufferLargerThan255 ? 256 : 0;
tehampson marked this conversation as resolved.
Show resolved Hide resolved
err = commandHandler.AddResponseData(aRequestCommandPath, ForcedSizeBuffer(sizeOfForcedSizeBuffer));
NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR);
uint32_t remainingSizeAfter = commandHandler.mInvokeResponseBuilder.GetWriter()->GetRemainingFreeLength();
uint32_t delta = remainingSizeBefore - remainingSizeAfter - sizeOfForcedSizeBuffer;

return delta;
}

void TestCommandInteraction::FillCurrentInvokeResponseBuffer(nlTestSuite * apSuite, CommandHandler * apCommandHandler,
const ConcreteCommandPath & aRequestCommandPath,
uint32_t aSizeToLeaveInBuffer)
Expand All @@ -587,13 +619,17 @@ void TestCommandInteraction::FillCurrentInvokeResponseBuffer(nlTestSuite * apSui
NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR);
uint32_t remainingSize = apCommandHandler->mInvokeResponseBuilder.GetWriter()->GetRemainingFreeLength();

// TODO(#30453): Have this value derived from IM layer similar to GetSizeToEndInvokeResponseMessage()
// This number was derived by executing this method with aSizeToLeaveInBuffer = 100 and
// subsequently analyzing the remaining size to determine the overhead associated with calling
// AddResponseData with `ForcedSizeBuffer`.
uint32_t sizeNeededForAddingResponse = 27;
NL_TEST_ASSERT(apSuite, remainingSize > (aSizeToLeaveInBuffer + sizeNeededForAddingResponse));
uint32_t sizeToFill = remainingSize - aSizeToLeaveInBuffer - sizeNeededForAddingResponse;
// AddResponseData's overhead calculation depends on the size of ForcedSizeBuffer. If the buffer exceeds 255 bytes, an extra
// length byte is required. Since tests using FillCurrentInvokeResponseBuffer currently end up with sizeToFill > 255, we
// inform the calculation of this expectation. Nonetheless, we also validate this assumption for correctness.
bool isForcedSizeBufferLargerThan255 = true;
uint32_t overheadSizeNeededForAddingResponse =
GetAddResponseDataOverheadSizeForPath(apSuite, aRequestCommandPath, isForcedSizeBufferLargerThan255);
NL_TEST_ASSERT(apSuite, remainingSize > (aSizeToLeaveInBuffer + overheadSizeNeededForAddingResponse));
uint32_t sizeToFill = remainingSize - aSizeToLeaveInBuffer - overheadSizeNeededForAddingResponse;

// Validating assumption. If this fails, it means overheadSizeNeededForAddingResponse is likely too large.
NL_TEST_ASSERT(apSuite, sizeToFill >= 256);

err = apCommandHandler->AddResponseData(aRequestCommandPath, ForcedSizeBuffer(sizeToFill));
NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR);
Expand Down
Loading