[thread] delay scanning for Sleepy End Devices #62
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
LuDuda
force-pushed
the
pr/discovery-delay
branch
from
f62215b to
f785a82
Compare
| @@ -1918,7 +1952,7 @@ CHIP_ERROR GenericThreadStackManagerImpl_OpenThread<ImplClass>::_RequestSEDActiv | |||
| } | |||
|
|
|||
| template <class ImplClass> | |||
| CHIP_ERROR GenericThreadStackManagerImpl_OpenThread<ImplClass>::SEDUpdateMode() | |||
| CHIP_ERROR GenericThreadStackManagerImpl_OpenThread<ImplClass>::SEDUpdateMode(void) | |||
There was a problem hiding this comment.
Those are not C++-idiomatic. People are moving void arguments in other Matter components, so let's not move in the opposite direction :)
| { | ||
| if (mpScanCallback != nullptr) | ||
| { | ||
| mpScanCallback->OnFinished(NetworkCommissioning::Status::kUnknownError, CharSpan(), nullptr); |
There was a problem hiding this comment.
I think the callback triggers building CHIP response, so it should be run on the CHIP thread, e.g. using DeviceLayer::SystemLayer().ScheduleLambda([this]() {, right?
| @@ -411,12 +411,14 @@ GenericThreadStackManagerImpl_OpenThread<ImplClass>::_StartThreadScan(NetworkCom | |||
| mTemporaryRxOnWhenIdle = true; | |||
| linkMode.mRxOnWhenIdle = true; | |||
| otThreadSetLinkMode(mOTInst, linkMode); | |||
|
|
|||
| // Delay Thread scanning to allow Child Update Request/Response transaction to finish. | |||
| DeviceLayer::SystemLayer().StartTimer(System::Clock::Milliseconds32(kThreadScanDelayMs), RequestThreadDiscovery, this); | |||
There was a problem hiding this comment.
Suggested change
| DeviceLayer::SystemLayer().StartTimer(System::Clock::Milliseconds32(kThreadScanDelayMs), RequestThreadDiscovery, this); | |
| error = DeviceLayer::SystemLayer().StartTimer(System::Clock::Milliseconds32(kThreadScanDelayMs), RequestThreadDiscovery, this); |
| error = MapOpenThreadError(otThreadDiscover(mOTInst, 0, /* all channels */ | ||
| OT_PANID_BROADCAST, false, false, /* disable PAN ID, EUI64 and Joiner filtering */ | ||
| _OnNetworkScanFinished, this)); | ||
| error = StartThreadDiscovery(); | ||
|
|
||
| exit: | ||
| Impl()->UnlockThreadStack(); |
There was a problem hiding this comment.
Might be good to add code for restoring RxOnWhenIdle to cover the case that DeviceLayer::SystemLayer().StartTimer (or anything else) returns an error.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When SED changes its mode from RxOffWhenIdle to RxOnWhenIdle (by sending Child Update Request) and immediately starts the Thread Discovery procedure (on all channels) it is not able to receive the Child Update Response from its parent. This causes a lot of retransmissions from the parent side and even sporadic re-attachments.
This PR adds a small 300-ms delay to allow the child to receive the Child Update Response.