Skip to content

Commit ba0a467

Browse files
pidarpedpidarped
committed
Add more constraint checking for command params.
Add a delegate notification callback for attribute changes. Address other review comments.
1 parent ccf3fdd commit ba0a467

File tree

2 files changed

+46
-41
lines changed

2 files changed

+46
-41
lines changed

‎src/app/clusters/camera-av-stream-management-server/camera-av-stream-management-server.cpp

+38-41
Original file line numberDiff line numberDiff line change
@@ -229,6 +229,7 @@ CHIP_ERROR CameraAVStreamMgmtServer::AddToFabricsUsingCamera(FabricIndex aFabric
229229
{
230230
mFabricsUsingCamera.insert(aFabricIndex);
231231
auto path = ConcreteAttributePath(mEndpointId, CameraAvStreamManagement::Id, Attributes::FabricsUsingCamera::Id);
232+
mDelegate.OnAttributeChanged(Attributes::FabricsUsingCamera::Id);
232233
MatterReportingAttributeChangeCallback(path);
233234

234235
return CHIP_NO_ERROR;
@@ -238,6 +239,7 @@ CHIP_ERROR CameraAVStreamMgmtServer::RemoveFromFabricsUsingCamera(FabricIndex aF
238239
{
239240
mFabricsUsingCamera.erase(aFabricIndex);
240241
auto path = ConcreteAttributePath(mEndpointId, CameraAvStreamManagement::Id, Attributes::FabricsUsingCamera::Id);
242+
mDelegate.OnAttributeChanged(Attributes::FabricsUsingCamera::Id);
241243
MatterReportingAttributeChangeCallback(path);
242244

243245
return CHIP_NO_ERROR;
@@ -250,6 +252,7 @@ CHIP_ERROR CameraAVStreamMgmtServer::SetRankedVideoStreamPriorities(
250252

251253
ReturnErrorOnFailure(StoreRankedVideoStreamPriorities());
252254
auto path = ConcreteAttributePath(mEndpointId, CameraAvStreamManagement::Id, Attributes::RankedVideoStreamPrioritiesList::Id);
255+
mDelegate.OnAttributeChanged(Attributes::RankedVideoStreamPrioritiesList::Id);
253256
MatterReportingAttributeChangeCallback(path);
254257

255258
return CHIP_NO_ERROR;
@@ -259,6 +262,7 @@ CHIP_ERROR CameraAVStreamMgmtServer::AddVideoStream(const VideoStreamStruct & vi
259262
{
260263
mAllocatedVideoStreams.push_back(videoStream);
261264
auto path = ConcreteAttributePath(mEndpointId, CameraAvStreamManagement::Id, Attributes::AllocatedVideoStreams::Id);
265+
mDelegate.OnAttributeChanged(Attributes::AllocatedVideoStreams::Id);
262266
MatterReportingAttributeChangeCallback(path);
263267

264268
return CHIP_NO_ERROR;
@@ -271,6 +275,7 @@ CHIP_ERROR CameraAVStreamMgmtServer::RemoveVideoStream(uint16_t videoStreamId)
271275
[&](const VideoStreamStruct & vStream) { return vStream.videoStreamID == videoStreamId; }),
272276
mAllocatedVideoStreams.end());
273277
auto path = ConcreteAttributePath(mEndpointId, CameraAvStreamManagement::Id, Attributes::AllocatedVideoStreams::Id);
278+
mDelegate.OnAttributeChanged(Attributes::AllocatedVideoStreams::Id);
274279
MatterReportingAttributeChangeCallback(path);
275280

276281
return CHIP_NO_ERROR;
@@ -280,6 +285,7 @@ CHIP_ERROR CameraAVStreamMgmtServer::AddAudioStream(const AudioStreamStruct & au
280285
{
281286
mAllocatedAudioStreams.push_back(audioStream);
282287
auto path = ConcreteAttributePath(mEndpointId, CameraAvStreamManagement::Id, Attributes::AllocatedAudioStreams::Id);
288+
mDelegate.OnAttributeChanged(Attributes::AllocatedAudioStreams::Id);
283289
MatterReportingAttributeChangeCallback(path);
284290

285291
return CHIP_NO_ERROR;
@@ -292,6 +298,7 @@ CHIP_ERROR CameraAVStreamMgmtServer::RemoveAudioStream(uint16_t audioStreamId)
292298
[&](const AudioStreamStruct & aStream) { return aStream.audioStreamID == audioStreamId; }),
293299
mAllocatedAudioStreams.end());
294300
auto path = ConcreteAttributePath(mEndpointId, CameraAvStreamManagement::Id, Attributes::AllocatedAudioStreams::Id);
301+
mDelegate.OnAttributeChanged(Attributes::AllocatedAudioStreams::Id);
295302
MatterReportingAttributeChangeCallback(path);
296303

297304
return CHIP_NO_ERROR;
@@ -301,6 +308,7 @@ CHIP_ERROR CameraAVStreamMgmtServer::AddSnapshotStream(const SnapshotStreamStruc
301308
{
302309
mAllocatedSnapshotStreams.push_back(snapshotStream);
303310
auto path = ConcreteAttributePath(mEndpointId, CameraAvStreamManagement::Id, Attributes::AllocatedSnapshotStreams::Id);
311+
mDelegate.OnAttributeChanged(Attributes::AllocatedSnapshotStreams::Id);
304312
MatterReportingAttributeChangeCallback(path);
305313

306314
return CHIP_NO_ERROR;
@@ -313,6 +321,7 @@ CHIP_ERROR CameraAVStreamMgmtServer::RemoveSnapshotStream(uint16_t snapshotStrea
313321
[&](const SnapshotStreamStruct & sStream) { return sStream.snapshotStreamID == snapshotStreamId; }),
314322
mAllocatedSnapshotStreams.end());
315323
auto path = ConcreteAttributePath(mEndpointId, CameraAvStreamManagement::Id, Attributes::AllocatedSnapshotStreams::Id);
324+
mDelegate.OnAttributeChanged(Attributes::AllocatedSnapshotStreams::Id);
316325
MatterReportingAttributeChangeCallback(path);
317326

318327
return CHIP_NO_ERROR;
@@ -776,6 +785,7 @@ CHIP_ERROR CameraAVStreamMgmtServer::SetNightVision(TriStateAutoEnum aNightVisio
776785
mNightVision = aNightVision;
777786
auto path = ConcreteAttributePath(mEndpointId, CameraAvStreamManagement::Id, Attributes::NightVision::Id);
778787
ReturnErrorOnFailure(GetSafeAttributePersistenceProvider()->WriteScalarValue(path, to_underlying(mNightVision)));
788+
mDelegate.OnAttributeChanged(Attributes::NightVision::Id);
779789
MatterReportingAttributeChangeCallback(path);
780790
}
781791
return CHIP_NO_ERROR;
@@ -788,6 +798,7 @@ CHIP_ERROR CameraAVStreamMgmtServer::SetNightVisionIllum(TriStateAutoEnum aNight
788798
mNightVisionIllum = aNightVisionIllum;
789799
auto path = ConcreteAttributePath(mEndpointId, CameraAvStreamManagement::Id, Attributes::NightVisionIllum::Id);
790800
ReturnErrorOnFailure(GetSafeAttributePersistenceProvider()->WriteScalarValue(path, to_underlying(mNightVisionIllum)));
801+
mDelegate.OnAttributeChanged(Attributes::NightVisionIllum::Id);
791802
MatterReportingAttributeChangeCallback(path);
792803
}
793804
return CHIP_NO_ERROR;
@@ -799,6 +810,7 @@ CHIP_ERROR CameraAVStreamMgmtServer::SetViewport(const ViewportStruct & aViewpor
799810

800811
StoreViewport(mViewport);
801812
auto path = ConcreteAttributePath(mEndpointId, CameraAvStreamManagement::Id, Attributes::Viewport::Id);
813+
mDelegate.OnAttributeChanged(Attributes::Viewport::Id);
802814
MatterReportingAttributeChangeCallback(path);
803815

804816
return CHIP_NO_ERROR;
@@ -923,6 +935,7 @@ CHIP_ERROR CameraAVStreamMgmtServer::SetStatusLightBrightness(Globals::ThreeLeve
923935
mStatusLightBrightness = aStatusLightBrightness;
924936
auto path = ConcreteAttributePath(mEndpointId, CameraAvStreamManagement::Id, Attributes::StatusLightBrightness::Id);
925937
ReturnErrorOnFailure(GetSafeAttributePersistenceProvider()->WriteScalarValue(path, to_underlying(mStatusLightBrightness)));
938+
mDelegate.OnAttributeChanged(Attributes::StatusLightBrightness::Id);
926939
MatterReportingAttributeChangeCallback(path);
927940
}
928941
return CHIP_NO_ERROR;
@@ -1472,15 +1485,13 @@ void CameraAVStreamMgmtServer::HandleVideoStreamAllocate(HandlerContext & ctx,
14721485
// If Watermark feature is supported, then command should have the
14731486
// isWaterMarkEnabled param. Or, if it is not supported, then command should
14741487
// not have the param.
1475-
VerifyOrReturn((HasFeature(Feature::kWatermark) && commandData.watermarkEnabled.HasValue()) ||
1476-
(!HasFeature(Feature::kWatermark) && !commandData.watermarkEnabled.HasValue()),
1488+
VerifyOrReturn((HasFeature(Feature::kWatermark) == commandData.watermarkEnabled.HasValue()),
14771489
ctx.mCommandHandler.AddStatus(ctx.mRequestPath, Status::InvalidCommand));
14781490

14791491
// If OSD feature is supported, then command should have the
14801492
// isOSDEnabled param. Or, if it is not supported, then command should
14811493
// not have the param.
1482-
VerifyOrReturn((HasFeature(Feature::kOnScreenDisplay) && commandData.OSDEnabled.HasValue()) ||
1483-
(!HasFeature(Feature::kOnScreenDisplay) && !commandData.OSDEnabled.HasValue()),
1494+
VerifyOrReturn((HasFeature(Feature::kOnScreenDisplay) == commandData.OSDEnabled.HasValue()),
14841495
ctx.mCommandHandler.AddStatus(ctx.mRequestPath, Status::InvalidCommand));
14851496

14861497
VerifyOrReturn(IsStreamUsageValid(streamUsage), {
@@ -1493,11 +1504,11 @@ void CameraAVStreamMgmtServer::HandleVideoStreamAllocate(HandlerContext & ctx,
14931504
ctx.mCommandHandler.AddStatus(ctx.mRequestPath, Status::InvalidCommand);
14941505
});
14951506

1496-
VerifyOrReturn(minFrameRate <= maxFrameRate, ctx.mCommandHandler.AddStatus(ctx.mRequestPath, Status::ConstraintError));
1507+
VerifyOrReturn(minFrameRate >= 1 && minFrameRate <= maxFrameRate && maxFrameRate >= 1, ctx.mCommandHandler.AddStatus(ctx.mRequestPath, Status::ConstraintError));
14971508

1498-
VerifyOrReturn(minBitRate <= maxBitRate, ctx.mCommandHandler.AddStatus(ctx.mRequestPath, Status::ConstraintError));
1509+
VerifyOrReturn(minBitRate >= 1 && minBitRate <= maxBitRate && maxBitRate >= 1, ctx.mCommandHandler.AddStatus(ctx.mRequestPath, Status::ConstraintError));
14991510

1500-
VerifyOrReturn(minFragmentLen <= maxFragmentLen, ctx.mCommandHandler.AddStatus(ctx.mRequestPath, Status::ConstraintError));
1511+
VerifyOrReturn(minFragmentLen <= maxFragmentLen && maxFragmentLen <= kMaxFragmentLenMaxValue, ctx.mCommandHandler.AddStatus(ctx.mRequestPath, Status::ConstraintError));
15011512

15021513
// Call the delegate
15031514
status =
@@ -1530,15 +1541,13 @@ void CameraAVStreamMgmtServer::HandleVideoStreamModify(HandlerContext & ctx,
15301541
// If Watermark feature is supported, then command should have the
15311542
// isWaterMarkEnabled param. Or, if it is not supported, then command should
15321543
// not have the param.
1533-
VerifyOrReturn((HasFeature(Feature::kWatermark) && commandData.watermarkEnabled.HasValue()) ||
1534-
(!HasFeature(Feature::kWatermark) && !commandData.watermarkEnabled.HasValue()),
1544+
VerifyOrReturn((HasFeature(Feature::kWatermark) == commandData.watermarkEnabled.HasValue()),
15351545
ctx.mCommandHandler.AddStatus(ctx.mRequestPath, Status::InvalidCommand));
15361546

15371547
// If OSD feature is supported, then command should have the
15381548
// isOSDEnabled param. Or, if it is not supported, then command should
15391549
// not have the param.
1540-
VerifyOrReturn((HasFeature(Feature::kOnScreenDisplay) && commandData.OSDEnabled.HasValue()) ||
1541-
(!HasFeature(Feature::kOnScreenDisplay) && !commandData.OSDEnabled.HasValue()),
1550+
VerifyOrReturn((HasFeature(Feature::kOnScreenDisplay) == commandData.OSDEnabled.HasValue()),
15421551
ctx.mCommandHandler.AddStatus(ctx.mRequestPath, Status::InvalidCommand));
15431552

15441553
// Call the delegate
@@ -1550,19 +1559,12 @@ void CameraAVStreamMgmtServer::HandleVideoStreamModify(HandlerContext & ctx,
15501559
void CameraAVStreamMgmtServer::HandleVideoStreamDeallocate(HandlerContext & ctx,
15511560
const Commands::VideoStreamDeallocate::DecodableType & commandData)
15521561
{
1553-
15541562
auto & videoStreamID = commandData.videoStreamID;
15551563

15561564
// Call the delegate
15571565
Status status = mDelegate.VideoStreamDeallocate(videoStreamID);
15581566

1559-
if (status != Status::Success)
1560-
{
1561-
ctx.mCommandHandler.AddStatus(ctx.mRequestPath, status);
1562-
return;
1563-
}
1564-
1565-
ctx.mCommandHandler.AddStatus(ctx.mRequestPath, Protocols::InteractionModel::Status::Success);
1567+
ctx.mCommandHandler.AddStatus(ctx.mRequestPath, status);
15661568
}
15671569

15681570
void CameraAVStreamMgmtServer::HandleAudioStreamAllocate(HandlerContext & ctx,
@@ -1588,8 +1590,13 @@ void CameraAVStreamMgmtServer::HandleAudioStreamAllocate(HandlerContext & ctx,
15881590
ctx.mCommandHandler.AddStatus(ctx.mRequestPath, Status::ConstraintError);
15891591
});
15901592

1591-
VerifyOrReturn(sampleRate > 0 && bitRate > 0, {
1592-
ChipLogError(Zcl, "CameraAVStreamMgmt: Invalid sampleRate or bitRate ");
1593+
VerifyOrReturn(sampleRate > 0, {
1594+
ChipLogError(Zcl, "CameraAVStreamMgmt: Invalid sampleRate");
1595+
ctx.mCommandHandler.AddStatus(ctx.mRequestPath, Status::ConstraintError);
1596+
});
1597+
1598+
VerifyOrReturn(bitRate > 0, {
1599+
ChipLogError(Zcl, "CameraAVStreamMgmt: Invalid bitRate ");
15931600
ctx.mCommandHandler.AddStatus(ctx.mRequestPath, Status::ConstraintError);
15941601
});
15951602

@@ -1598,7 +1605,7 @@ void CameraAVStreamMgmtServer::HandleAudioStreamAllocate(HandlerContext & ctx,
15981605
ctx.mCommandHandler.AddStatus(ctx.mRequestPath, Status::ConstraintError);
15991606
});
16001607

1601-
VerifyOrReturn(channelCount <= kMaxChannelCount, {
1608+
VerifyOrReturn(channelCount >=1 && channelCount <= kMaxChannelCount, {
16021609
ChipLogError(Zcl, "CameraAVStreamMgmt: Invalid channel count");
16031610
ctx.mCommandHandler.AddStatus(ctx.mRequestPath, Status::ConstraintError);
16041611
});
@@ -1615,7 +1622,6 @@ void CameraAVStreamMgmtServer::HandleAudioStreamAllocate(HandlerContext & ctx,
16151622

16161623
response.audioStreamID = audioStreamID;
16171624
ctx.mCommandHandler.AddResponse(ctx.mRequestPath, response);
1618-
ctx.mCommandHandler.AddStatus(ctx.mRequestPath, Protocols::InteractionModel::Status::Success);
16191625
}
16201626

16211627
void CameraAVStreamMgmtServer::HandleAudioStreamDeallocate(HandlerContext & ctx,
@@ -1627,13 +1633,7 @@ void CameraAVStreamMgmtServer::HandleAudioStreamDeallocate(HandlerContext & ctx,
16271633
// Call the delegate
16281634
Status status = mDelegate.AudioStreamDeallocate(audioStreamID);
16291635

1630-
if (status != Status::Success)
1631-
{
1632-
ctx.mCommandHandler.AddStatus(ctx.mRequestPath, status);
1633-
return;
1634-
}
1635-
1636-
ctx.mCommandHandler.AddStatus(ctx.mRequestPath, Protocols::InteractionModel::Status::Success);
1636+
ctx.mCommandHandler.AddStatus(ctx.mRequestPath, status);
16371637
}
16381638

16391639
void CameraAVStreamMgmtServer::HandleSnapshotStreamAllocate(HandlerContext & ctx,
@@ -1654,8 +1654,13 @@ void CameraAVStreamMgmtServer::HandleSnapshotStreamAllocate(HandlerContext & ctx
16541654
ctx.mCommandHandler.AddStatus(ctx.mRequestPath, Protocols::InteractionModel::Status::InvalidCommand);
16551655
});
16561656

1657-
VerifyOrReturn(maxFrameRate > 0 && bitRate > 0, {
1658-
ChipLogError(Zcl, "CameraAVStreamMgmt: Invalid maxFrameRate or bitRate");
1657+
VerifyOrReturn(maxFrameRate > 0, {
1658+
ChipLogError(Zcl, "CameraAVStreamMgmt: Invalid maxFrameRate");
1659+
ctx.mCommandHandler.AddStatus(ctx.mRequestPath, Status::ConstraintError);
1660+
});
1661+
1662+
VerifyOrReturn(bitRate > 0, {
1663+
ChipLogError(Zcl, "CameraAVStreamMgmt: Invalid bitRate");
16591664
ctx.mCommandHandler.AddStatus(ctx.mRequestPath, Status::ConstraintError);
16601665
});
16611666

@@ -1676,7 +1681,6 @@ void CameraAVStreamMgmtServer::HandleSnapshotStreamAllocate(HandlerContext & ctx
16761681

16771682
response.snapshotStreamID = snapshotStreamID;
16781683
ctx.mCommandHandler.AddResponse(ctx.mRequestPath, response);
1679-
ctx.mCommandHandler.AddStatus(ctx.mRequestPath, Protocols::InteractionModel::Status::Success);
16801684
}
16811685

16821686
void CameraAVStreamMgmtServer::HandleSnapshotStreamDeallocate(HandlerContext & ctx,
@@ -1688,13 +1692,7 @@ void CameraAVStreamMgmtServer::HandleSnapshotStreamDeallocate(HandlerContext & c
16881692
// Call the delegate
16891693
Status status = mDelegate.SnapshotStreamDeallocate(snapshotStreamID);
16901694

1691-
if (status != Status::Success)
1692-
{
1693-
ctx.mCommandHandler.AddStatus(ctx.mRequestPath, status);
1694-
return;
1695-
}
1696-
1697-
ctx.mCommandHandler.AddStatus(ctx.mRequestPath, Protocols::InteractionModel::Status::Success);
1695+
ctx.mCommandHandler.AddStatus(ctx.mRequestPath, status);
16981696
}
16991697

17001698
void CameraAVStreamMgmtServer::HandleSetStreamPriorities(HandlerContext & ctx,
@@ -1749,7 +1747,6 @@ void CameraAVStreamMgmtServer::HandleCaptureSnapshot(HandlerContext & ctx,
17491747
}
17501748

17511749
ctx.mCommandHandler.AddResponse(ctx.mRequestPath, response);
1752-
ctx.mCommandHandler.AddStatus(ctx.mRequestPath, Protocols::InteractionModel::Status::Success);
17531750
}
17541751

17551752
} // namespace CameraAvStreamManagement

‎src/app/clusters/camera-av-stream-management-server/camera-av-stream-management-server.h

+8
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ constexpr uint8_t kMaxMicrophoneLevel = 254;
4949
constexpr uint16_t kMaxImageRotationDegrees = 359;
5050
constexpr uint8_t kMaxChannelCount = 8;
5151
constexpr uint8_t kMaxImageQualityMetric = 100;
52+
constexpr uint16_t kMaxFragmentLenMaxValue = 65500;
5253

5354
constexpr size_t kViewportStructMaxSerializedSize =
5455
TLV::EstimateStructOverhead(sizeof(uint16_t), sizeof(uint16_t), sizeof(uint16_t), sizeof(uint16_t));
@@ -230,6 +231,12 @@ class CameraAVStreamMgmtDelegate
230231
*/
231232
virtual void OnRankedStreamPrioritiesChanged() = 0;
232233

234+
/**
235+
* @brief Delegate callback for notifying change in an attribute.
236+
*
237+
*/
238+
virtual void OnAttributeChanged(AttributeId attributeId) = 0;
239+
233240
/**
234241
* @brief Handle Command Delegate for CaptureSnapshot.
235242
*
@@ -590,6 +597,7 @@ class CameraAVStreamMgmtServer : public CommandHandlerInterface, public Attribut
590597
{
591598
ReturnErrorOnFailure(GetSafeAttributePersistenceProvider()->WriteScalarValue(path, currentValue));
592599
}
600+
mDelegate.OnAttributeChanged(attributeId);
593601
MatterReportingAttributeChangeCallback(path);
594602
}
595603
return CHIP_NO_ERROR;

0 commit comments

Comments
 (0)