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

Update AppDelegate to report error param and callback for when session establishment starts #28138

Merged
merged 3 commits into from
Jul 21, 2023
Merged

Update AppDelegate to report error param and callback for when session establishment starts #28138

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9e39ba2
Adding CHIP_ERROR param and a new callback to AppDelegate
sharadb-amazon Jul 12, 2023
5fba77c
Added docs for OnCommissioningSessionEstablishmentStarted
sharadb-amazon Jul 21, 2023
dd5cfaa
Restyled by whitespace
restyled-commits Jul 21, 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
3 changes: 2 additions & 1 deletion examples/all-clusters-app/esp32/main/main.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,8 +85,9 @@ namespace {
class AppCallbacks : public AppDelegate
{
public:
void OnCommissioningSessionEstablishmentStarted() {}
void OnCommissioningSessionStarted() override { bluetoothLED.Set(true); }
void OnCommissioningSessionStopped() override
void OnCommissioningSessionStopped(CHIP_ERROR err) override
Copy link
Contributor

Choose a reason for hiding this comment

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

This changes public API. This should have been a new callback for status in addition to OnCommissioningSessionStopped.

Furthermore, usage of the err argument is not documented.

Copy link
Contributor Author

@sharadb-amazon sharadb-amazon Jul 24, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Addressing part of this in this PR:  #28240

and tracking the rest in this issue: #28136

{
bluetoothLED.Set(false);
pairingWindowLED.Set(false);
Expand Down
3 changes: 2 additions & 1 deletion examples/all-clusters-minimal-app/esp32/main/main.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,8 +84,9 @@ namespace {
class AppCallbacks : public AppDelegate
{
public:
void OnCommissioningSessionEstablishmentStarted() {}
void OnCommissioningSessionStarted() override { bluetoothLED.Set(true); }
void OnCommissioningSessionStopped() override
void OnCommissioningSessionStopped(CHIP_ERROR err) override
{
bluetoothLED.Set(false);
pairingWindowLED.Set(false);
Expand Down
3 changes: 2 additions & 1 deletion examples/platform/telink/common/src/AppTaskCommon.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,8 +138,9 @@ class AppCallbacks : public AppDelegate
bool isComissioningStarted;

public:
void OnCommissioningSessionEstablishmentStarted() {}
void OnCommissioningSessionStarted() override { isComissioningStarted = true; }
void OnCommissioningSessionStopped() override { isComissioningStarted = false; }
void OnCommissioningSessionStopped(CHIP_ERROR err) override { isComissioningStarted = false; }
void OnCommissioningWindowClosed() override
{
if (!isComissioningStarted)
Expand Down
8 changes: 7 additions & 1 deletion src/app/server/AppDelegate.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,18 @@

#pragma once

#include <lib/core/CHIPError.h>

class AppDelegate
{
public:
virtual ~AppDelegate() {}
/**
* This is called on start of session establishment process
Copy link
Contributor

Choose a reason for hiding this comment

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

This says nothing useful. What is considered the "start of the session establishment process" @sharadb-amazon? This specifically gets called when PBKDFParamRequest is received, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Going to address it in this PR: #28186

*/
virtual void OnCommissioningSessionEstablishmentStarted() {}
sharadb-amazon marked this conversation as resolved.
Show resolved Hide resolved
virtual void OnCommissioningSessionStarted() {}
virtual void OnCommissioningSessionStopped() {}
virtual void OnCommissioningSessionStopped(CHIP_ERROR err) {}

/*
* This is called anytime a basic or enhanced commissioning window is opened.
Expand Down
8 changes: 7 additions & 1 deletion src/app/server/CommissioningWindowManager.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ void CommissioningWindowManager::HandleFailedAttempt(CHIP_ERROR err)

if (mAppDelegate != nullptr)
{
mAppDelegate->OnCommissioningSessionStopped();
mAppDelegate->OnCommissioningSessionStopped(err);
}
}
}
Expand All @@ -175,6 +175,12 @@ void CommissioningWindowManager::OnSessionEstablishmentStarted()
// As per specifications, section 5.5: Commissioning Flows
constexpr System::Clock::Timeout kPASESessionEstablishmentTimeout = System::Clock::Seconds16(60);
DeviceLayer::SystemLayer().StartTimer(kPASESessionEstablishmentTimeout, HandleSessionEstablishmentTimeout, this);

ChipLogProgress(AppServer, "Commissioning session establishment step started");
if (mAppDelegate != nullptr)
{
mAppDelegate->OnCommissioningSessionEstablishmentStarted();
sharadb-amazon marked this conversation as resolved.
Show resolved Hide resolved
}
}

void CommissioningWindowManager::OnSessionEstablished(const SessionHandle & session)
Expand Down