Skip to content

Commit

Permalink
Fix some -Wconversion warnings in src/controller. (#25325)
Browse files Browse the repository at this point in the history
  • Loading branch information
bzbarsky-apple authored and pull[bot] committed Nov 10, 2023
1 parent bf37197 commit 1254129
Show file tree
Hide file tree
Showing 4 changed files with 34 additions and 30 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@

#include <lib/core/CHIPPersistentStorageDelegate.h>
#include <lib/support/CodeUtils.h>
#include <lib/support/SafeInt.h>
#include <lib/support/logging/CHIPLogging.h>

namespace chip {
Expand All @@ -39,7 +40,13 @@ CHIP_ERROR PythonPersistentStorageDelegate::SyncGetKeyValue(const char * key, vo
return CHIP_ERROR_PERSISTED_STORAGE_VALUE_NOT_FOUND;
}

uint16_t neededSize = val->second.size();
if (!CanCastTo<uint16_t>(val->second.size()))
{
size = 0;
return CHIP_ERROR_BUFFER_TOO_SMALL;
}

uint16_t neededSize = static_cast<uint16_t>(val->second.size());
ReturnErrorCodeIf(size == 0 && neededSize == 0, CHIP_NO_ERROR);
ReturnErrorCodeIf(value == nullptr, CHIP_ERROR_BUFFER_TOO_SMALL);

Expand Down
4 changes: 2 additions & 2 deletions src/controller/python/chip/clusters/Attribute.py
Original file line number Diff line number Diff line change
Expand Up @@ -977,8 +977,8 @@ def WriteGroupAttributes(groupId: int, devCtrl: c_void_p, attributes: List[Attri

# This struct matches the PyReadAttributeParams in attribute.cpp, for passing various params together.
_ReadParams = construct.Struct(
"MinInterval" / construct.Int32ul,
"MaxInterval" / construct.Int32ul,
"MinInterval" / construct.Int16ul,
"MaxInterval" / construct.Int16ul,
"IsSubscription" / construct.Flag,
"IsFabricFiltered" / construct.Flag,
"KeepSubscriptions" / construct.Flag,
Expand Down
4 changes: 2 additions & 2 deletions src/controller/python/chip/clusters/attribute.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -238,8 +238,8 @@ extern "C" {

struct __attribute__((packed)) PyReadAttributeParams
{
uint32_t minInterval; // MinInterval in subscription request
uint32_t maxInterval; // MaxInterval in subscription request
uint16_t minInterval; // MinInterval in subscription request
uint16_t maxInterval; // MaxInterval in subscription request
bool isSubscription;
bool isFabricFiltered;
bool keepSubscriptions;
Expand Down
47 changes: 22 additions & 25 deletions src/controller/tests/data_model/TestRead.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2958,12 +2958,12 @@ class TestReadCallback : public app::ReadClient::Callback
mLastError = CHIP_NO_ERROR;
}

int32_t mAttributeCount = 0;
int32_t mOnReportEnd = 0;
int32_t mOnSubscriptionEstablishedCount = 0;
int32_t mOnDone = 0;
int32_t mOnError = 0;
CHIP_ERROR mLastError = CHIP_NO_ERROR;
uint32_t mAttributeCount = 0;
uint32_t mOnReportEnd = 0;
uint32_t mOnSubscriptionEstablishedCount = 0;
uint32_t mOnDone = 0;
uint32_t mOnError = 0;
CHIP_ERROR mLastError = CHIP_NO_ERROR;
};

class TestPerpetualListReadCallback : public app::ReadClient::Callback
Expand Down Expand Up @@ -2991,7 +2991,7 @@ class TestPerpetualListReadCallback : public app::ReadClient::Callback
int32_t reportsReceived = 0;
};

void EstablishReadOrSubscriptions(nlTestSuite * apSuite, const SessionHandle & sessionHandle, int32_t numSubs, int32_t pathPerSub,
void EstablishReadOrSubscriptions(nlTestSuite * apSuite, const SessionHandle & sessionHandle, size_t numSubs, size_t pathPerSub,
app::AttributePathParams path, app::ReadClient::InteractionType type,
app::ReadClient::Callback * callback, std::vector<std::unique_ptr<app::ReadClient>> & readClients)
{
Expand All @@ -3006,7 +3006,7 @@ void EstablishReadOrSubscriptions(nlTestSuite * apSuite, const SessionHandle & s
readParams.mKeepSubscriptions = true;
}

for (int32_t i = 0; i < numSubs; i++)
for (uint32_t i = 0; i < numSubs; i++)
{
std::unique_ptr<app::ReadClient> readClient =
std::make_unique<app::ReadClient>(app::InteractionModelEngine::GetInstance(),
Expand Down Expand Up @@ -3066,9 +3066,9 @@ void TestReadInteraction::TestReadHandler_KillOverQuotaSubscriptions(nlTestSuite
TestContext & ctx = *static_cast<TestContext *>(apContext);
auto sessionHandle = ctx.GetSessionBobToAlice();

const int32_t kExpectedParallelSubs =
const auto kExpectedParallelSubs =
app::InteractionModelEngine::kMinSupportedSubscriptionsPerFabric * ctx.GetFabricTable().FabricCount();
const int32_t kExpectedParallelPaths = kExpectedParallelSubs * app::InteractionModelEngine::kMinSupportedPathsPerSubscription;
const auto kExpectedParallelPaths = kExpectedParallelSubs * app::InteractionModelEngine::kMinSupportedPathsPerSubscription;

app::InteractionModelEngine::GetInstance()->RegisterReadHandlerAppCallback(&gTestReadInteraction);

Expand Down Expand Up @@ -3117,23 +3117,23 @@ void TestReadInteraction::TestReadHandler_KillOverQuotaSubscriptions(nlTestSuite
ctx.GetIOContext().DriveIOUntil(System::Clock::Seconds16(5), [&]() {
return readCallback.mOnSubscriptionEstablishedCount == kExpectedParallelSubs + 1 &&
readCallback.mAttributeCount ==
static_cast<int32_t>(kExpectedParallelSubs * app::InteractionModelEngine::kMinSupportedPathsPerSubscription +
app::InteractionModelEngine::kMinSupportedPathsPerSubscription + 1);
kExpectedParallelSubs * app::InteractionModelEngine::kMinSupportedPathsPerSubscription +
app::InteractionModelEngine::kMinSupportedPathsPerSubscription + 1;
});

NL_TEST_ASSERT(apSuite,
readCallback.mAttributeCount ==
static_cast<int32_t>(kExpectedParallelSubs * app::InteractionModelEngine::kMinSupportedPathsPerSubscription +
app::InteractionModelEngine::kMinSupportedPathsPerSubscription + 1));
kExpectedParallelSubs * app::InteractionModelEngine::kMinSupportedPathsPerSubscription +
app::InteractionModelEngine::kMinSupportedPathsPerSubscription + 1);
NL_TEST_ASSERT(apSuite, readCallback.mOnSubscriptionEstablishedCount == kExpectedParallelSubs + 1);

// We have set up the environment for testing the evicting logic.
// We now have a full stable of subscriptions setup AND we've artificially limited the capacity, creation of further
// subscriptions will require the eviction of existing subscriptions, OR potential rejection of the subscription if it exceeds
// minimas.
app::InteractionModelEngine::GetInstance()->SetForceHandlerQuota(true);
app::InteractionModelEngine::GetInstance()->SetHandlerCapacityForSubscriptions(kExpectedParallelSubs);
app::InteractionModelEngine::GetInstance()->SetPathPoolCapacityForSubscriptions(kExpectedParallelPaths);
app::InteractionModelEngine::GetInstance()->SetHandlerCapacityForSubscriptions(static_cast<int32_t>(kExpectedParallelSubs));
app::InteractionModelEngine::GetInstance()->SetPathPoolCapacityForSubscriptions(static_cast<int32_t>(kExpectedParallelPaths));

// Part 1: Test per subscription minimas.
// Rejection of the subscription that exceeds minimas.
Expand Down Expand Up @@ -3291,18 +3291,18 @@ void TestReadInteraction::TestReadHandler_KillOldestSubscriptions(nlTestSuite *
TestContext & ctx = *static_cast<TestContext *>(apContext);
auto sessionHandle = ctx.GetSessionBobToAlice();

const int32_t kExpectedParallelSubs =
const auto kExpectedParallelSubs =
app::InteractionModelEngine::kMinSupportedSubscriptionsPerFabric * ctx.GetFabricTable().FabricCount();
const int32_t kExpectedParallelPaths = kExpectedParallelSubs * app::InteractionModelEngine::kMinSupportedPathsPerSubscription;
const auto kExpectedParallelPaths = kExpectedParallelSubs * app::InteractionModelEngine::kMinSupportedPathsPerSubscription;

app::InteractionModelEngine::GetInstance()->RegisterReadHandlerAppCallback(&gTestReadInteraction);

TestReadCallback readCallback;
std::vector<std::unique_ptr<app::ReadClient>> readClients;

app::InteractionModelEngine::GetInstance()->SetForceHandlerQuota(true);
app::InteractionModelEngine::GetInstance()->SetHandlerCapacityForSubscriptions(kExpectedParallelSubs);
app::InteractionModelEngine::GetInstance()->SetPathPoolCapacityForSubscriptions(kExpectedParallelPaths);
app::InteractionModelEngine::GetInstance()->SetHandlerCapacityForSubscriptions(static_cast<int32_t>(kExpectedParallelSubs));
app::InteractionModelEngine::GetInstance()->SetPathPoolCapacityForSubscriptions(static_cast<int32_t>(kExpectedParallelPaths));

// This should just use all availbale resources.
EstablishReadOrSubscriptions(
Expand All @@ -3314,12 +3314,9 @@ void TestReadInteraction::TestReadHandler_KillOldestSubscriptions(nlTestSuite *

NL_TEST_ASSERT(apSuite,
readCallback.mAttributeCount ==
kExpectedParallelSubs *
static_cast<int32_t>(app::InteractionModelEngine::kMinSupportedPathsPerSubscription));
kExpectedParallelSubs * app::InteractionModelEngine::kMinSupportedPathsPerSubscription);
NL_TEST_ASSERT(apSuite, readCallback.mOnSubscriptionEstablishedCount == kExpectedParallelSubs);
NL_TEST_ASSERT(apSuite,
app::InteractionModelEngine::GetInstance()->GetNumActiveReadHandlers() ==
static_cast<size_t>(kExpectedParallelSubs));
NL_TEST_ASSERT(apSuite, app::InteractionModelEngine::GetInstance()->GetNumActiveReadHandlers() == kExpectedParallelSubs);

// The following check will trigger the logic in im to kill the read handlers that uses more paths than the limit per fabric.
{
Expand Down

0 comments on commit 1254129

Please sign in to comment.