Skip to content

Commit

Permalink
TC-IDM-5.2: Fix status code in test and SDK (#33952)
Browse files Browse the repository at this point in the history
* TC-IDM-5.2: Fix status code in test and SDK

Please see:
#33936

* Fix unit tests

We have unit tests! They didn't match the spec, but they exist!

One for two is still better than 0 for 2.

* Fix unit test

* Fix error on write request with incorrect timed request flag

Please see
#33952 (comment)

This was changed in the spec, but not in the SDK or unit tests.

The unit test here covers this, but we should consider adding it to
the cert testing as well. Please see
https://github.com/CHIP-Specifications/chip-test-plans/issues/4256

* fix write in test as well
  • Loading branch information
cecille authored and pull[bot] committed Sep 20, 2024
1 parent 133877a commit 2175104
Show file tree
Hide file tree
Showing 6 changed files with 37 additions and 48 deletions.
3 changes: 1 addition & 2 deletions src/app/TimedHandler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -66,10 +66,9 @@ CHIP_ERROR TimedHandler::OnMessageReceived(Messaging::ExchangeContext * aExchang
ChipLogValueX64(now.count()), this, ChipLogValueExchange(aExchangeContext));
if (now > mTimeLimit)
{
// Time is up. Spec says to send UNSUPPORTED_ACCESS.
ChipLogError(DataManagement, "Timeout expired: handler %p exchange " ChipLogFormatExchange, this,
ChipLogValueExchange(aExchangeContext));
return StatusResponse::Send(Status::UnsupportedAccess, aExchangeContext, /* aExpectResponse = */ false);
return StatusResponse::Send(Status::Timeout, aExchangeContext, /* aExpectResponse = */ false);
}

if (aPayloadHeader.HasMessageType(MsgType::InvokeCommandRequest))
Expand Down
4 changes: 2 additions & 2 deletions src/app/WriteHandler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -587,8 +587,8 @@ Status WriteHandler::ProcessWriteRequest(System::PacketBufferHandle && aPayload,
if (mIsTimedRequest != aIsTimedWrite)
{
// The message thinks it should be part of a timed interaction but it's
// not, or vice versa. Spec says to Respond with UNSUPPORTED_ACCESS.
status = Status::UnsupportedAccess;
// not, or vice versa.
status = Status::TimedRequestMismatch;
goto exit;
}

Expand Down
8 changes: 4 additions & 4 deletions src/app/tests/TestTimedHandler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ void TestTimedHandler::TestFollowingMessageFastEnough(MsgType aMsgType)
EXPECT_EQ(delegate.mError, CHIP_NO_ERROR);

// Send an empty payload, which will error out but not with the
// UNSUPPORTED_ACCESS status we expect if we miss our timeout.
// TIMEOUT status we expect if we miss our timeout.
payload = MessagePacketBuffer::New(0);
ASSERT_FALSE(payload.IsNull());

Expand All @@ -151,7 +151,7 @@ void TestTimedHandler::TestFollowingMessageFastEnough(MsgType aMsgType)
mpTestContext->DrainAndServiceIO();
EXPECT_TRUE(delegate.mNewMessageReceived);
EXPECT_TRUE(delegate.mLastMessageWasStatus);
EXPECT_NE(StatusIB(delegate.mError).mStatus, Status::UnsupportedAccess);
EXPECT_NE(StatusIB(delegate.mError).mStatus, Status::Timeout);
}

TEST_F(TestTimedHandler, TestInvokeFastEnough)
Expand Down Expand Up @@ -189,7 +189,7 @@ void TestTimedHandler::TestFollowingMessageTooSlow(MsgType aMsgType)
chip::test_utils::SleepMillis(75);

// Send an empty payload, which will error out but not with the
// UNSUPPORTED_ACCESS status we expect if we miss our timeout.
// TIMEOUT status we expect if we miss our timeout.
payload = MessagePacketBuffer::New(0);
EXPECT_FALSE(payload.IsNull());

Expand All @@ -201,7 +201,7 @@ void TestTimedHandler::TestFollowingMessageTooSlow(MsgType aMsgType)
mpTestContext->DrainAndServiceIO();
EXPECT_TRUE(delegate.mNewMessageReceived);
EXPECT_TRUE(delegate.mLastMessageWasStatus);
EXPECT_EQ(StatusIB(delegate.mError).mStatus, Status::UnsupportedAccess);
EXPECT_EQ(StatusIB(delegate.mError).mStatus, Status::Timeout);
}

TEST_F(TestTimedHandler, TestInvokeTooSlow)
Expand Down
2 changes: 1 addition & 1 deletion src/app/tests/TestWriteInteraction.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -328,7 +328,7 @@ TEST_F(TestWriteInteraction, TestWriteHandler)
}
else
{
EXPECT_EQ(status, Status::UnsupportedAccess);
EXPECT_EQ(status, Status::TimedRequestMismatch);
}

mpTestContext->DrainAndServiceIO();
Expand Down
8 changes: 4 additions & 4 deletions src/app/tests/suites/TestClusterComplexTypes.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ tests:
# expires.
busyWaitMs: 100
response:
error: UNSUPPORTED_ACCESS
error: TIMEOUT

- label:
"Send command that does not need timed invoke with a long timeout
Expand All @@ -88,7 +88,7 @@ tests:
# expires.
busyWaitMs: 100
response:
error: UNSUPPORTED_ACCESS
error: TIMEOUT

# The timed write tests are not in TestCluster.yaml for now because Darwin
# SDK APIs have not been updated to do timed invoke properly yet.
Expand Down Expand Up @@ -125,7 +125,7 @@ tests:
# expires.
busyWaitMs: 100
response:
error: UNSUPPORTED_ACCESS
error: TIMEOUT

- label: "Read attribute that needs timed write state unchanged 2"
command: "readAttribute"
Expand Down Expand Up @@ -173,7 +173,7 @@ tests:
# expires.
busyWaitMs: 100
response:
error: UNSUPPORTED_ACCESS
error: TIMEOUT

- label: "Read attribute that does not need timed write unchanged value"
command: "readAttribute"
Expand Down
60 changes: 25 additions & 35 deletions src/app/tests/suites/certification/Test_TC_IDM_5_2.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -94,30 +94,19 @@ tests:
verification: |
./chip-tool onoff on 1 1 --repeat-delay-ms 5000 --timedInteractionTimeoutMs 200
On TH(chip-tool), Verify we are getting status response and UNSUPPORTED_ACCESS from DUT to TH for above command
[1649686333.696111][3252:3257] CHIP:DMG: StatusResponseMessage =
[1649686333.696178][3252:3257] CHIP:DMG: {
[1649686333.696236][3252:3257] CHIP:DMG: Status = 0x0,
[1649686333.696311][3252:3257] CHIP:DMG: InteractionModelRevision = 1
[1649686333.696375][3252:3257] CHIP:DMG: }
[1649686333.696434][3252:3257] CHIP:IM: Received status response, status is 0x00 (SUCCESS)
[1649686333.696513][3252:3257] CHIP:EM: Piggybacking Ack for MessageCounter:3570360 on exchange: 28108i
[1649686333.696626][3252:3257] CHIP:IN: Prepared secure message 0xaaaae019e098 to 0x0000000000000001 (1) of type 0x8 and protocolId (0, 1) on exchange 28108i with MessageCounter:6840786.
[1649686333.696711][3252:3257] CHIP:IN: Sending encrypted msg 0xaaaae019e098 with MessageCounter:6840786 to 0x0000000000000001 (1) at monotonic time: 0000000000CE3312 msec
[1649686333.696989][3252:3257] CHIP:DMG: ICR moving to [CommandSen]
[1649686333.698938][3252:3257] CHIP:EM: Received message of type 0x1 with protocolId (0, 1) and MessageCounter:3570361 on exchange 28108i
[1649686333.699013][3252:3257] CHIP:EM: Found matching exchange: 28108i, Delegate: 0xffff68005c30
[1649686333.699089][3252:3257] CHIP:EM: Rxd Ack; Removing MessageCounter:6840786 from Retrans Table on exchange 28108i
[1649686333.699147][3252:3257] CHIP:EM: Removed CHIP MessageCounter:6840786 from RetransTable on exchange 28108i
[1649686333.699215][3252:3257] CHIP:DMG: ICR moving to [ResponseRe]
[1649686333.699294][3252:3257] CHIP:DMG: StatusResponseMessage =
[1649686333.699354][3252:3257] CHIP:DMG: {
[1649686333.699410][3252:3257] CHIP:DMG: Status = 0x7e,
[1649686333.699471][3252:3257] CHIP:DMG: InteractionModelRevision = 1
[1649686333.699528][3252:3257] CHIP:DMG: }
[1649686333.699585][3252:3257] CHIP:IM: Received status response, status is 0x7e (UNSUPPORTED_ACCESS)
[1649686333.699661][3252:3257] CHIP:TOO: Error: IM Error 0x0000057E: General error: 0x7e (UNSUPPORTED_ACCESS)
If the device being certified is Matter release 1.4 or later, verify DUT sends back a Status response with the TIMEOUT status code.
If the device being certified is Matter release 1.3 or earlier, verify the DUT sends back a Status response with either TIMEOUT or UNSUPPORTED_ACCESS status code.
[1718641649.158222][1587554:1587556] CHIP:EM: Rxd Ack; Removing MessageCounter:70404585 from Retrans Table on exchange 44026i
[1718641649.158241][1587554:1587556] CHIP:DMG: ICR moving to [ResponseRe]
[1718641649.158261][1587554:1587556] CHIP:DMG: StatusResponseMessage =
[1718641649.158276][1587554:1587556] CHIP:DMG: {
[1718641649.158290][1587554:1587556] CHIP:DMG: Status = 0x94 (TIMEOUT),
[1718641649.158304][1587554:1587556] CHIP:DMG: InteractionModelRevision = 11
[1718641649.158318][1587554:1587556] CHIP:DMG: }
[1718641649.158332][1587554:1587556] CHIP:IM: Received status response, status is 0x94 (TIMEOUT)
[1718641649.158355][1587554:1587556] CHIP:TOO: Error: IM Error 0x00000594: General error: 0x94 (TIMEOUT)
disabled: true

- label:
Expand All @@ -128,15 +117,16 @@ tests:
verification: |
./chip-tool modeselect write on-mode 0 1 1 --repeat-delay-ms 1000 --timedInteractionTimeoutMs 500
On TH(chip-tool), Verify we are getting status response and UNSUPPORTED_ACCESS from DUT to TH for above command
[1654771611.106067][28715:28720] CHIP:DMG: WriteClient moving to [ResponseRe]
[1654771611.106112][28715:28720] CHIP:DMG: StatusResponseMessage =
[1654771611.106140][28715:28720] CHIP:DMG: {
[1654771611.106165][28715:28720] CHIP:DMG: Status = 0x7e (UNSUPPORTED_ACCESS),
[1654771611.106192][28715:28720] CHIP:DMG: InteractionModelRevision = 1
[1654771611.106216][28715:28720] CHIP:DMG: }
[1654771611.106244][28715:28720] CHIP:IM: Received status response, status is 0x7e (UNSUPPORTED_ACCESS)
[1654771611.106280][28715:28720] CHIP:TOO: Error: IM Error 0x0000057E: General error: 0x7e (UNSUPPORTED_ACCESS)
[1654771611.106303][28715:28720] CHIP:DMG: WriteClient moving to [AwaitingDe]
If the device being certified is Matter release 1.4 or later, verify DUT sends back a Status response with the TIMEOUT status code.
If the device being certified is Matter release 1.3 or earlier, verify the DUT sends back a Status response with either TIMEOUT or UNSUPPORTED_ACCESS status code.
[1720104134.620521][3325282:3325284] CHIP:DMG: WriteClient moving to [ResponseRe]
[1720104134.620540][3325282:3325284] CHIP:DMG: StatusResponseMessage =
[1720104134.620555][3325282:3325284] CHIP:DMG: {
[1720104134.620569][3325282:3325284] CHIP:DMG: Status = 0x94 (TIMEOUT),
[1720104134.620583][3325282:3325284] CHIP:DMG: InteractionModelRevision = 11
[1720104134.620596][3325282:3325284] CHIP:DMG: }
[1720104134.620611][3325282:3325284] CHIP:IM: Received status response, status is 0x94 (TIMEOUT)
[1720104134.620689][3325282:3325284] CHIP:TOO: Error: IM Error 0x00000594: General error: 0x94 (TIMEOUT)
disabled: true

0 comments on commit 2175104

Please sign in to comment.