Default initialize controller vendor ID (project-chip#20017)

* Default initialize controller vendor ID

Problem:
The Python REPL was failing to commission a device when built with
detail logging disabled.

Cause:
It was not correctly initializing the
Controller::SetupParams::controllerVendorId when creating a commissioner
instance. This resulted in it using garbage values that sometimes, were
correct and not zero.

Fix:
As per discussion here, this default initializes that field to
VendorId::Unspecified as well as adding an early check for a valid value
at controller creation time. This ensures that applications creating
commissioners are forced to explicitly provide a value for their vendor
ID. It also ensures we catch applications who don't do so early at
commissioner instantiation time.

Testing:
Validate that the REPL and REPL tests pass correctly.
For other platforms, will have to check the various PR tests and see
which ones fail.

* Review feedback

* Rebase and fix-up test logic that got added recently

src/controller/CHIPDeviceControllerFactory.cpp

@@ -291,6 +291,8 @@ void DeviceControllerFactory::PopulateInitParams(ControllerInitParams & controll
 CHIP_ERROR DeviceControllerFactory::SetupController(SetupParams params, DeviceController & controller)
 {
     VerifyOrReturnError(mSystemState != nullptr, CHIP_ERROR_INCORRECT_STATE);
+    VerifyOrReturnError(params.controllerVendorId != VendorId::Unspecified, CHIP_ERROR_INVALID_ARGUMENT);
+
     ReturnErrorOnFailure(InitSystemState());
 
     ControllerInitParams controllerParams;
@@ -303,9 +305,12 @@ CHIP_ERROR DeviceControllerFactory::SetupController(SetupParams params, DeviceCo
 CHIP_ERROR DeviceControllerFactory::SetupCommissioner(SetupParams params, DeviceCommissioner & commissioner)
 {
     VerifyOrReturnError(mSystemState != nullptr, CHIP_ERROR_INCORRECT_STATE);
+    VerifyOrReturnError(params.controllerVendorId != VendorId::Unspecified, CHIP_ERROR_INVALID_ARGUMENT);
+
     ReturnErrorOnFailure(InitSystemState());
 
     CommissionerInitParams commissionerParams;
+
     // PopulateInitParams works against ControllerInitParams base class of CommissionerInitParams only
     PopulateInitParams(commissionerParams, params);
 

src/controller/CHIPDeviceControllerFactory.h

@@ -60,7 +60,11 @@ struct SetupParams
     ByteSpan controllerICAC;
     ByteSpan controllerRCAC;
 
-    chip::VendorId controllerVendorId;
+    //
+    // This must be set to a valid, operational VendorId value associated with
+    // the controller/commissioner.
+    //
+    chip::VendorId controllerVendorId = VendorId::Unspecified;
 
     // The Device Pairing Delegated used to initialize a Commissioner
     DevicePairingDelegate * pairingDelegate = nullptr;

src/controller/python/OpCredsBinding.cpp

@@ -325,8 +325,8 @@ void pychip_OnCommissioningStatusUpdate(chip::PeerId peerId, chip::Controller::C
 
 ChipError::StorageType pychip_OpCreds_AllocateController(OpCredsContext * context,
                                                          chip::Controller::DeviceCommissioner ** outDevCtrl, uint8_t fabricIndex,
-                                                         FabricId fabricId, chip::NodeId nodeId, const char * paaTrustStorePath,
-                                                         bool useTestCommissioner)
+                                                         FabricId fabricId, chip::NodeId nodeId, chip::VendorId adminVendorId,
+                                                         const char * paaTrustStorePath, bool useTestCommissioner)
 {
     ChipLogDetail(Controller, "Creating New Device Controller");
 
@@ -373,6 +373,7 @@ ChipError::StorageType pychip_OpCreds_AllocateController(OpCredsContext * contex
     initParams.controllerICAC                 = icacSpan;
     initParams.controllerNOC                  = nocSpan;
     initParams.enableServerInteractions       = true;
+    initParams.controllerVendorId             = adminVendorId;
 
     if (useTestCommissioner)
     {

src/controller/python/chip/ChipDeviceCtrl.py

@@ -120,7 +120,7 @@ class DCState(enum.IntEnum):
 class ChipDeviceController():
     activeList = set()
 
-    def __init__(self, opCredsContext: ctypes.c_void_p, fabricId: int, fabricIndex: int, nodeId: int, paaTrustStorePath: str = "", useTestCommissioner: bool = False):
+    def __init__(self, opCredsContext: ctypes.c_void_p, fabricId: int, fabricIndex: int, nodeId: int, adminVendorId: int, paaTrustStorePath: str = "", useTestCommissioner: bool = False):
         self.state = DCState.NOT_INITIALIZED
         self.devCtrl = None
         self._ChipStack = builtins.chipStack
@@ -134,7 +134,7 @@ def __init__(self, opCredsContext: ctypes.c_void_p, fabricId: int, fabricIndex:
 
         res = self._ChipStack.Call(
             lambda: self._dmLib.pychip_OpCreds_AllocateController(ctypes.c_void_p(
-                opCredsContext), pointer(devCtrl), fabricIndex, fabricId, nodeId, ctypes.c_char_p(None if len(paaTrustStorePath) == 0 else str.encode(paaTrustStorePath)), useTestCommissioner)
+                opCredsContext), pointer(devCtrl), fabricIndex, fabricId, nodeId, adminVendorId, ctypes.c_char_p(None if len(paaTrustStorePath) == 0 else str.encode(paaTrustStorePath)), useTestCommissioner)
         )
 
         if res != 0:

src/controller/python/chip/ChipReplStartup.py

@@ -37,16 +37,20 @@ def LoadFabricAdmins():
     except KeyError:
         console.print(
             "\n[purple]No previous fabric admins discovered in persistent storage - creating a new one...")
-        _fabricAdmins.append(chip.FabricAdmin.FabricAdmin())
+
+        #
+        # Initialite a FabricAdmin with a VendorID of TestVendor1 (0xfff1)
+        #
+        _fabricAdmins.append(chip.FabricAdmin.FabricAdmin(0XFFF1))
         return _fabricAdmins
 
     console.print('\n')
 
     for k in adminList:
         console.print(
             f"[purple]Restoring FabricAdmin from storage to manage FabricId {adminList[k]['fabricId']}, FabricIndex {k}...")
-        _fabricAdmins.append(chip.FabricAdmin.FabricAdmin(
-            fabricId=adminList[k]['fabricId'], fabricIndex=int(k)))
+        _fabricAdmins.append(chip.FabricAdmin.FabricAdmin(vendorId=int(adminList[k]['vendorId']),
+                                                          fabricId=adminList[k]['fabricId'], fabricIndex=int(k)))
 
     console.print(
         '\n[blue]Fabric Admins have been loaded and are available at [red]fabricAdmins')

src/controller/python/chip/FabricAdmin.py

@@ -65,10 +65,11 @@ class FabricAdmin:
     '''
 
     _handle = chip.native.GetLibraryHandle()
+    _isActive = False
     activeFabricIndexList = set()
     activeFabricIdList = set()
-
     activeAdmins = set()
+    vendorId = None
 
     def AllocateNextFabricIndex(self):
         ''' Allocate the next un-used fabric index.
@@ -87,10 +88,11 @@ def AllocateNextFabricId(self):
             nextFabricId = nextFabricId + 1
         return nextFabricId
 
-    def __init__(self, rcac: bytes = None, icac: bytes = None, fabricIndex: int = None, fabricId: int = None):
+    def __init__(self, vendorId: int, rcac: bytes = None, icac: bytes = None, fabricIndex: int = None, fabricId: int = None):
         ''' Creates a valid FabricAdmin object with valid RCAC/ICAC, and registers itself as an OperationalCredentialsDelegate
             for other parts of the system (notably, DeviceController) to vend NOCs.
 
+            vendorId:       Valid operational Vendor ID associated with this fabric.
             rcac, icac:     Specify the RCAC and ICAC to be used with this fabric (not-supported). If not specified, an RCAC and ICAC will
                             be automatically generated.
 
@@ -101,6 +103,12 @@ def __init__(self, rcac: bytes = None, icac: bytes = None, fabricIndex: int = No
             raise ValueError(
                 "Providing valid rcac/icac values is not supported right now!")
 
+        if (vendorId is None or vendorId == 0):
+            raise ValueError(
+                f"Invalid VendorID ({vendorId}) provided!")
+
+        self.vendorId = vendorId
+
         if (fabricId is None):
             self._fabricId = self.AllocateNextFabricId()
         else:
@@ -124,7 +132,7 @@ def __init__(self, rcac: bytes = None, icac: bytes = None, fabricIndex: int = No
         FabricAdmin.activeFabricIndexList.add(self._fabricIndex)
 
         print(
-            f"New FabricAdmin: FabricId: {self._fabricId}({self._fabricIndex})")
+            f"New FabricAdmin: FabricId: {self._fabricId}({self._fabricIndex}), VendorId = {hex(self.vendorId)}")
         self._handle.pychip_OpCreds_InitializeDelegate.restype = c_void_p
 
         self.closure = builtins.chipStack.Call(
@@ -144,7 +152,7 @@ def __init__(self, rcac: bytes = None, icac: bytes = None, fabricIndex: int = No
             adminList = {str(self._fabricIndex): {'fabricId': self._fabricId}}
             builtins.chipStack.GetStorageManager().SetReplKey('fabricAdmins', adminList)
 
-        adminList[str(self._fabricIndex)] = {'fabricId': self._fabricId}
+        adminList[str(self._fabricIndex)] = {'fabricId': self._fabricId, 'vendorId': self.vendorId}
         builtins.chipStack.GetStorageManager().SetReplKey('fabricAdmins', adminList)
 
         self._isActive = True
@@ -166,7 +174,7 @@ def NewController(self, nodeId: int = None, paaTrustStorePath: str = "", useTest
         print(
             f"Allocating new controller with FabricId: {self._fabricId}({self._fabricIndex}), NodeId: {nodeId}")
         controller = ChipDeviceCtrl.ChipDeviceController(
-            self.closure, self._fabricId, self._fabricIndex, nodeId, paaTrustStorePath, useTestCommissioner)
+            self.closure, self._fabricId, self._fabricIndex, nodeId, self.vendorId, paaTrustStorePath, useTestCommissioner)
         return controller
 
     def ShutdownAll():

src/controller/python/test/test_scripts/base.py

@@ -172,8 +172,8 @@ def assertValueEqual(self, expected):
 class BaseTestHelper:
     def __init__(self, nodeid: int, paaTrustStorePath: str, testCommissioner: bool = False):
         self.chipStack = ChipStack('/tmp/repl_storage.json')
-        self.fabricAdmin = chip.FabricAdmin.FabricAdmin(
-            fabricId=1, fabricIndex=1)
+        self.fabricAdmin = chip.FabricAdmin.FabricAdmin(vendorId=0XFFF1,
+                                                        fabricId=1, fabricIndex=1)
         self.devCtrl = self.fabricAdmin.NewController(
             nodeid, paaTrustStorePath, testCommissioner)
         self.controllerNodeId = nodeid
@@ -369,7 +369,7 @@ async def TestAddUpdateRemoveFabric(self, nodeid: int):
         self.logger.info("Waiting for attribute read for CommissionedFabrics")
         startOfTestFabricCount = await self._GetCommissonedFabricCount(nodeid)
 
-        tempFabric = chip.FabricAdmin.FabricAdmin(fabricId=3, fabricIndex=3)
+        tempFabric = chip.FabricAdmin.FabricAdmin(vendorId=0xFFF1, fabricId=3, fabricIndex=3)
         tempDevCtrl = tempFabric.NewController(self.controllerNodeId, self.paaTrustStorePath)
 
         self.logger.info("Setting failsafe on CASE connection")
@@ -590,8 +590,8 @@ async def TestMultiFabric(self, ip: str, setuppin: int, nodeid: int):
         await self.devCtrl.SendCommand(nodeid, 0, Clusters.AdministratorCommissioning.Commands.OpenBasicCommissioningWindow(180), timedRequestTimeoutMs=10000)
 
         self.logger.info("Creating 2nd Fabric Admin")
-        self.fabricAdmin2 = chip.FabricAdmin.FabricAdmin(
-            fabricId=2, fabricIndex=2)
+        self.fabricAdmin2 = chip.FabricAdmin.FabricAdmin(vendorId=0xFFF1,
+                                                         fabricId=2, fabricIndex=2)
 
         self.logger.info("Creating Device Controller on 2nd Fabric")
         self.devCtrl2 = self.fabricAdmin2.NewController(
@@ -613,9 +613,10 @@ async def TestMultiFabric(self, ip: str, setuppin: int, nodeid: int):
 
         self.logger.info("Shutdown completed, starting new controllers...")
 
-        self.fabricAdmin = chip.FabricAdmin.FabricAdmin(
-            fabricId=1, fabricIndex=1)
-        fabricAdmin2 = chip.FabricAdmin.FabricAdmin(fabricId=2, fabricIndex=2)
+        self.fabricAdmin = chip.FabricAdmin.FabricAdmin(vendorId=0XFFF1,
+                                                        fabricId=1, fabricIndex=1)
+        fabricAdmin2 = chip.FabricAdmin.FabricAdmin(vendorId=0xFFF1,
+                                                    fabricId=2, fabricIndex=2)
 
         self.devCtrl = self.fabricAdmin.NewController(
             self.controllerNodeId, self.paaTrustStorePath)