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

Fix Thread diagnostic nullable attributes so they read as null when … #26960

Merged
merged 5 commits into from
Jun 1, 2023
Merged

Fix Thread diagnostic nullable attributes so they read as null when … #26960

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
0a406ac
Fix Thread diagnostic nullable attributes so they read as null when t…
jmartinez-silabs May 30, 2023
cf3e89f
Revert "Fix Thread diagnostic nullable attributes so they read as nul…
jmartinez-silabs May 31, 2023
80259de
Simplify how the nullEncode cases are caught. The condition is based …
jmartinez-silabs May 31, 2023
bb26960
update comment
jmartinez-silabs May 31, 2023
3200088
Apply suggestions from code review
jmartinez-silabs Jun 1, 2023
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
29 changes: 29 additions & 0 deletions src/app/AttributeAccessInterface.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,35 @@ class AttributeValueEncoder
return Encode(DataModel::List<uint8_t>());
}

/**
* @brief Conditionaly uses the provided aCallback to encode the attribute data.
* When the condition evaluates to false, it will encode null instead by using EncodeNull
*
* aCallback is expected to take a const auto argument (which will convert to a AttributeValueEncoder pointer)
* and encode, with the available method of this class, an attribute data.
* Any error during its process needs to be directly returned to the user as a CHIP_ERROR
*
* This method should only be used to encode nullable attributes
* aCallback is not called when the condition evaluates to false
*
* @tparam AttributeEncoder
* @param condition When true, use aCallback to encode the attribute data. When false, encode null
* @param aCallback Provide your lamda function encoding the data.
* @return CHIP_ERROR Encoding succeed or failed with error code
*/
template <typename AttributeEncoder>
CHIP_ERROR EncodeNullableOnCondition(bool condition, AttributeEncoder aCallback)
jmartinez-silabs marked this conversation as resolved.
Show resolved Hide resolved
{
if (condition)
{
return aCallback(this);
}
else
{
return EncodeNull();
}
}

/**
* aCallback is expected to take a const auto & argument and Encode() on it as many times as needed to encode all the list
* elements one by one. If any of those Encode() calls returns failure, aCallback must stop encoding and return failure. When
Expand Down
200 changes: 104 additions & 96 deletions src/platform/OpenThread/GenericThreadStackManagerImpl_OpenThread.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1036,7 +1036,7 @@ void GenericThreadStackManagerImpl_OpenThread<ImplClass>::_ResetThreadNetworkDia
* The info is encoded via the AttributeValueEncoder.
*
* @param attributeId Id of the attribute for the requested info.
* @param aEncoder Encoder to encode the attribute value.
* @param encoder Encoder to encode the attribute value.
*
* @return CHIP_NO_ERROR = Succes.
* CHIP_ERROR_NOT_IMPLEMENTED = Runtime value for this attribute to yet available to send as reply
Expand All @@ -1055,82 +1055,94 @@ CHIP_ERROR GenericThreadStackManagerImpl_OpenThread<ImplClass>::_WriteThreadNetw
switch (attributeId)
{
case ThreadNetworkDiagnostics::Attributes::Channel::Id: {
uint16_t channel = otLinkGetChannel(mOTInst);
err = encoder.Encode(channel);
err = encoder.EncodeNullableOnCondition(mIsAttached, [this](const auto pAttrEncoder) -> CHIP_ERROR {
jmartinez-silabs marked this conversation as resolved.
Show resolved Hide resolved
uint16_t channel = otLinkGetChannel(mOTInst);
return pAttrEncoder->Encode(channel);
});
}
break;

case ThreadNetworkDiagnostics::Attributes::RoutingRole::Id: {
using ThreadNetworkDiagnostics::RoutingRoleEnum;
RoutingRoleEnum routingRole;
otDeviceRole otRole = otThreadGetDeviceRole(mOTInst);
err = encoder.EncodeNullableOnCondition(mIsAttached, [this](const auto pAttrEncoder) -> CHIP_ERROR {
using ThreadNetworkDiagnostics::RoutingRoleEnum;
RoutingRoleEnum routingRole;
otDeviceRole otRole = otThreadGetDeviceRole(mOTInst);

if (otRole == OT_DEVICE_ROLE_DISABLED)
{
routingRole = RoutingRoleEnum::kUnspecified;
}
else if (otRole == OT_DEVICE_ROLE_DETACHED)
{
routingRole = RoutingRoleEnum::kUnassigned;
}
else if (otRole == OT_DEVICE_ROLE_ROUTER)
{
routingRole = RoutingRoleEnum::kRouter;
}
else if (otRole == OT_DEVICE_ROLE_LEADER)
{
routingRole = RoutingRoleEnum::kLeader;
}
else if (otRole == OT_DEVICE_ROLE_CHILD)
{
otLinkModeConfig linkMode = otThreadGetLinkMode(mOTInst);

if (linkMode.mRxOnWhenIdle)
if (otRole == OT_DEVICE_ROLE_DISABLED)
{
routingRole = RoutingRoleEnum::kUnspecified;
}
else if (otRole == OT_DEVICE_ROLE_DETACHED)
{
routingRole = RoutingRoleEnum::kUnassigned;
}
else if (otRole == OT_DEVICE_ROLE_ROUTER)
{
routingRole = RoutingRoleEnum::kEndDevice;
routingRole = RoutingRoleEnum::kRouter;
}
else if (otRole == OT_DEVICE_ROLE_LEADER)
{
routingRole = RoutingRoleEnum::kLeader;
}
else if (otRole == OT_DEVICE_ROLE_CHILD)
{
otLinkModeConfig linkMode = otThreadGetLinkMode(mOTInst);

if (linkMode.mRxOnWhenIdle)
{
routingRole = RoutingRoleEnum::kEndDevice;
#if CHIP_DEVICE_CONFIG_THREAD_FTD
if (otThreadIsRouterEligible(mOTInst))
if (otThreadIsRouterEligible(mOTInst))
{
routingRole = RoutingRoleEnum::kReed;
}
#endif
}
else
{
routingRole = RoutingRoleEnum::kReed;
routingRole = RoutingRoleEnum::kSleepyEndDevice;
}
#endif
}
else
{
routingRole = RoutingRoleEnum::kSleepyEndDevice;
}
}

err = encoder.Encode(routingRole);
return pAttrEncoder->Encode(routingRole);
});
}
break;

case ThreadNetworkDiagnostics::Attributes::NetworkName::Id: {
const char * networkName = otThreadGetNetworkName(mOTInst);
err = encoder.Encode(CharSpan::fromCharString(networkName));
err = encoder.EncodeNullableOnCondition(mIsAttached, [this](const auto pAttrEncoder) -> CHIP_ERROR {
const char * networkName = otThreadGetNetworkName(mOTInst);
return pAttrEncoder->Encode(CharSpan::fromCharString(networkName));
});
}
break;

case ThreadNetworkDiagnostics::Attributes::PanId::Id: {
uint16_t panId = otLinkGetPanId(mOTInst);
err = encoder.Encode(panId);
err = encoder.EncodeNullableOnCondition(mIsAttached, [this](const auto pAttrEncoder) -> CHIP_ERROR {
uint16_t panId = otLinkGetPanId(mOTInst);
return pAttrEncoder->Encode(panId);
});
}
break;

case ThreadNetworkDiagnostics::Attributes::ExtendedPanId::Id: {
const otExtendedPanId * pExtendedPanid = otThreadGetExtendedPanId(mOTInst);
err = encoder.Encode(Encoding::BigEndian::Get64(pExtendedPanid->m8));
err = encoder.EncodeNullableOnCondition(mIsAttached, [this](const auto pAttrEncoder) -> CHIP_ERROR {
jmartinez-silabs marked this conversation as resolved.
Show resolved Hide resolved
const otExtendedPanId * pExtendedPanid = otThreadGetExtendedPanId(mOTInst);
return pAttrEncoder->Encode(Encoding::BigEndian::Get64(pExtendedPanid->m8));
});
}
break;

case ThreadNetworkDiagnostics::Attributes::MeshLocalPrefix::Id: {
uint8_t meshLocaPrefix[OT_MESH_LOCAL_PREFIX_SIZE + 1] = { 0 }; // + 1 to encode prefix Len in the octstr
err = encoder.EncodeNullableOnCondition(mIsAttached, [this](const auto pAttrEncoder) -> CHIP_ERROR {
uint8_t meshLocaPrefix[OT_MESH_LOCAL_PREFIX_SIZE + 1] = { 0 }; // + 1 to encode prefix Len in the octstr

const otMeshLocalPrefix * pMeshLocalPrefix = otThreadGetMeshLocalPrefix(mOTInst);
meshLocaPrefix[0] = OT_IP6_PREFIX_BITSIZE;
const otMeshLocalPrefix * pMeshLocalPrefix = otThreadGetMeshLocalPrefix(mOTInst);
meshLocaPrefix[0] = OT_IP6_PREFIX_BITSIZE;

memcpy(&meshLocaPrefix[1], pMeshLocalPrefix->m8, OT_MESH_LOCAL_PREFIX_SIZE);
err = encoder.Encode(ByteSpan(meshLocaPrefix));
memcpy(&meshLocaPrefix[1], pMeshLocalPrefix->m8, OT_MESH_LOCAL_PREFIX_SIZE);
return pAttrEncoder->Encode(ByteSpan(meshLocaPrefix));
});
}
break;

Expand Down Expand Up @@ -1255,32 +1267,42 @@ CHIP_ERROR GenericThreadStackManagerImpl_OpenThread<ImplClass>::_WriteThreadNetw
break;

case ThreadNetworkDiagnostics::Attributes::PartitionId::Id: {
uint32_t partitionId = otThreadGetPartitionId(mOTInst);
err = encoder.Encode(partitionId);
err = encoder.EncodeNullableOnCondition(mIsAttached, [this](const auto pAttrEncoder) -> CHIP_ERROR {
uint32_t partitionId = otThreadGetPartitionId(mOTInst);
return pAttrEncoder->Encode(partitionId);
});
}
break;

case ThreadNetworkDiagnostics::Attributes::Weighting::Id: {
uint8_t weight = otThreadGetLeaderWeight(mOTInst);
err = encoder.Encode(weight);
err = encoder.EncodeNullableOnCondition(mIsAttached, [this](const auto pAttrEncoder) -> CHIP_ERROR {
jmartinez-silabs marked this conversation as resolved.
Show resolved Hide resolved
uint8_t weight = otThreadGetLeaderWeight(mOTInst);
return pAttrEncoder->Encode(weight);
});
}
break;

case ThreadNetworkDiagnostics::Attributes::DataVersion::Id: {
uint8_t dataVersion = otNetDataGetVersion(mOTInst);
err = encoder.Encode(dataVersion);
err = encoder.EncodeNullableOnCondition(mIsAttached, [this](const auto pAttrEncoder) -> CHIP_ERROR {
uint8_t dataVersion = otNetDataGetVersion(mOTInst);
return pAttrEncoder->Encode(dataVersion);
});
}
break;

case ThreadNetworkDiagnostics::Attributes::StableDataVersion::Id: {
uint8_t stableVersion = otNetDataGetStableVersion(mOTInst);
err = encoder.Encode(stableVersion);
err = encoder.EncodeNullableOnCondition(mIsAttached, [this](const auto pAttrEncoder) -> CHIP_ERROR {
uint8_t stableVersion = otNetDataGetStableVersion(mOTInst);
return pAttrEncoder->Encode(stableVersion);
});
}
break;

case ThreadNetworkDiagnostics::Attributes::LeaderRouterId::Id: {
uint8_t leaderRouterId = otThreadGetLeaderRouterId(mOTInst);
err = encoder.Encode(leaderRouterId);
err = encoder.EncodeNullableOnCondition(mIsAttached, [this](const auto pAttrEncoder) -> CHIP_ERROR {
uint8_t leaderRouterId = otThreadGetLeaderRouterId(mOTInst);
return pAttrEncoder->Encode(leaderRouterId);
});
}
break;

Expand Down Expand Up @@ -1537,62 +1559,53 @@ CHIP_ERROR GenericThreadStackManagerImpl_OpenThread<ImplClass>::_WriteThreadNetw
break;

case ThreadNetworkDiagnostics::Attributes::ActiveTimestamp::Id: {
err = CHIP_ERROR_INCORRECT_STATE;
if (otDatasetIsCommissioned(mOTInst))
{
err = encoder.EncodeNullableOnCondition(otDatasetIsCommissioned(mOTInst), [this](const auto pAttrEncoder) -> CHIP_ERROR {
otOperationalDataset activeDataset;
otError otErr = otDatasetGetActive(mOTInst, &activeDataset);
VerifyOrExit(otErr == OT_ERROR_NONE, err = MapOpenThreadError(otErr));
VerifyOrReturnError(otErr == OT_ERROR_NONE, MapOpenThreadError(otErr));
#if OPENTHREAD_API_VERSION >= 219
uint64_t activeTimestamp = (activeDataset.mActiveTimestamp.mSeconds << 16) |
(activeDataset.mActiveTimestamp.mTicks << 1) | activeDataset.mActiveTimestamp.mAuthoritative;
#else
uint64_t activeTimestamp = activeDataset.mActiveTimestamp;
#endif
err = encoder.Encode(activeTimestamp);
}
return pAttrEncoder->Encode(activeTimestamp);
});
}
break;

case ThreadNetworkDiagnostics::Attributes::PendingTimestamp::Id: {
err = CHIP_ERROR_INCORRECT_STATE;
if (otDatasetIsCommissioned(mOTInst))
{
err = encoder.EncodeNullableOnCondition(otDatasetIsCommissioned(mOTInst), [this](const auto pAttrEncoder) -> CHIP_ERROR {
otOperationalDataset activeDataset;
otError otErr = otDatasetGetActive(mOTInst, &activeDataset);
VerifyOrExit(otErr == OT_ERROR_NONE, err = MapOpenThreadError(otErr));
VerifyOrReturnError(otErr == OT_ERROR_NONE, MapOpenThreadError(otErr));
#if OPENTHREAD_API_VERSION >= 219
uint64_t pendingTimestamp = (activeDataset.mPendingTimestamp.mSeconds << 16) |
(activeDataset.mPendingTimestamp.mTicks << 1) | activeDataset.mPendingTimestamp.mAuthoritative;
#else
uint64_t pendingTimestamp = activeDataset.mPendingTimestamp;
#endif
err = encoder.Encode(pendingTimestamp);
}
return pAttrEncoder->Encode(pendingTimestamp);
});
}
break;

case ThreadNetworkDiagnostics::Attributes::Delay::Id: {
err = CHIP_ERROR_INCORRECT_STATE;
if (otDatasetIsCommissioned(mOTInst))
{
err = encoder.EncodeNullableOnCondition(otDatasetIsCommissioned(mOTInst), [this](const auto pAttrEncoder) -> CHIP_ERROR {
otOperationalDataset activeDataset;
otError otErr = otDatasetGetActive(mOTInst, &activeDataset);
VerifyOrExit(otErr == OT_ERROR_NONE, err = MapOpenThreadError(otErr));
VerifyOrReturnError(otErr == OT_ERROR_NONE, MapOpenThreadError(otErr));
uint32_t delay = activeDataset.mDelay;
err = encoder.Encode(delay);
}
return pAttrEncoder->Encode(delay);
});
}
break;

case ThreadNetworkDiagnostics::Attributes::SecurityPolicy::Id: {
err = CHIP_ERROR_INCORRECT_STATE;

if (otDatasetIsCommissioned(mOTInst))
{
err = encoder.EncodeNullableOnCondition(otDatasetIsCommissioned(mOTInst), [this](const auto pAttrEncoder) -> CHIP_ERROR {
otOperationalDataset activeDataset;
otError otErr = otDatasetGetActive(mOTInst, &activeDataset);
VerifyOrExit(otErr == OT_ERROR_NONE, err = MapOpenThreadError(otErr));
VerifyOrReturnError(otErr == OT_ERROR_NONE, MapOpenThreadError(otErr));

ThreadNetworkDiagnostics::Structs::SecurityPolicy::Type securityPolicy;
static_assert(sizeof(securityPolicy) == sizeof(activeDataset.mSecurityPolicy),
Expand All @@ -1604,18 +1617,16 @@ CHIP_ERROR GenericThreadStackManagerImpl_OpenThread<ImplClass>::_WriteThreadNetw
securityPolicy.rotationTime = policyAsInts[0];
securityPolicy.flags = policyAsInts[1];

err = encoder.Encode(securityPolicy);
}
return pAttrEncoder->Encode(securityPolicy);
});
}
break;

case ThreadNetworkDiagnostics::Attributes::ChannelPage0Mask::Id: {
err = CHIP_ERROR_INCORRECT_STATE;
if (otDatasetIsCommissioned(mOTInst))
{
err = encoder.EncodeNullableOnCondition(otDatasetIsCommissioned(mOTInst), [this](const auto pAttrEncoder) -> CHIP_ERROR {
otOperationalDataset activeDataset;
otError otErr = otDatasetGetActive(mOTInst, &activeDataset);
VerifyOrExit(otErr == OT_ERROR_NONE, err = MapOpenThreadError(otErr));
VerifyOrReturnError(otErr == OT_ERROR_NONE, MapOpenThreadError(otErr));

// In the resultant Octet string, the most significant bit of the left-most byte indicates channel 0
// We have to bitswap the entire uint32_t before converting to octet string
Expand All @@ -1627,18 +1638,16 @@ CHIP_ERROR GenericThreadStackManagerImpl_OpenThread<ImplClass>::_WriteThreadNetw

uint8_t buffer[sizeof(uint32_t)] = { 0 };
Encoding::BigEndian::Put32(buffer, bitSwappedChannelMask);
err = encoder.Encode(ByteSpan(buffer));
}
return pAttrEncoder->Encode(ByteSpan(buffer));
});
}
break;

case ThreadNetworkDiagnostics::Attributes::OperationalDatasetComponents::Id: {
err = CHIP_ERROR_INCORRECT_STATE;
if (otDatasetIsCommissioned(mOTInst))
{
err = encoder.EncodeNullableOnCondition(otDatasetIsCommissioned(mOTInst), [this](const auto pAttrEncoder) -> CHIP_ERROR {
otOperationalDataset activeDataset;
otError otErr = otDatasetGetActive(mOTInst, &activeDataset);
VerifyOrExit(otErr == OT_ERROR_NONE, err = MapOpenThreadError(otErr));
VerifyOrReturnError(otErr == OT_ERROR_NONE, MapOpenThreadError(otErr));
ThreadNetworkDiagnostics::Structs::OperationalDatasetComponents::Type OpDatasetComponents;

OpDatasetComponents.activeTimestampPresent = activeDataset.mComponents.mIsActiveTimestampPresent;
Expand All @@ -1654,8 +1663,8 @@ CHIP_ERROR GenericThreadStackManagerImpl_OpenThread<ImplClass>::_WriteThreadNetw
OpDatasetComponents.securityPolicyPresent = activeDataset.mComponents.mIsSecurityPolicyPresent;
OpDatasetComponents.channelMaskPresent = activeDataset.mComponents.mIsChannelMaskPresent;

err = encoder.Encode(OpDatasetComponents);
}
return pAttrEncoder->Encode(OpDatasetComponents);
});
}
break;

Expand All @@ -1680,7 +1689,6 @@ CHIP_ERROR GenericThreadStackManagerImpl_OpenThread<ImplClass>::_WriteThreadNetw
break;
}

exit:
if (err != CHIP_NO_ERROR)
{
ChipLogError(DeviceLayer, "_WriteThreadNetworkDiagnosticAttributeToTlv failed: %s", ErrorStr(err));
Expand Down