Skip to content

Commit 501348e

Browse files
sabollim-silabsrestyled-commits
sabollim-silabs
and
restyled-commits
authored
Closure App command timeout optimisation (project-chip#40714)
* Closure optimize timeouts * Restyled by clang-format * Removing slow tests --------- Co-authored-by: Restyled.io <commits@restyled.io>
1 parent f8c9e8e commit 501348e

5 files changed

Lines changed: 48 additions & 43 deletions

File tree

‎examples/closure-app/linux/ClosureManager.cpp‎

Lines changed: 22 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,9 @@ using namespace chip::app::Clusters::ClosureDimension;
3434
namespace {
3535
// Define a constant for the countdown time
3636
constexpr uint32_t kCountdownTimeSeconds = 10;
37+
constexpr uint32_t kCalibrateCountdownTimeMs = 3000; // 3 seconds for calibrate motion
3738
constexpr uint32_t kMotionCountdownTimeMs = 1000; // 1 second for each motion.
38-
constexpr chip::Percent100ths kMotionPositionStep = 1000; // 10% of the total range per motion interval.
39+
constexpr chip::Percent100ths kMotionPositionStep = 2000; // 20% of the total range per motion interval.
3940

4041
// Define the Namespace and Tag for the endpoint
4142
// Derived from https://github.com/CHIP-Specifications/connectedhomeip-spec/blob/master/src/namespaces/Namespace-Closure.adoc
@@ -169,7 +170,7 @@ CHIP_ERROR ClosureManager::SetClosurePanelInitialState(ClosureDimensionEndpoint
169170
ReturnErrorOnFailure(closurePanelEndpoint.GetLogic().SetTargetState(targetState));
170171

171172
ReturnErrorOnFailure(closurePanelEndpoint.GetLogic().SetResolution(Percent100ths(100)));
172-
ReturnErrorOnFailure(closurePanelEndpoint.GetLogic().SetStepValue(1000));
173+
ReturnErrorOnFailure(closurePanelEndpoint.GetLogic().SetStepValue(2000));
173174
ReturnErrorOnFailure(closurePanelEndpoint.GetLogic().SetUnit(ClosureUnitEnum::kMillimeter));
174175
ReturnErrorOnFailure(closurePanelEndpoint.GetLogic().SetUnitRange(
175176
ClosureDimension::Structs::UnitRangeStruct::Type{ .min = static_cast<int16_t>(0), .max = static_cast<int16_t>(10000) }));
@@ -198,7 +199,7 @@ chip::Protocols::InteractionModel::Status ClosureManager::OnCalibrateCommand()
198199

199200
// For sample application, we are using a timer to simulate the hardware calibration action.
200201
// In a real application, this would be replaced with actual calibration logic and call HandleClosureActionComplete.
201-
VerifyOrReturnValue(DeviceLayer::SystemLayer().StartTimer(System::Clock::Seconds32(kCountdownTimeSeconds),
202+
VerifyOrReturnValue(DeviceLayer::SystemLayer().StartTimer(System::Clock::Milliseconds32(kCalibrateCountdownTimeMs),
202203
HandleEp1ClosureActionTimer, nullptr) == CHIP_NO_ERROR,
203204
Status::Failure, ChipLogError(AppServer, "Failed to start closure action timer"));
204205

@@ -275,16 +276,16 @@ ClosureManager::OnMoveToCommand(const Optional<TargetPositionEnum> & position, c
275276
ep3Position = static_cast<chip::Percent100ths>(0);
276277
break;
277278
case TargetPositionEnum::kMoveToPedestrianPosition:
278-
ep2Position = static_cast<chip::Percent100ths>(3000);
279-
ep3Position = static_cast<chip::Percent100ths>(3000);
279+
ep2Position = static_cast<chip::Percent100ths>(6000);
280+
ep3Position = static_cast<chip::Percent100ths>(6000);
280281
break;
281282
case TargetPositionEnum::kMoveToSignaturePosition:
282-
ep2Position = static_cast<chip::Percent100ths>(2000);
283-
ep3Position = static_cast<chip::Percent100ths>(2000);
283+
ep2Position = static_cast<chip::Percent100ths>(4000);
284+
ep3Position = static_cast<chip::Percent100ths>(4000);
284285
break;
285286
case TargetPositionEnum::kMoveToVentilationPosition:
286-
ep2Position = static_cast<chip::Percent100ths>(1000);
287-
ep3Position = static_cast<chip::Percent100ths>(1000);
287+
ep2Position = static_cast<chip::Percent100ths>(2000);
288+
ep3Position = static_cast<chip::Percent100ths>(2000);
288289
break;
289290
default:
290291
ChipLogError(AppServer, "Invalid target position received in OnMoveToCommand");
@@ -750,7 +751,11 @@ void ClosureManager::HandlePanelStepAction(EndpointId endpointId)
750751
}
751752
else
752753
{
753-
nextCurrentPosition = std::max(static_cast<chip::Percent100ths>(currentPosition - stepValue), targetPosition);
754+
// Underflow protection: if currentPosition <= stepValue, set to 0.
755+
chip::Percent100ths decreasedCurrentPosition = (currentPosition > stepValue)
756+
? static_cast<chip::Percent100ths>(currentPosition - stepValue)
757+
: static_cast<chip::Percent100ths>(0);
758+
nextCurrentPosition = std::max(decreasedCurrentPosition, targetPosition);
754759
}
755760

756761
panelCurrentState.Value().position.SetValue(DataModel::MakeNullable(nextCurrentPosition));
@@ -931,14 +936,18 @@ bool ClosureManager::GetPanelNextPosition(const GenericDimensionStateStruct & cu
931936

932937
if (currentPosition < targetPosition)
933938
{
934-
// Increment position by 1000 units, capped at target.
939+
// Increment position by 2000 units, capped at target.
940+
// No overflow handling needed due to currentposition max value is 10000
935941
nextPosition.SetNonNull(std::min(static_cast<chip::Percent100ths>(currentPosition + kMotionPositionStep), targetPosition));
936942
}
937943
else if (currentPosition > targetPosition)
938944
{
939-
// Moving down: Decreasing the current position by a step of 1000 units,
945+
// Handling overflow for CurrentPosition
946+
chip::Percent100ths newCurrentPosition =
947+
(currentPosition > kMotionPositionStep) ? currentPosition - kMotionPositionStep : 0;
948+
// Moving down: Decreasing the current position by a step of 2000 units,
940949
// ensuring it does not go below the target position.
941-
nextPosition.SetNonNull(std::max(static_cast<chip::Percent100ths>(currentPosition - kMotionPositionStep), targetPosition));
950+
nextPosition.SetNonNull(std::max(newCurrentPosition, targetPosition));
942951
}
943952
else
944953
{

‎examples/closure-app/silabs/src/ClosureManager.cpp‎

Lines changed: 24 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -37,10 +37,10 @@ using namespace chip::app::Clusters::ClosureControl;
3737
using namespace chip::app::Clusters::ClosureDimension;
3838

3939
namespace {
40-
constexpr uint32_t kDefaultCountdownTimeSeconds = 10; // 10 seconds
41-
constexpr uint32_t kCalibrateTimerMs = 10000; // 10 seconds
42-
constexpr uint32_t kMotionCountdownTimeMs = 1000; // 1 second for each motion.
43-
constexpr chip::Percent100ths kMotionPositionStep = 1000; // 10% of the total range per motion interval.
40+
constexpr uint32_t kDefaultCountdownTimeSeconds = 10; // 10 seconds
41+
constexpr uint32_t kCalibrateCountdownTimeMs = 3000; // 3 seconds
42+
constexpr uint32_t kMotionCountdownTimeMs = 1000; // 1 second for each motion.
43+
constexpr chip::Percent100ths kMotionPositionStep = 2000; // 20% of the total range per motion interval.
4444

4545
// Define the Namespace and Tag for the endpoint
4646
// Derived from https://github.com/CHIP-Specifications/connectedhomeip-spec/blob/master/src/namespaces/Namespace-Closure.adoc
@@ -168,7 +168,7 @@ CHIP_ERROR ClosureManager::SetClosurePanelInitialState(ClosureDimensionEndpoint
168168
ReturnErrorOnFailure(closurePanelEndpoint.GetLogic().SetTargetState(targetState));
169169

170170
ReturnErrorOnFailure(closurePanelEndpoint.GetLogic().SetResolution(Percent100ths(100)));
171-
ReturnErrorOnFailure(closurePanelEndpoint.GetLogic().SetStepValue(1000));
171+
ReturnErrorOnFailure(closurePanelEndpoint.GetLogic().SetStepValue(2000));
172172
ReturnErrorOnFailure(closurePanelEndpoint.GetLogic().SetUnit(ClosureUnitEnum::kMillimeter));
173173
ReturnErrorOnFailure(closurePanelEndpoint.GetLogic().SetUnitRange(
174174
ClosureDimension::Structs::UnitRangeStruct::Type{ .min = static_cast<int16_t>(0), .max = static_cast<int16_t>(10000) }));
@@ -227,7 +227,7 @@ void ClosureManager::InitiateAction(AppEvent * event)
227227
ChipLogDetail(AppServer, "Initiating calibration action");
228228
// Timer used in sample application to simulate the calibration process.
229229
// In a real application, this would be replaced with actual calibration logic.
230-
instance.StartTimer(kCalibrateTimerMs);
230+
instance.StartTimer(kCalibrateCountdownTimeMs);
231231
break;
232232
case Action_t::MOVE_TO_ACTION:
233233
ChipLogDetail(AppServer, "Initiating move to action");
@@ -535,16 +535,16 @@ chip::Protocols::InteractionModel::Status ClosureManager::OnMoveToCommand(const
535535
mClosurePanelEndpoint3Position = static_cast<Percent100ths>(0);
536536
break;
537537
case TargetPositionEnum::kMoveToPedestrianPosition:
538-
mClosurePanelEndpoint2Position = static_cast<Percent100ths>(3000);
539-
mClosurePanelEndpoint3Position = static_cast<Percent100ths>(3000);
538+
mClosurePanelEndpoint2Position = static_cast<Percent100ths>(6000);
539+
mClosurePanelEndpoint3Position = static_cast<Percent100ths>(6000);
540540
break;
541541
case TargetPositionEnum::kMoveToSignaturePosition:
542-
mClosurePanelEndpoint2Position = static_cast<Percent100ths>(2000);
543-
mClosurePanelEndpoint3Position = static_cast<Percent100ths>(2000);
542+
mClosurePanelEndpoint2Position = static_cast<Percent100ths>(4000);
543+
mClosurePanelEndpoint3Position = static_cast<Percent100ths>(4000);
544544
break;
545545
case TargetPositionEnum::kMoveToVentilationPosition:
546-
mClosurePanelEndpoint2Position = static_cast<Percent100ths>(1000);
547-
mClosurePanelEndpoint3Position = static_cast<Percent100ths>(1000);
546+
mClosurePanelEndpoint2Position = static_cast<Percent100ths>(2000);
547+
mClosurePanelEndpoint3Position = static_cast<Percent100ths>(2000);
548548
break;
549549
default:
550550
ChipLogError(AppServer, "Invalid target position received in OnMoveToCommand");
@@ -1102,7 +1102,11 @@ void ClosureManager::HandlePanelStepAction(EndpointId endpointId)
11021102
}
11031103
else
11041104
{
1105-
nextCurrentPosition = std::max(static_cast<chip::Percent100ths>(currentPosition - stepValue), targetPosition);
1105+
// Underflow protection: if currentPosition <= stepValue, set to 0.
1106+
chip::Percent100ths decreasedCurrentPosition = (currentPosition > stepValue)
1107+
? static_cast<chip::Percent100ths>(currentPosition - stepValue)
1108+
: static_cast<chip::Percent100ths>(0);
1109+
nextCurrentPosition = std::max(decreasedCurrentPosition, targetPosition);
11061110
}
11071111

11081112
panelCurrentState.Value().position.SetValue(DataModel::MakeNullable(nextCurrentPosition));
@@ -1157,14 +1161,18 @@ bool ClosureManager::GetPanelNextPosition(const GenericDimensionStateStruct & cu
11571161

11581162
if (currentPosition < targetPosition)
11591163
{
1160-
// Increment position by 1000 units, capped at target.
1164+
// Increment position by 2000 units, capped at target.
1165+
// No overflow handling needed due to currentposition max value is 10000
11611166
nextPosition.SetNonNull(std::min(static_cast<chip::Percent100ths>(currentPosition + kMotionPositionStep), targetPosition));
11621167
}
11631168
else if (currentPosition > targetPosition)
11641169
{
1165-
// Moving down: Decreasing the current position by a step of 1000 units,
1170+
// Handling overflow for CurrentPosition
1171+
chip::Percent100ths newCurrentPosition =
1172+
(currentPosition > kMotionPositionStep) ? currentPosition - kMotionPositionStep : 0;
1173+
// Moving down: Decreasing the current position by a step of 2000 units,
11661174
// ensuring it does not go below the target position.
1167-
nextPosition.SetNonNull(std::max(static_cast<chip::Percent100ths>(currentPosition - kMotionPositionStep), targetPosition));
1175+
nextPosition.SetNonNull(std::max(newCurrentPosition, targetPosition));
11681176
}
11691177
else
11701178
{

‎src/python_testing/TC_CLCTRL_3_1.py‎

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@
2525
# --commissioning-method on-network
2626
# --discriminator 1234
2727
# --passcode 20202021
28-
# --timeout 120
28+
# --timeout 30
2929
# --endpoint 1
3030
# --trace-to json:${TRACE_TEST_JSON}.json
3131
# --trace-to perfetto:${TRACE_TEST_PERFETTO}.perfetto
@@ -69,11 +69,6 @@ def predicate(report: AttributeValue) -> bool:
6969

7070

7171
class TC_CLCTRL_3_1(MatterBaseTest):
72-
@property
73-
def default_timeout(self) -> int:
74-
# Default timeout for this test case is 120 seconds, multiple calibrates can take a while
75-
return 120
76-
7772
async def read_clctrl_attribute_expect_success(self, endpoint, attribute):
7873
cluster = Clusters.Objects.ClosureControl
7974
return await self.read_single_attribute_check_success(endpoint=endpoint, cluster=cluster, attribute=attribute)

‎src/python_testing/TC_CLCTRL_6_1.py‎

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@
2525
# --commissioning-method on-network
2626
# --discriminator 1234
2727
# --passcode 20202021
28-
# --timeout 60
28+
# --timeout 30
2929
# --endpoint 1
3030
# --hex-arg enableKey:000102030405060708090a0b0c0d0e0f
3131
# --trace-to json:${TRACE_TEST_JSON}.json
@@ -68,11 +68,6 @@ def predicate(report: AttributeValue) -> bool:
6868

6969

7070
class TC_CLCTRL_6_1(MatterBaseTest):
71-
@property
72-
def default_timeout(self) -> int:
73-
# This test has potentially 45s waits, so set the timeout to 60
74-
return 60
75-
7671
async def read_clctrl_attribute_expect_success(self, endpoint, attribute):
7772
cluster = Clusters.Objects.ClosureControl
7873
return await self.read_single_attribute_check_success(endpoint=endpoint, cluster=cluster, attribute=attribute)

‎src/python_testing/test_metadata.yaml‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -177,5 +177,3 @@ slow_tests:
177177
- { name: TC_TIMESYNC_2_12.py, duration: 20 seconds }
178178
- { name: TC_TIMESYNC_2_7.py, duration: 20 seconds }
179179
- { name: TC_TIMESYNC_2_8.py, duration: 1.5 minutes }
180-
- { name: TC_CLCTRL_3_1.py, duration: 45 seconds }
181-
- { name: TC_CLCTRL_6_1.py, duration: 45 seconds }

0 commit comments

Comments
 (0)