Skip to content

Commit

Permalink
Merge pull request #1 from ksperling-apple/disconnect-lingering
Browse files Browse the repository at this point in the history 
Use IntrusiveList and other minor tweaks
  • Loading branch information
@DejinChen
DejinChen authored Sep 3, 2024
2 parents b4fd1f2 + d9974b0 commit f4a81a6
Show file tree
Hide file tree
Showing 3 changed files with 56 additions and 76 deletions.
94 changes: 33 additions & 61 deletions src/app/clusters/network-commissioning/network-commissioning.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -336,6 +336,10 @@ CHIP_ERROR ThreadScanResponseToTLV::EncodeTo(TLV::TLVWriter & writer, TLV::Tag t

} // namespace

#if CHIP_DEVICE_CONFIG_SUPPORTS_CONCURRENT_CONNECTION
IntrusiveList<InstanceListNode> Instance::sInstances;
#endif

Instance::Instance(EndpointId aEndpointId, WiFiDriver * apDelegate) :
CommandHandlerInterface(Optional<EndpointId>(aEndpointId), Id), AttributeAccessInterface(Optional<EndpointId>(aEndpointId), Id),
mEndpointId(aEndpointId), mFeatureFlags(WiFiFeatures(apDelegate)), mpWirelessDriver(apDelegate), mpBaseDriver(apDelegate)
Expand Down Expand Up @@ -365,29 +369,18 @@ CHIP_ERROR Instance::Init()
mLastNetworkingStatusValue.SetNull();
mLastConnectErrorValue.SetNull();
mLastNetworkIDLen = 0;
RecordNetwork(this);
#if CHIP_DEVICE_CONFIG_SUPPORTS_CONCURRENT_CONNECTION
sInstances.PushBack(this);
#endif
return CHIP_NO_ERROR;
}

void Instance::Shutdown()
{
#if CHIP_DEVICE_CONFIG_SUPPORTS_CONCURRENT_CONNECTION
sInstances.Remove(this);
#endif
mpBaseDriver->Shutdown();

Instance * prev = nullptr;
Instance * curr = sInstanceRecord;
while (curr && curr != this)
{
prev = curr;
curr = curr->mNext;
}
if (!prev)
{
sInstanceRecord = curr->mNext;
}
else
{
prev->mNext = curr->mNext;
}
}

#if !CHIP_DEVICE_CONFIG_SUPPORTS_CONCURRENT_CONNECTION
Expand Down Expand Up @@ -1048,7 +1041,16 @@ void Instance::HandleConnectNetwork(HandlerContext & ctx, const Commands::Connec
mCurrentOperationBreadcrumb = req.breadcrumb;

#if CHIP_DEVICE_CONFIG_SUPPORTS_CONCURRENT_CONNECTION
DisconnectOtherNetworks(this);
// Per spec, lingering connections on any other interfaces need to be disconnected at this point.
for (auto & node : sInstances)
{
Instance * instance = static_cast<Instance *>(&node);
if (instance != this)
{
instance->DisconnectLingeringConnection();
}
}

mpWirelessDriver->ConnectNetwork(req.networkID, this);
#else
// In Non-concurrent mode postpone the final execution of ConnectNetwork until the operational
Expand Down Expand Up @@ -1168,54 +1170,24 @@ void Instance::HandleQueryIdentity(HandlerContext & ctx, const Commands::QueryId
}
#endif // CHIP_DEVICE_CONFIG_ENABLE_WIFI_PDC

Instance * Instance::sInstanceRecord = nullptr;

void Instance::RecordNetwork(Instance * currentNetwork)
void Instance::DisconnectLingeringConnection()
{
Instance * curr = sInstanceRecord;
while (curr)
{
if (curr == currentNetwork)
bool haveConnectedNetwork = false;
EnumerateAndRelease(mpBaseDriver->GetNetworks(), [&](const Network & network) {
if (network.connected)
{
return;
haveConnectedNetwork = true;
return Loop::Break;
}
curr = curr->mNext;
}
return Loop::Continue;
});

currentNetwork->mNext = sInstanceRecord;
sInstanceRecord = currentNetwork;
}

void Instance::DisconnectOtherNetworks(Instance * currentNetwork)
{
Instance * curr = sInstanceRecord;
while (curr)
// If none of the configured networks is `connected`, we may have a
// lingering connection to a different network that we need to disconnect.
// Note: The driver may or may not be actually connected
if (!haveConnectedNetwork)
{
if (curr != currentNetwork && curr->mpWirelessDriver)
{
bool haveConnectedNetwork = false;
EnumerateAndRelease(curr->mpBaseDriver->GetNetworks(), [&](const Network & network) {
if (network.connected)
{
haveConnectedNetwork = true;
return Loop::Break;
}
return Loop::Continue;
});

// If none of the configured networks is `connected`, we may have a
// lingering connection to a different network that we need to disconnect.
if (!haveConnectedNetwork)
{
/* It's difficult to check the media driver connection state here.
* Check the `Connected` flag in `NetworkInfoStruct` in stead, if not set,
* exec `DisconnectFromNetwork` for driver. However, the physical media connection
* may not necessarily exist in this case, such as after initialization.
*/
ReturnOnFailure(curr->mpWirelessDriver->DisconnectFromNetwork());
}
}
curr = curr->mNext;
LogErrorOnFailure(mpWirelessDriver->DisconnectFromNetwork());
}
}

Expand Down
24 changes: 16 additions & 8 deletions src/app/clusters/network-commissioning/network-commissioning.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
#include <app/AttributeAccessInterface.h>
#include <app/CommandHandlerInterface.h>
#include <app/data-model/Nullable.h>
#include <lib/support/IntrusiveList.h>
#include <lib/support/ThreadOperationalDataset.h>
#include <lib/support/Variant.h>
#include <platform/NetworkCommissioning.h>
Expand All @@ -33,9 +34,17 @@ namespace app {
namespace Clusters {
namespace NetworkCommissioning {

// Instance inherits privately from this class to participate in Instance::sInstances
class InstanceListNode : public IntrusiveListNodeBase<>
{
};

// TODO: Use macro to disable some wifi or thread
class Instance : public CommandHandlerInterface,
public AttributeAccessInterface,
#if CHIP_DEVICE_CONFIG_SUPPORTS_CONCURRENT_CONNECTION
private InstanceListNode,
#endif
public DeviceLayer::NetworkCommissioning::Internal::BaseDriver::NetworkStatusChangeCallback,
public DeviceLayer::NetworkCommissioning::Internal::WirelessDriver::ConnectCallback,
public DeviceLayer::NetworkCommissioning::WiFiDriver::ScanCallback,
Expand Down Expand Up @@ -81,14 +90,10 @@ class Instance : public CommandHandlerInterface,
void SendNonConcurrentConnectNetworkResponse();
#endif

// NetworkCommissioningInstance records.
static Instance * sInstanceRecord;
// Record current network.
static void RecordNetwork(Instance * currentNetwork);
// Disconnect networks (if connected) in the array other than the current network.
static void DisconnectOtherNetworks(Instance * currentNetwork);

Instance * mNext;
// TODO: This could be guarded by a separate multi-interface condition instead
#if CHIP_DEVICE_CONFIG_SUPPORTS_CONCURRENT_CONNECTION
static IntrusiveList<InstanceListNode> sInstances;
#endif

EndpointId mEndpointId = kInvalidEndpointId;
const BitFlags<Feature> mFeatureFlags;
Expand Down Expand Up @@ -120,6 +125,9 @@ class Instance : public CommandHandlerInterface,
void SetLastNetworkId(ByteSpan lastNetworkId);
void ReportNetworksListChanged() const;

// Disconnect if the current connection is not in the Networks list
void DisconnectLingeringConnection();

// Commits the breadcrumb value saved in mCurrentOperationBreadcrumb to the breadcrumb attribute in GeneralCommissioning
// cluster. Will set mCurrentOperationBreadcrumb to NullOptional.
void CommitSavedBreadcrumb();
Expand Down
14 changes: 7 additions & 7 deletions src/include/platform/NetworkCommissioning.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -218,13 +218,6 @@ class WirelessDriver : public Internal::BaseDriver
*/
virtual CHIP_ERROR RevertConfiguration() = 0;

#if CHIP_DEVICE_CONFIG_SUPPORTS_CONCURRENT_CONNECTION
/**
* @brief Disconnect from network, but maybe it is not connected on media driver.
*/
virtual CHIP_ERROR DisconnectFromNetwork() { return CHIP_ERROR_NOT_IMPLEMENTED; }
#endif

virtual uint8_t GetScanNetworkTimeoutSeconds() = 0;
virtual uint8_t GetConnectNetworkTimeoutSeconds() = 0;

Expand Down Expand Up @@ -254,6 +247,13 @@ class WirelessDriver : public Internal::BaseDriver
* called inside ConnectNetwork.
*/
virtual void ConnectNetwork(ByteSpan networkId, ConnectCallback * callback) = 0;

#if CHIP_DEVICE_CONFIG_SUPPORTS_CONCURRENT_CONNECTION
/**
* @brief Disconnect from network, if currently connected.
*/
virtual CHIP_ERROR DisconnectFromNetwork() { return CHIP_ERROR_NOT_IMPLEMENTED; }
#endif
};
} // namespace Internal

Expand Down

0 comments on commit f4a81a6

Please sign in to comment.