Addressed comments by sharadb-amazon

Author: pgregorr-amazon

examples/tv-casting-app/android/App/app/src/main/jni/cpp/core/MatterCastingPlayer-JNI.cpp

@@ -53,9 +53,14 @@ JNI_METHOD(jobject, verifyOrEstablishConnection)
     CastingPlayer * castingPlayer = support::convertCastingPlayerFromJavaToCpp(thiz);
     VerifyOrReturnValue(castingPlayer != nullptr, support::convertMatterErrorFromCppToJava(CHIP_ERROR_INVALID_ARGUMENT));
 
+    matter::casting::core::IdentificationDeclarationOptions idOptions;
+
+    // TODO: In the following PRs. Replace EndpointFilter Java class with IdentificationDeclarationOptions Java class.
     matter::casting::core::EndpointFilter desiredEndpointFilter;
     if (desiredEndpointFilterJavaObject != nullptr)
     {
+        chip::Protocols::UserDirectedCommissioning::TargetAppInfo targetAppInfo;
+
         // Convert the EndpointFilter Java class to a C++ EndpointFilter
         jclass endpointFilterJavaClass = env->GetObjectClass(desiredEndpointFilterJavaObject);
         jfieldID vendorIdFieldId       = env->GetFieldID(endpointFilterJavaClass, "vendorId", "Ljava/lang/Integer;");
@@ -66,23 +71,26 @@ JNI_METHOD(jobject, verifyOrEstablishConnection)
         // "Ljava/util/List;");
 
         // Value of 0 means unspecified
-        desiredEndpointFilter.vendorId  = vendorIdIntegerObject != nullptr
+        targetAppInfo.vendorId  = vendorIdIntegerObject != nullptr
              ? static_cast<uint16_t>(env->CallIntMethod(
                   vendorIdIntegerObject, env->GetMethodID(env->GetObjectClass(vendorIdIntegerObject), "intValue", "()I")))
              : 0;
-        desiredEndpointFilter.productId = productIdIntegerObject != nullptr
+        targetAppInfo.productId = productIdIntegerObject != nullptr
             ? static_cast<uint16_t>(env->CallIntMethod(
                   productIdIntegerObject, env->GetMethodID(env->GetObjectClass(productIdIntegerObject), "intValue", "()I")))
             : 0;
-        // TODO: In following PRs. Translate the Java requiredDeviceTypes list to a C++ requiredDeviceTypes vector. For now we're
-        // passing an empty list of DeviceTypeStruct.
+
+        CHIP_ERROR result = idOptions.addTargetAppInfo(targetAppInfo);
+        if (result != CHIP_NO_ERROR)
+        {
+            ChipLogError(AppServer, "MatterCastingPlayer-JNI::verifyOrEstablishConnection() failed to add targetAppInfo: %" CHIP_ERROR_FORMAT, result.Format());
+        }
     }
 
     MatterCastingPlayerJNIMgr().mConnectionSuccessHandler.SetUp(env, jSuccessCallback);
     MatterCastingPlayerJNIMgr().mConnectionFailureHandler.SetUp(env, jFailureCallback);
 
-    // TODO: In the following PRs. Removed desiredEndpointFilter to fix Android app build issue. Replace desiredEndpointFilter with
-    // optional IdentificationDeclarationOptions. Add optional CommissionerDeclarationHandler callback parameter.
+    // TODO: In the following PRs. Add optional CommissionerDeclarationHandler callback parameter.
     castingPlayer->VerifyOrEstablishConnection(
         [](CHIP_ERROR err, CastingPlayer * playerPtr) {
             ChipLogProgress(AppServer, "MatterCastingPlayer-JNI::verifyOrEstablishConnection() ConnectCallback called");
@@ -99,7 +107,7 @@ JNI_METHOD(jobject, verifyOrEstablishConnection)
                 MatterCastingPlayerJNIMgr().mConnectionFailureHandler.Handle(err);
             }
         },
-        static_cast<unsigned long long int>(commissioningWindowTimeoutSec));
+        static_cast<unsigned long long int>(commissioningWindowTimeoutSec), idOptions);
     return support::convertMatterErrorFromCppToJava(CHIP_NO_ERROR);
 }
 

examples/tv-casting-app/darwin/MatterTvCastingBridge/MatterTvCastingBridge/MCCastingPlayer.mm

@@ -52,20 +52,30 @@ - (void)verifyOrEstablishConnectionWithCompletionBlock:(void (^_Nonnull)(NSError
 
     dispatch_queue_t workQueue = [[MCCastingApp getSharedInstance] getWorkQueue];
     dispatch_sync(workQueue, ^{
+        matter::casting::core::IdentificationDeclarationOptions idOptions;
+
+        // TODO: In the following PRs. Replace EndpointFilter objC class with IdentificationDeclarationOptions objC class.
         __block matter::casting::core::EndpointFilter cppDesiredEndpointFilter;
         if (desiredEndpointFilter != nil) {
-            cppDesiredEndpointFilter.vendorId = desiredEndpointFilter.vendorId;
-            cppDesiredEndpointFilter.productId = desiredEndpointFilter.productId;
+            chip::Protocols::UserDirectedCommissioning::TargetAppInfo targetAppInfo;
+            targetAppInfo.vendorId = desiredEndpointFilter.vendorId;
+            targetAppInfo.productId = desiredEndpointFilter.productId;
+
+            CHIP_ERROR result = idOptions.addTargetAppInfo(targetAppInfo);
+            if (result != CHIP_NO_ERROR)
+            {
+                ChipLogError(AppServer, "MCCastingPlayer.verifyOrEstablishConnectionWithCompletionBlock failed to add targetAppInfo: %" CHIP_ERROR_FORMAT, result.Format());
+            }
         }
 
-        // TODO: In the following PRs. Removed desiredEndpointFilter to fix iOS app build issue. Replace desiredEndpointFilter with optional IdentificationDeclarationOptions. Add optional CommissionerDeclarationHandler callback parameter.
+        // TODO: In the following PRs. Add optional CommissionerDeclarationHandler callback parameter.
         _cppCastingPlayer->VerifyOrEstablishConnection(
             [completion](CHIP_ERROR err, matter::casting::core::CastingPlayer * castingPlayer) {
                 dispatch_queue_t clientQueue = [[MCCastingApp getSharedInstance] getClientQueue];
                 dispatch_async(clientQueue, ^{
                     completion(err == CHIP_NO_ERROR ? nil : [MCErrorUtils NSErrorFromChipError:err]);
                 });
-            }, timeout);
+            }, timeout, idOptions);
     });
 }
 

examples/tv-casting-app/linux/simple-app-helper.cpp

@@ -49,8 +49,10 @@ void DiscoveryDelegateImpl::HandleOnAdded(matter::casting::memory::Strong<matter
     if (commissionersCount == 0)
     {
         ChipLogProgress(AppServer, "Select discovered Casting Player (start index = 0) to request commissioning");
+        ChipLogProgress(AppServer, "Include the cgp flag to attempt the Commissioner-Generated Passcode commissioning flow");
 
-        ChipLogProgress(AppServer, "Example: cast request 0");
+        ChipLogProgress(AppServer, "Example1: cast request 0");
+        ChipLogProgress(AppServer, "Example2: cast request 0 cgp");
     }
     ChipLogProgress(AppServer, "Discovered CastingPlayer #%d", commissionersCount);
     ++commissionersCount;
@@ -221,13 +223,13 @@ CHIP_ERROR CommandHandler(int argc, char ** argv)
     }
     if (strcmp(argv[0], "discover") == 0)
     {
-        ChipLogProgress(AppServer, "discover");
+        ChipLogProgress(AppServer, "CommandHandler() discover");
 
         return matter::casting::core::CastingPlayerDiscovery::GetInstance()->StartDiscovery(kTargetPlayerDeviceType);
     }
     if (strcmp(argv[0], "stop-discovery") == 0)
     {
-        ChipLogProgress(AppServer, "stop-discovery");
+        ChipLogProgress(AppServer, "CommandHandler() stop-discovery");
         return matter::casting::core::CastingPlayerDiscovery::GetInstance()->StopDiscovery();
     }
     if (strcmp(argv[0], "request") == 0)
@@ -246,12 +248,27 @@ CHIP_ERROR CommandHandler(int argc, char ** argv)
         std::shared_ptr<matter::casting::core::CastingPlayer> targetCastingPlayer = castingPlayers.at(index);
 
         matter::casting::core::IdentificationDeclarationOptions idOptions;
-        idOptions.mNumTargetAppInfos          = 1;
-        idOptions.mTargetAppInfos[0].vendorId = kDesiredEndpointVendorId;
-        // Attempt Commissioner-Generated Passcode commissioning flow only if the CastingPlayer indicates support for it.
-        if (targetCastingPlayer->GetSupportsCommissionerGeneratedPasscode())
+        if (argc == 3)
         {
-            idOptions.mCommissionerPasscode = true;
+            if (strcmp(argv[2], "cgp") == 0)
+            {
+                // Attempt Commissioner-Generated Passcode (cgp) commissioning flow only if the CastingPlayer indicates support for it.
+                if (targetCastingPlayer->GetSupportsCommissionerGeneratedPasscode())
+                {
+                    ChipLogProgress(AppServer, "CommandHandler() request %lu cgp. Attempting the Commissioner-Generated Passcode commissioning flow", index);
+                    idOptions.mCommissionerPasscode = true;
+                } else 
+                {
+                    ChipLogError(AppServer, "CommandHandler() request %lu cgp. Selected CastingPLayer does not support the Commissioner-Generated Passcode commissioning flow", index);
+                }
+            }
+        }
+        chip::Protocols::UserDirectedCommissioning::TargetAppInfo targetAppInfo;
+        targetAppInfo.vendorId = kDesiredEndpointVendorId;
+        CHIP_ERROR result = idOptions.addTargetAppInfo(targetAppInfo);
+        if (result != CHIP_NO_ERROR)
+        {
+            ChipLogError(AppServer, "CommandHandler() request, failed to add targetAppInfo: %" CHIP_ERROR_FORMAT, result.Format());
         }
 
         targetCastingPlayer->VerifyOrEstablishConnection(ConnectionHandler, matter::casting::core::kCommissioningWindowTimeoutSec,
@@ -291,6 +308,8 @@ CHIP_ERROR PrintAllCommands()
     streamer_printf(sout, "  stop-discovery       Stop Discovery of Casting Players. Usage: cast stop-discovery\r\n");
     streamer_printf(
         sout, "  request <index>      Request connecting to discovered Casting Player with [index]. Usage: cast request 0\r\n");
+    streamer_printf(
+        sout, "  request <index> cgp  Request connecting to discovered Casting Player with [index] using the Commissioner-Generated Passcode commissioning flow. Usage: cast request 0 cgp\r\n");
     streamer_printf(sout, "\r\n");
 
     return CHIP_NO_ERROR;

examples/tv-casting-app/linux/simple-app-helper.h

@@ -20,6 +20,7 @@
 
 #include "core/CastingPlayer.h"
 #include "core/CastingPlayerDiscovery.h"
+#include "core/IdentificationDeclarationOptions.h"
 #include "core/Types.h"
 #include <platform/CHIPDeviceLayer.h>
 

examples/tv-casting-app/tv-casting-common/core/CastingPlayer.cpp

@@ -66,7 +66,7 @@ void CastingPlayer::VerifyOrEstablishConnection(ConnectCallback onCompleted, uns
         if (it != cachedCastingPlayers.end())
         {
             unsigned index = (unsigned int) std::distance(cachedCastingPlayers.begin(), it);
-            if (ContainsDesiredTargetApp(&cachedCastingPlayers[index], idOptions.mTargetAppInfos, idOptions.mNumTargetAppInfos))
+            if (ContainsDesiredTargetApp(&cachedCastingPlayers[index], idOptions.getTargetAppInfoList()))
             {
                 ChipLogProgress(
                     AppServer,
@@ -166,17 +166,35 @@ void CastingPlayer::RegisterEndpoint(const memory::Strong<Endpoint> endpoint)
 #if CHIP_DEVICE_CONFIG_ENABLE_COMMISSIONER_DISCOVERY_CLIENT
 CHIP_ERROR CastingPlayer::SendUserDirectedCommissioningRequest()
 {
-    ChipLogProgress(AppServer, "CastingPlayer::SendUserDirectedCommissioningRequest() creating IdentificationDeclaration message");
+    ChipLogProgress(AppServer, "CastingPlayer::SendUserDirectedCommissioningRequest()");
     chip::Inet::IPAddress * ipAddressToUse = GetIpAddressForUDCRequest();
     VerifyOrReturnValue(ipAddressToUse != nullptr, CHIP_ERROR_INCORRECT_STATE,
                         ChipLogError(AppServer, "No IP Address found to send UDC request to"));
 
     ReturnErrorOnFailure(support::ChipDeviceEventHandler::SetUdcStatus(true));
 
+    chip::Protocols::UserDirectedCommissioning::IdentificationDeclaration id = buildIdentificationDeclarationMessage();
+
+    // TODO: In the following PRs. Implement handler for CommissionerDeclaration messages and expose messages to higher layers for
+    // Linux, Android and iOS.
+    chip::Server::GetInstance().GetUserDirectedCommissioningClient()->SetCommissionerDeclarationHandler(
+        mCommissionerDeclarationHandler);
+
+    ReturnErrorOnFailure(chip::Server::GetInstance().SendUserDirectedCommissioningRequest(
+        chip::Transport::PeerAddress::UDP(*ipAddressToUse, mAttributes.port, mAttributes.interfaceId), id));
+
+    return CHIP_NO_ERROR;
+}
+
+chip::Protocols::UserDirectedCommissioning::IdentificationDeclaration CastingPlayer::buildIdentificationDeclarationMessage()
+{
+    ChipLogProgress(AppServer, "CastingPlayer::buildIdentificationDeclarationMessage() building IdentificationDeclaration message");
     chip::Protocols::UserDirectedCommissioning::IdentificationDeclaration id;
-    for (size_t i = 0; i < mIdOptions.mNumTargetAppInfos; i++)
+
+    std::vector<chip::Protocols::UserDirectedCommissioning::TargetAppInfo> mTargetAppInfos = mIdOptions.getTargetAppInfoList();
+    for (size_t i = 0; i < mTargetAppInfos.size(); i++)
     {
-        id.AddTargetAppInfo(mIdOptions.mTargetAppInfos[i]);
+        id.AddTargetAppInfo(mTargetAppInfos[i]);
     }
     id.SetNoPasscode(mIdOptions.mNoPasscode);
     id.SetCdUponPasscodeDialog(mIdOptions.mCdUponPasscodeDialog);
@@ -194,15 +212,7 @@ CHIP_ERROR CastingPlayer::SendUserDirectedCommissioningRequest()
         }
     }
 
-    // TODO: In the following PRs. Implement handler for CommissionerDeclaration messages and expose messages to higher layers for
-    // Linux, Android and iOS.
-    chip::Server::GetInstance().GetUserDirectedCommissioningClient()->SetCommissionerDeclarationHandler(
-        mCommissionerDeclarationHandler);
-
-    ReturnErrorOnFailure(chip::Server::GetInstance().SendUserDirectedCommissioningRequest(
-        chip::Transport::PeerAddress::UDP(*ipAddressToUse, mAttributes.port, mAttributes.interfaceId), id));
-
-    return CHIP_NO_ERROR;
+    return id;
 }
 
 chip::Inet::IPAddress * CastingPlayer::GetIpAddressForUDCRequest()
@@ -244,12 +254,10 @@ void CastingPlayer::FindOrEstablishSession(void * clientContext, chip::OnDeviceC
         connectionContext->mOnConnectionFailureCallback);
 }
 
-bool CastingPlayer::ContainsDesiredTargetApp(core::CastingPlayer * cachedCastingPlayer,
-                                             chip::Protocols::UserDirectedCommissioning::TargetAppInfo * desiredTargetApps,
-                                             uint8_t numTargetApps)
+bool CastingPlayer::ContainsDesiredTargetApp(core::CastingPlayer * cachedCastingPlayer, std::vector<chip::Protocols::UserDirectedCommissioning::TargetAppInfo> desiredTargetApps)
 {
     std::vector<memory::Strong<Endpoint>> cachedEndpoints = cachedCastingPlayer->GetEndpoints();
-    for (size_t i = 0; i < numTargetApps; i++)
+    for (size_t i = 0; i < desiredTargetApps.size(); i++)
     {
         for (const auto & cachedEndpoint : cachedEndpoints)
         {

examples/tv-casting-app/tv-casting-common/core/CastingPlayer.h

@@ -18,7 +18,9 @@
 
 #pragma once
 
+#include "CommissionerDeclarationHandler.h"
 #include "Endpoint.h"
+#include "IdentificationDeclarationOptions.h"
 #include "Types.h"
 #include "support/ChipDeviceEventHandler.h"
 #include "support/EndpointListLoader.h"
@@ -69,65 +71,6 @@ class CastingPlayerAttributes
     chip::FabricIndex fabricIndex = 0;
 };
 
-/**
- * This class contains the optional parameters used in the IdentificationDeclaration Message, sent by the Commissionee to the
- * Commissioner. The options specify information relating to the requested UDC commissioning session.
- */
-class IdentificationDeclarationOptions
-{
-public:
-    constexpr static size_t kMaxTargetAppInfos = 10;
-    uint8_t mNumTargetAppInfos                 = 0; // The number of TargetAppInfos included.
-    /**
-     * Feature: Target Content Application
-     * The set of content app Vendor IDs (and optionally, Product IDs) that can be used for authentication.
-     * Also, if TargetAppInfo is passed in, VerifyOrEstablishConnection() will force User Directed Commissioning, in case the
-     * desired TargetApp is not found in the on-device CastingStore.
-     */
-    chip::Protocols::UserDirectedCommissioning::TargetAppInfo mTargetAppInfos[kMaxTargetAppInfos];
-
-    /**
-     * Feature: Target Content Application
-     * Flag to instruct the Commissioner not to display a Passcode input dialog, and instead send a CommissionerDeclaration message
-     * if a commissioning Passcode is needed.
-     */
-    bool mNoPasscode = false;
-    /**
-     * Feature: Coordinate Passcode Dialogs
-     * Flag to instruct the Commissioner to send a CommissionerDeclaration message when the Passcode input dialog on the
-     * Commissioner has been shown to the user.
-     */
-    bool mCdUponPasscodeDialog = false;
-    /**
-     * Feature: Commissioner-Generated Passcode
-     * Flag to instruct the Commissioner to use the Commissioner-generated Passcode for commissioning.
-     */
-    bool mCommissionerPasscode = false;
-    /**
-     * Feature: Commissioner-Generated Passcode
-     * Flag to indicate whether or not the Commissionee has obtained the Commissioner Passcode from the user and is therefore ready
-     * for commissioning.
-     */
-    bool mCommissionerPasscodeReady = false;
-    /**
-     * Feature: Coordinate Passcode Dialogs
-     * Flag to indicate when the Commissionee user has decided to exit the commissioning process.
-     */
-    bool mCancelPasscode = false;
-};
-
-// TODO: In the following PRs. This is work in progress. Break out to a seperate cpp file.
-class CommissionerDeclarationHandler : public chip::Protocols::UserDirectedCommissioning::CommissionerDeclarationHandler
-{
-public:
-    void OnCommissionerDeclarationMessage(const chip::Transport::PeerAddress & source,
-                                          chip::Protocols::UserDirectedCommissioning::CommissionerDeclaration cd) override
-    {
-        ChipLogProgress(AppServer, "CastingPlayer::CommissionerDeclarationHandler::OnCommissionerDeclarationMessage() called");
-        cd.DebugLog();
-    }
-};
-
 class Endpoint;
 
 /**
@@ -155,7 +98,10 @@ class CastingPlayer : public std::enable_shared_from_this<CastingPlayer>
     CastingPlayer(CastingPlayerAttributes playerAttributes)
     {
         mAttributes                     = playerAttributes;
-        mCommissionerDeclarationHandler = new CommissionerDeclarationHandler();
+    }
+    ~CastingPlayer()
+    {
+        delete mCommissionerDeclarationHandler;
     }
 
     /**
@@ -272,7 +218,7 @@ class CastingPlayer : public std::enable_shared_from_this<CastingPlayer>
      * for commissioning.
      */
     bool mUdcCommissionerPasscodeReady = false;
-    CommissionerDeclarationHandler * mCommissionerDeclarationHandler;
+    CommissionerDeclarationHandler * mCommissionerDeclarationHandler = new CommissionerDeclarationHandler();
     static CastingPlayer * mTargetCastingPlayer;
     unsigned long long int mCommissioningWindowTimeoutSec = kCommissioningWindowTimeoutSec;
     ConnectCallback mOnCompleted                          = {};
@@ -283,6 +229,11 @@ class CastingPlayer : public std::enable_shared_from_this<CastingPlayer>
      */
     CHIP_ERROR SendUserDirectedCommissioningRequest();
 
+    /**
+     * @brief Builds an IdentificationDeclaration message to be sent to this CastingPlayer
+     */
+    chip::Protocols::UserDirectedCommissioning::IdentificationDeclaration buildIdentificationDeclarationMessage();
+
     /**
      * @brief Selects an IP Address to send the UDC request to.
      * Prioritizes IPV4 addresses over IPV6.
@@ -294,13 +245,9 @@ class CastingPlayer : public std::enable_shared_from_this<CastingPlayer>
 
     /**
      * @brief Checks if the cachedCastingPlayer contains at least one Endpoint/TargetApp described in the desiredTargetApps list.
-     * @return true - cachedCastingPlayer contains at least one endpoints with matching (non-default) values as described in the
-     * desiredTargetApps list, false otherwise.
-     * @param numTargetApps Specifies the number of TargetAppInfos passed in the desiredTargetApps array.
+     * @return true - cachedCastingPlayer contains at least one endpoints with matching (non-default) values for vendorID and productID as described in the desiredTargetApps list, false otherwise.
      */
-    bool ContainsDesiredTargetApp(core::CastingPlayer * cachedCastingPlayer,
-                                  chip::Protocols::UserDirectedCommissioning::TargetAppInfo * desiredTargetApps,
-                                  uint8_t numTargetApps);
+    bool ContainsDesiredTargetApp(core::CastingPlayer * cachedCastingPlayer, std::vector<chip::Protocols::UserDirectedCommissioning::TargetAppInfo> desiredTargetApps);
 
     // ChipDeviceEventHandler handles chip::DeviceLayer::ChipDeviceEvent events and helps the CastingPlayer class commission with
     // and connect to a CastingPlayer