From f3f78010286f0dcdf990cede116360efae3f1bec Mon Sep 17 00:00:00 2001 From: Sharad Binjola Date: Mon, 24 Jul 2023 12:33:57 -0700 Subject: [PATCH 1/7] Adding OnCommissioningSessionEstablishmentError(err) to AppDelegate --- src/app/server/AppDelegate.h | 1 + src/app/server/CommissioningWindowManager.cpp | 8 ++++++++ 2 files changed, 9 insertions(+) diff --git a/src/app/server/AppDelegate.h b/src/app/server/AppDelegate.h index 24dcf2e3a92ec5..761de2d387c67b 100644 --- a/src/app/server/AppDelegate.h +++ b/src/app/server/AppDelegate.h @@ -33,6 +33,7 @@ class AppDelegate */ virtual void OnCommissioningSessionEstablishmentStarted() {} virtual void OnCommissioningSessionStarted() {} + virtual void OnCommissioningSessionEstablishmentError(CHIP_ERROR err) {} virtual void OnCommissioningSessionStopped(CHIP_ERROR err) {} /* diff --git a/src/app/server/CommissioningWindowManager.cpp b/src/app/server/CommissioningWindowManager.cpp index b8fb6a54e6e629..ee0ed354d24eb4 100644 --- a/src/app/server/CommissioningWindowManager.cpp +++ b/src/app/server/CommissioningWindowManager.cpp @@ -150,6 +150,8 @@ void CommissioningWindowManager::HandleFailedAttempt(CHIP_ERROR err) #if CONFIG_NETWORK_LAYER_BLE mServer->GetBleLayerObject()->CloseAllBleConnections(); #endif + + CHIP_ERROR prevErr = err; if (mFailedCommissioningAttempts < kMaxFailedCommissioningAttempts) { // If the number of commissioning attempts has not exceeded maximum @@ -157,6 +159,12 @@ void CommissioningWindowManager::HandleFailedAttempt(CHIP_ERROR err) err = AdvertiseAndListenForPASE(); } + if (mAppDelegate != nullptr) + { + ChipLogProgress(AppServer, "Calling mAppDelegate->OnCommissioningSessionEstablishmentError"); + mAppDelegate->OnCommissioningSessionEstablishmentError(prevErr); + } + if (err != CHIP_NO_ERROR) { // The commissioning attempts limit was exceeded, or listening for From 42f6eae4ff9a4de65c0fae2c47f9c4e8723b4c2d Mon Sep 17 00:00:00 2001 From: Sharad Binjola Date: Mon, 24 Jul 2023 14:41:05 -0700 Subject: [PATCH 2/7] Removed error param from OnCommissioningSessionStopped and added docs for all of AppDelegate --- examples/all-clusters-app/esp32/main/main.cpp | 2 +- .../esp32/main/main.cpp | 2 +- .../telink/common/src/AppTaskCommon.cpp | 2 +- src/app/server/AppDelegate.h | 22 +++++++++++++++++-- src/app/server/CommissioningWindowManager.cpp | 3 +-- 5 files changed, 24 insertions(+), 7 deletions(-) diff --git a/examples/all-clusters-app/esp32/main/main.cpp b/examples/all-clusters-app/esp32/main/main.cpp index 83d6abc102701f..ecca799c81d07b 100644 --- a/examples/all-clusters-app/esp32/main/main.cpp +++ b/examples/all-clusters-app/esp32/main/main.cpp @@ -87,7 +87,7 @@ class AppCallbacks : public AppDelegate public: void OnCommissioningSessionEstablishmentStarted() {} void OnCommissioningSessionStarted() override { bluetoothLED.Set(true); } - void OnCommissioningSessionStopped(CHIP_ERROR err) override + void OnCommissioningSessionStopped() override { bluetoothLED.Set(false); pairingWindowLED.Set(false); diff --git a/examples/all-clusters-minimal-app/esp32/main/main.cpp b/examples/all-clusters-minimal-app/esp32/main/main.cpp index 43c6faa51a2972..a673a706291a58 100644 --- a/examples/all-clusters-minimal-app/esp32/main/main.cpp +++ b/examples/all-clusters-minimal-app/esp32/main/main.cpp @@ -86,7 +86,7 @@ class AppCallbacks : public AppDelegate public: void OnCommissioningSessionEstablishmentStarted() {} void OnCommissioningSessionStarted() override { bluetoothLED.Set(true); } - void OnCommissioningSessionStopped(CHIP_ERROR err) override + void OnCommissioningSessionStopped() override { bluetoothLED.Set(false); pairingWindowLED.Set(false); diff --git a/examples/platform/telink/common/src/AppTaskCommon.cpp b/examples/platform/telink/common/src/AppTaskCommon.cpp index cd9fe12a60a0bc..a13e84ca527cad 100644 --- a/examples/platform/telink/common/src/AppTaskCommon.cpp +++ b/examples/platform/telink/common/src/AppTaskCommon.cpp @@ -140,7 +140,7 @@ class AppCallbacks : public AppDelegate public: void OnCommissioningSessionEstablishmentStarted() {} void OnCommissioningSessionStarted() override { isComissioningStarted = true; } - void OnCommissioningSessionStopped(CHIP_ERROR err) override { isComissioningStarted = false; } + void OnCommissioningSessionStopped() override { isComissioningStarted = false; } void OnCommissioningWindowClosed() override { if (!isComissioningStarted) diff --git a/src/app/server/AppDelegate.h b/src/app/server/AppDelegate.h index 761de2d387c67b..56c4587f66139e 100644 --- a/src/app/server/AppDelegate.h +++ b/src/app/server/AppDelegate.h @@ -29,12 +29,26 @@ class AppDelegate public: virtual ~AppDelegate() {} /** - * This is called on start of session establishment process + * This is called when the PBKDFParamRequest is received and indicates the start of the session establishment process */ virtual void OnCommissioningSessionEstablishmentStarted() {} + + /** + * This is called when the commissioning session has been established + */ virtual void OnCommissioningSessionStarted() {} + + /** + * This is called when there is an error in establishing a commissioning session (such as, when an invalid passcode is provided) + * + * @param err CHIP_ERROR indicating the error that occurred during session establishment + */ virtual void OnCommissioningSessionEstablishmentError(CHIP_ERROR err) {} - virtual void OnCommissioningSessionStopped(CHIP_ERROR err) {} + + /** + * This is called when the commissioning session establishment stops + */ + virtual void OnCommissioningSessionStopped() {} /* * This is called anytime a basic or enhanced commissioning window is opened. @@ -45,5 +59,9 @@ class AppDelegate * fact open. */ virtual void OnCommissioningWindowOpened() {} + + /* + * This is called anytime a basic or enhanced commissioning window is closed. + */ virtual void OnCommissioningWindowClosed() {} }; diff --git a/src/app/server/CommissioningWindowManager.cpp b/src/app/server/CommissioningWindowManager.cpp index ee0ed354d24eb4..da428bbacf67f2 100644 --- a/src/app/server/CommissioningWindowManager.cpp +++ b/src/app/server/CommissioningWindowManager.cpp @@ -161,7 +161,6 @@ void CommissioningWindowManager::HandleFailedAttempt(CHIP_ERROR err) if (mAppDelegate != nullptr) { - ChipLogProgress(AppServer, "Calling mAppDelegate->OnCommissioningSessionEstablishmentError"); mAppDelegate->OnCommissioningSessionEstablishmentError(prevErr); } @@ -173,7 +172,7 @@ void CommissioningWindowManager::HandleFailedAttempt(CHIP_ERROR err) if (mAppDelegate != nullptr) { - mAppDelegate->OnCommissioningSessionStopped(err); + mAppDelegate->OnCommissioningSessionStopped(); } } } From 494bbc9161c00681effa8c19f02c4ea5154c3cab Mon Sep 17 00:00:00 2001 From: Sharad Binjola Date: Tue, 25 Jul 2023 10:49:37 -0700 Subject: [PATCH 3/7] Addressing feedback from tcarmelveilleux@ --- src/app/server/AppDelegate.h | 9 +++++++-- src/app/server/CommissioningWindowManager.cpp | 14 +++++++------- 2 files changed, 14 insertions(+), 9 deletions(-) diff --git a/src/app/server/AppDelegate.h b/src/app/server/AppDelegate.h index 56c4587f66139e..091a4149fcef7d 100644 --- a/src/app/server/AppDelegate.h +++ b/src/app/server/AppDelegate.h @@ -42,11 +42,16 @@ class AppDelegate * This is called when there is an error in establishing a commissioning session (such as, when an invalid passcode is provided) * * @param err CHIP_ERROR indicating the error that occurred during session establishment + * + * @return true if the commissioning window should be closed, false otherwise */ - virtual void OnCommissioningSessionEstablishmentError(CHIP_ERROR err) {} + virtual bool OnCommissioningSessionEstablishmentError(CHIP_ERROR err) { return false; } /** - * This is called when the commissioning session establishment stops + * This is called in addition to OnCommissioningSessionEstablishmentError i.e. when there is an error in establishing a + * commissioning session AND the commissioning window is closed. The window may be closed because + * OnCommissioningSessionEstablishmentError returned 'true', the commissioning attempts limit was reached or listening for PASE + * failed. */ virtual void OnCommissioningSessionStopped() {} diff --git a/src/app/server/CommissioningWindowManager.cpp b/src/app/server/CommissioningWindowManager.cpp index da428bbacf67f2..799b6214c04496 100644 --- a/src/app/server/CommissioningWindowManager.cpp +++ b/src/app/server/CommissioningWindowManager.cpp @@ -151,17 +151,17 @@ void CommissioningWindowManager::HandleFailedAttempt(CHIP_ERROR err) mServer->GetBleLayerObject()->CloseAllBleConnections(); #endif - CHIP_ERROR prevErr = err; - if (mFailedCommissioningAttempts < kMaxFailedCommissioningAttempts) + bool closeWindow = false; + if (mAppDelegate != nullptr) { - // If the number of commissioning attempts has not exceeded maximum - // retries, let's start listening for commissioning connections again. - err = AdvertiseAndListenForPASE(); + closeWindow = mAppDelegate->OnCommissioningSessionEstablishmentError(err); } - if (mAppDelegate != nullptr) + if (!closeWindow && mFailedCommissioningAttempts < kMaxFailedCommissioningAttempts) { - mAppDelegate->OnCommissioningSessionEstablishmentError(prevErr); + // If the number of commissioning attempts has not exceeded maximum + // retries, let's start listening for commissioning connections again. + err = AdvertiseAndListenForPASE(); } if (err != CHIP_NO_ERROR) From 06fdf86dc4a40f97f33057afb7df85c83b30f6d5 Mon Sep 17 00:00:00 2001 From: Sharad Binjola Date: Wed, 26 Jul 2023 13:06:21 -0700 Subject: [PATCH 4/7] Revert "Addressing feedback from tcarmelveilleux@" This reverts commit 494bbc9161c00681effa8c19f02c4ea5154c3cab. --- src/app/server/AppDelegate.h | 9 ++------- src/app/server/CommissioningWindowManager.cpp | 14 +++++++------- 2 files changed, 9 insertions(+), 14 deletions(-) diff --git a/src/app/server/AppDelegate.h b/src/app/server/AppDelegate.h index 091a4149fcef7d..56c4587f66139e 100644 --- a/src/app/server/AppDelegate.h +++ b/src/app/server/AppDelegate.h @@ -42,16 +42,11 @@ class AppDelegate * This is called when there is an error in establishing a commissioning session (such as, when an invalid passcode is provided) * * @param err CHIP_ERROR indicating the error that occurred during session establishment - * - * @return true if the commissioning window should be closed, false otherwise */ - virtual bool OnCommissioningSessionEstablishmentError(CHIP_ERROR err) { return false; } + virtual void OnCommissioningSessionEstablishmentError(CHIP_ERROR err) {} /** - * This is called in addition to OnCommissioningSessionEstablishmentError i.e. when there is an error in establishing a - * commissioning session AND the commissioning window is closed. The window may be closed because - * OnCommissioningSessionEstablishmentError returned 'true', the commissioning attempts limit was reached or listening for PASE - * failed. + * This is called when the commissioning session establishment stops */ virtual void OnCommissioningSessionStopped() {} diff --git a/src/app/server/CommissioningWindowManager.cpp b/src/app/server/CommissioningWindowManager.cpp index 799b6214c04496..da428bbacf67f2 100644 --- a/src/app/server/CommissioningWindowManager.cpp +++ b/src/app/server/CommissioningWindowManager.cpp @@ -151,19 +151,19 @@ void CommissioningWindowManager::HandleFailedAttempt(CHIP_ERROR err) mServer->GetBleLayerObject()->CloseAllBleConnections(); #endif - bool closeWindow = false; - if (mAppDelegate != nullptr) - { - closeWindow = mAppDelegate->OnCommissioningSessionEstablishmentError(err); - } - - if (!closeWindow && mFailedCommissioningAttempts < kMaxFailedCommissioningAttempts) + CHIP_ERROR prevErr = err; + if (mFailedCommissioningAttempts < kMaxFailedCommissioningAttempts) { // If the number of commissioning attempts has not exceeded maximum // retries, let's start listening for commissioning connections again. err = AdvertiseAndListenForPASE(); } + if (mAppDelegate != nullptr) + { + mAppDelegate->OnCommissioningSessionEstablishmentError(prevErr); + } + if (err != CHIP_NO_ERROR) { // The commissioning attempts limit was exceeded, or listening for From dc25949d61d608e5de3aa3080c273b18b3f84e02 Mon Sep 17 00:00:00 2001 From: Sharad Binjola Date: Wed, 26 Jul 2023 13:21:53 -0700 Subject: [PATCH 5/7] Addressing feedback from bzbarsky-apple@ --- src/app/server/AppDelegate.h | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/app/server/AppDelegate.h b/src/app/server/AppDelegate.h index 56c4587f66139e..3c00ba059c626d 100644 --- a/src/app/server/AppDelegate.h +++ b/src/app/server/AppDelegate.h @@ -39,19 +39,22 @@ class AppDelegate virtual void OnCommissioningSessionStarted() {} /** - * This is called when there is an error in establishing a commissioning session (such as, when an invalid passcode is provided) + * This is called when the PASE establishment failed (such as, when an invalid passcode is provided) or PASE was established + * fine but then the fail-safe expired (including being expired by the commissioner) * * @param err CHIP_ERROR indicating the error that occurred during session establishment */ virtual void OnCommissioningSessionEstablishmentError(CHIP_ERROR err) {} /** - * This is called when the commissioning session establishment stops + * This is called when the PASE establishment failed or PASE was established fine but then the fail-safe expired (including + * being expired by the commissioner) AND the commissioning window is closed. The window may be closed because the commissioning + * attempts limit was reached or advertising/listening for PASE failed. */ virtual void OnCommissioningSessionStopped() {} /* - * This is called anytime a basic or enhanced commissioning window is opened. + * This is called any time a basic or enhanced commissioning window is opened. * * The type of the window can be retrieved by calling * CommissioningWindowManager::CommissioningWindowStatusForCluster(), but @@ -61,7 +64,7 @@ class AppDelegate virtual void OnCommissioningWindowOpened() {} /* - * This is called anytime a basic or enhanced commissioning window is closed. + * This is called any time a basic or enhanced commissioning window is closed. */ virtual void OnCommissioningWindowClosed() {} }; From 577998b841d3704d91dc2d6cd933ef75eb664a6c Mon Sep 17 00:00:00 2001 From: Sharad Binjola <31142146+sharadb-amazon@users.noreply.github.com> Date: Wed, 26 Jul 2023 14:07:53 -0700 Subject: [PATCH 6/7] Update src/app/server/AppDelegate.h Co-authored-by: Boris Zbarsky --- src/app/server/AppDelegate.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/app/server/AppDelegate.h b/src/app/server/AppDelegate.h index 3c00ba059c626d..244412bc67d06d 100644 --- a/src/app/server/AppDelegate.h +++ b/src/app/server/AppDelegate.h @@ -42,7 +42,7 @@ class AppDelegate * This is called when the PASE establishment failed (such as, when an invalid passcode is provided) or PASE was established * fine but then the fail-safe expired (including being expired by the commissioner) * - * @param err CHIP_ERROR indicating the error that occurred during session establishment + * @param err CHIP_ERROR indicating the error that occurred during session establishment or the error accompanying the fail-safe timeout. */ virtual void OnCommissioningSessionEstablishmentError(CHIP_ERROR err) {} From 5229056ed5642d8694c24adaffd8e86483d17e12 Mon Sep 17 00:00:00 2001 From: Sharad Binjola Date: Wed, 26 Jul 2023 14:09:47 -0700 Subject: [PATCH 7/7] Restyled --- src/app/server/AppDelegate.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/app/server/AppDelegate.h b/src/app/server/AppDelegate.h index 244412bc67d06d..78aafd578ecb75 100644 --- a/src/app/server/AppDelegate.h +++ b/src/app/server/AppDelegate.h @@ -42,7 +42,8 @@ class AppDelegate * This is called when the PASE establishment failed (such as, when an invalid passcode is provided) or PASE was established * fine but then the fail-safe expired (including being expired by the commissioner) * - * @param err CHIP_ERROR indicating the error that occurred during session establishment or the error accompanying the fail-safe timeout. + * @param err CHIP_ERROR indicating the error that occurred during session establishment or the error accompanying the fail-safe + * timeout. */ virtual void OnCommissioningSessionEstablishmentError(CHIP_ERROR err) {}