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.

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

Thread Scan Network command handling for end devices #15105

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
48 changes: 45 additions & 3 deletions src/platform/OpenThread/GenericNetworkCommissioningThreadDriver.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,48 @@ namespace chip {
namespace DeviceLayer {
namespace NetworkCommissioning {

template <typename T>
class otScanResponseIterator : public Iterator<T>
{
public:
otScanResponseIterator(T * apScanResponse) : mpScanResponse(apScanResponse) {}
size_t Count() override { return itemCount; }
bool Next(T & item) override
{
if (mpScanResponse == nullptr || currentIterating >= itemCount)
{
return false;
}
item = mpScanResponse[currentIterating];
currentIterating++;
return true;
}
void Release() override
{
itemCount = currentIterating = 0;
Platform::MemoryFree(mpScanResponse);
mpScanResponse = nullptr;
}

void Add(T * pResponse)
{
size_t tempCount = itemCount + 1;
mpScanResponse = static_cast<T *>(Platform::MemoryRealloc(mpScanResponse, kItemSize * tempCount));
if (mpScanResponse)
{
// first item at index. update after the copy.
memcpy(&(mpScanResponse[itemCount]), pResponse, kItemSize);
itemCount = tempCount;
}
}

private:
size_t currentIterating = 0;
size_t itemCount = 0;
size_t kItemSize = sizeof(T);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use static constexpr not to occupy memory for that?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I did add the constexpr first, but the compiler was complaining that I wasn't assigning a constant value with sizeof(T). Let me try adding static also that might have been my mistake. If that works I'll add

T * mpScanResponse;
};

class GenericThreadDriver final : public ThreadDriver
{
public:
Expand Down Expand Up @@ -62,9 +104,9 @@ class GenericThreadDriver final : public ThreadDriver
void ScanNetworks(ScanCallback * callback) override;

private:
ThreadNetworkIterator mThreadIterator = ThreadNetworkIterator(this);
Thread::OperationalDataset mSavedNetwork;
Thread::OperationalDataset mStagingNetwork;
ThreadNetworkIterator mThreadIterator = ThreadNetworkIterator(this);
Thread::OperationalDataset mSavedNetwork = {};
Thread::OperationalDataset mStagingNetwork = {};
};

} // namespace NetworkCommissioning
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,8 @@ void initNetworkCommissioningThreadDriver(void)
#endif
}

NetworkCommissioning::ThreadScanResponse * scanResult;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should it be under #ifndef _NO_NETWORK_COMMISSIONING_DRIVER_, too? Also, nit, looks like we prefix globals in this file with s.

Copy link
Member Author

@jmartinez-silabs jmartinez-silabs Feb 15, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reasoning why this works here is that the NetworkCommissioning::ThreadScanResponse type is known by all has the header is included. But the cluster and the implementation of it isn't if the example does not have a zap file with the cluster configured. I can add a few #ifndef NO_NETWORK_COMMISSIONING_DRIVER over this whole scan functionality but I don't think it will impact the code behaviour if I leave it.

also gotcha for the s. Will do

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, no need to change that then.

otScanResponseIterator<NetworkCommissioning::ThreadScanResponse> mScanResponseIter(scanResult);
} // namespace
// Fully instantiate the generic implementation class in whatever compilation unit includes this file.
template class GenericThreadStackManagerImpl_OpenThread<ThreadStackManagerImpl>;
Expand Down Expand Up @@ -359,11 +361,55 @@ void GenericThreadStackManagerImpl_OpenThread<ImplClass>::_OnThreadAttachFinishe
template <class ImplClass>
CHIP_ERROR GenericThreadStackManagerImpl_OpenThread<ImplClass>::_StartThreadScan(ThreadDriver::ScanCallback * callback)
{
// TODO END scan feature + _OnNetworkScanFinished callback for response
// There is another ongoing scan request, reject the new one.
// If there is another ongoing scan request, reject the new one.
VerifyOrReturnError(mpScanCallback == nullptr, CHIP_ERROR_INCORRECT_STATE);
mpScanCallback = callback;
return CHIP_NO_ERROR;
CHIP_ERROR err = MapOpenThreadError(otLinkActiveScan(mOTInst, 0, /* all channels */
0, /* default value `kScanDurationDefault` = 300 ms. */
selissia marked this conversation as resolved.
Show resolved Hide resolved
_OnNetworkScanFinished, this));
return err;
}

template <class ImplClass>
void GenericThreadStackManagerImpl_OpenThread<ImplClass>::_OnNetworkScanFinished(otActiveScanResult * aResult, void * aContext)
{
reinterpret_cast<GenericThreadStackManagerImpl_OpenThread *>(aContext)->_OnNetworkScanFinished(aResult);
}

template <class ImplClass>
void GenericThreadStackManagerImpl_OpenThread<ImplClass>::_OnNetworkScanFinished(otActiveScanResult * aResult)
{
if (aResult == nullptr) // scan completed
{
if (mpScanCallback != nullptr)
{
DeviceLayer::SystemLayer().ScheduleLambda([this]() {
mpScanCallback->OnFinished(NetworkCommissioning::Status::kSuccess, CharSpan(), &mScanResponseIter);
mpScanCallback = nullptr;
});
}
}
else
{
ChipLogProgress(
DeviceLayer,
"Thread Network: %s Panid 0x%" PRIx16 " Channel %" PRIu16 " RSSI %" PRId16 " LQI %" PRIu16 " Version %" PRIu16,
aResult->mNetworkName.m8, aResult->mPanId, aResult->mChannel, aResult->mRssi, aResult->mLqi, aResult->mVersion);

NetworkCommissioning::ThreadScanResponse scanResponse = { 0 };

scanResponse.panId = aResult->mPanId; // why is scanResponse.panID 64b
scanResponse.channel = aResult->mChannel; // why is scanResponse.channel 16b
scanResponse.version = aResult->mVersion;
scanResponse.rssi = aResult->mRssi;
scanResponse.lqi = aResult->mLqi;
scanResponse.extendedAddress = Encoding::BigEndian::Get64(aResult->mExtAddress.m8);
scanResponse.extendedPanId = Encoding::BigEndian::Get64(aResult->mExtendedPanId.m8);
scanResponse.networkNameLen = strnlen(aResult->mNetworkName.m8, OT_NETWORK_NAME_MAX_SIZE);
memcpy(scanResponse.networkName, aResult->mNetworkName.m8, scanResponse.networkNameLen);

mScanResponseIter.Add(&scanResponse);
}
}

template <class ImplClass>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
#pragma once

#include <openthread/instance.h>
#include <openthread/link.h>
#include <openthread/netdata.h>

#if CHIP_DEVICE_CONFIG_ENABLE_THREAD_SRP_CLIENT
Expand Down Expand Up @@ -89,6 +90,8 @@ class GenericThreadStackManagerImpl_OpenThread
ConnectivityManager::ThreadDeviceType _GetThreadDeviceType(void);
CHIP_ERROR _SetThreadDeviceType(ConnectivityManager::ThreadDeviceType deviceType);
CHIP_ERROR _StartThreadScan(NetworkCommissioning::ThreadDriver::ScanCallback * callback);
static void _OnNetworkScanFinished(otActiveScanResult * aResult, void * aContext);
void _OnNetworkScanFinished(otActiveScanResult * aResult);

#if CHIP_DEVICE_CONFIG_ENABLE_SED
CHIP_ERROR _GetSEDPollingConfig(ConnectivityManager::SEDPollingConfig & pollingConfig);
Expand Down