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

[nrfconnect] Added recovering CASE session for the light switch #65

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
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
1 change: 0 additions & 1 deletion examples/light-switch-app/nrfconnect/main/AppTask.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@

#include "AppTask.h"
#include "AppConfig.h"
#include "BindingHandler.h"
#include "LEDWidget.h"
#include "LightSwitch.h"
#include "ThreadUtil.h"
Expand Down
56 changes: 49 additions & 7 deletions examples/light-switch-app/nrfconnect/main/BindingHandler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -35,17 +35,54 @@ void BindingHandler::Init()
DeviceLayer::PlatformMgr().ScheduleWork(InitInternal);
}

void BindingHandler::OnInvokeCommandFailure(DeviceProxy * aDevice, const BindingData & aBindingData, CHIP_ERROR aError)
{
CHIP_ERROR error;

if (aError == CHIP_ERROR_TIMEOUT && !BindingHandler::GetInstance().mCaseSessionRecovered)
{
LOG_INF("Response timeout for invoked command, trying to recover CASE session.");
if (!aDevice)
return;

// Release current CASE session.
error = aDevice->Disconnect();

if (CHIP_NO_ERROR != error)
{
LOG_ERR("Disconnecting from CASE session failed due to: %" CHIP_ERROR_FORMAT, error.Format());
return;
}

// Set flag to not try recover session multiple times.
BindingHandler::GetInstance().mCaseSessionRecovered = true;

// Establish new CASE session and retrasmit command that was not applied.
error = BindingManager::GetInstance().NotifyBoundClusterChanged(
aBindingData.EndpointId, aBindingData.ClusterId, static_cast<void *>(&const_cast<BindingData &>(aBindingData)));
kkasperczyk-no marked this conversation as resolved.
Show resolved Hide resolved
}
else
{
LOG_ERR("Binding command was not applied! Reason: %" CHIP_ERROR_FORMAT, aError.Format());
}
}

void BindingHandler::OnOffProcessCommand(CommandId aCommandId, const EmberBindingTableEntry & aBinding, DeviceProxy * aDevice,
void * aContext)
{
CHIP_ERROR ret = CHIP_NO_ERROR;
CHIP_ERROR ret = CHIP_NO_ERROR;
BindingData * data = reinterpret_cast<BindingData *>(aContext);

auto onSuccess = [](const ConcreteCommandPath & commandPath, const StatusIB & status, const auto & dataResponse) {
LOG_DBG("Binding command applied successfully!");

// If session was recovered and communication works, reset flag to the initial state.
if (BindingHandler::GetInstance().mCaseSessionRecovered)
BindingHandler::GetInstance().mCaseSessionRecovered = false;
};

auto onFailure = [](CHIP_ERROR error) {
LOG_INF("Binding command was not applied! Reason: %" CHIP_ERROR_FORMAT, error.Format());
auto onFailure = [aDevice, dataRef = *data](CHIP_ERROR aError) {
BindingHandler::OnInvokeCommandFailure(aDevice, dataRef, aError);
};

switch (aCommandId)
Expand Down Expand Up @@ -105,12 +142,18 @@ void BindingHandler::OnOffProcessCommand(CommandId aCommandId, const EmberBindin
void BindingHandler::LevelControlProcessCommand(CommandId aCommandId, const EmberBindingTableEntry & aBinding,
DeviceProxy * aDevice, void * aContext)
{
BindingData * data = reinterpret_cast<BindingData *>(aContext);

auto onSuccess = [](const ConcreteCommandPath & commandPath, const StatusIB & status, const auto & dataResponse) {
LOG_DBG("Binding command applied successfully!");

// If session was recovered and communication works, reset flag to the initial state.
if (BindingHandler::GetInstance().mCaseSessionRecovered)
BindingHandler::GetInstance().mCaseSessionRecovered = false;
};

auto onFailure = [](CHIP_ERROR error) {
LOG_INF("Binding command was not applied! Reason: %" CHIP_ERROR_FORMAT, error.Format());
auto onFailure = [aDevice, dataRef = *data](CHIP_ERROR aError) {
BindingHandler::OnInvokeCommandFailure(aDevice, dataRef, aError);
};

CHIP_ERROR ret = CHIP_NO_ERROR;
Expand All @@ -119,7 +162,6 @@ void BindingHandler::LevelControlProcessCommand(CommandId aCommandId, const Embe
{
case Clusters::LevelControl::Commands::MoveToLevel::Id: {
Clusters::LevelControl::Commands::MoveToLevel::Type moveToLevelCommand;
BindingData * data = reinterpret_cast<BindingData *>(aContext);
moveToLevelCommand.level = data->Value;
if (aDevice)
{
Expand Down Expand Up @@ -192,7 +234,7 @@ void BindingHandler::InitInternal(intptr_t aArg)
}

BindingManager::GetInstance().RegisterBoundDeviceChangedHandler(LightSwitchChangedHandler);
PrintBindingTable();
BindingHandler::GetInstance().PrintBindingTable();
}

bool BindingHandler::IsGroupBound()
Expand Down
6 changes: 3 additions & 3 deletions examples/light-switch-app/nrfconnect/main/LightSwitch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ using namespace chip::app;

void LightSwitch::Init(chip::EndpointId aLightSwitchEndpoint)
{
BindingHandler::Init();
BindingHandler::GetInstance().Init();
mLightSwitchEndpoint = aLightSwitchEndpoint;
}

Expand All @@ -55,7 +55,7 @@ void LightSwitch::InitiateActionSwitch(Action mAction)
Platform::Delete(data);
return;
}
data->IsGroup = BindingHandler::IsGroupBound();
data->IsGroup = BindingHandler::GetInstance().IsGroupBound();
DeviceLayer::PlatformMgr().ScheduleWork(BindingHandler::SwitchWorkerHandler, reinterpret_cast<intptr_t>(data));
Platform::Delete(data);
}
Expand All @@ -77,7 +77,7 @@ void LightSwitch::DimmerChangeBrightness()
sBrightness = 0;
}
data->Value = (uint8_t) sBrightness;
data->IsGroup = BindingHandler::IsGroupBound();
data->IsGroup = BindingHandler::GetInstance().IsGroupBound();
DeviceLayer::PlatformMgr().ScheduleWork(BindingHandler::SwitchWorkerHandler, reinterpret_cast<intptr_t>(data));
Platform::Delete(data);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ static CHIP_ERROR SwitchCommandHandler(int argc, char ** argv)

static CHIP_ERROR TableCommandHelper(int argc, char ** argv)
{
BindingHandler::PrintBindingTable();
BindingHandler::GetInstance().PrintBindingTable();
return CHIP_NO_ERROR;
}

Expand Down
20 changes: 15 additions & 5 deletions examples/light-switch-app/nrfconnect/main/include/BindingHandler.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,6 @@
class BindingHandler
{
public:
static void Init();
static void SwitchWorkerHandler(intptr_t);
static void PrintBindingTable();
static bool IsGroupBound();

struct BindingData
{
chip::EndpointId EndpointId;
Expand All @@ -42,9 +37,24 @@ class BindingHandler
bool IsGroup{ false };
};

void Init();
void PrintBindingTable();
bool IsGroupBound();

static void SwitchWorkerHandler(intptr_t);
static void OnInvokeCommandFailure(chip::DeviceProxy * aDevice, const BindingData & aBindingData, CHIP_ERROR aError);

static BindingHandler & GetInstance()
{
static BindingHandler sBindingHandler;
return sBindingHandler;
}

private:
static void OnOffProcessCommand(chip::CommandId, const EmberBindingTableEntry &, chip::DeviceProxy *, void *);
static void LevelControlProcessCommand(chip::CommandId, const EmberBindingTableEntry &, chip::DeviceProxy *, void *);
static void LightSwitchChangedHandler(const EmberBindingTableEntry &, chip::DeviceProxy *, void *);
static void InitInternal(intptr_t);

bool mCaseSessionRecovered = false;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Move to BindingData? You won't need to clean this state on each transactions then.

Copy link
Owner Author

Choose a reason for hiding this comment

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

But binding data are related to the specific cluster and command, so it will not work if somebody send different commands one by one. I believe we should store this flag for a specific node ID, but for now we support only one node ID for unicast traffic, so I didn't care much about it.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ok, but now if there are two simultaneous commands sent and the CASE is lost, won't the second one be simply ignored? Anyway, probably it would require more work to make such scenarios work correctly, so nevermind.

};