Stop using weak functions for Aliro command callbacks. (#33931)

We have a delegate; we should just make use of the delegate.  This is not a
breaking change because nothing implements Aliro bits yet.

Also:

* Fixes some incorrect early returns that did not respond to commands.
* Fixes some validity checks that were not quite correct per spec.
* Fixes some logging levels for errors.
* Adds missing "mark these attributes dirty" bits.
* Marks various functions that don't touch our members as static.

Author: bzbarsky-apple

src/app/clusters/door-lock-server/door-lock-delegate.h

@@ -125,6 +125,24 @@ class Delegate
      * @return The max number of Aliro endpoint keys supported.
      */
     virtual uint16_t GetNumberOfAliroEndpointKeysSupported() = 0;
+
+    /**
+     * Set the Aliro reader configuration for the lock.  The various arguments
+     * have already been checked for constraints and consistency with the
+     * FeatureMap.
+     *
+     * @param[in] signingKey Signing key component of the Reader's key pair.
+     * @param[in] verificationKey Verification key component of the Reader's key pair.
+     * @param[in] groupIdentifier Reader group identifier for the lock.
+     * @param[in] groupResolvingKey Group resolving key for the lock if Aliro BLE UWB feature is supported
+     */
+    virtual CHIP_ERROR SetAliroReaderConfig(const ByteSpan & signingKey, const ByteSpan & verificationKey,
+                                            const ByteSpan & groupIdentifier, const Optional<ByteSpan> & groupResolvingKey) = 0;
+
+    /**
+     * Clear the Aliro reader configuration for the lock.
+     */
+    virtual CHIP_ERROR ClearAliroReaderConfig() = 0;
 };
 
 } // namespace DoorLock

src/app/clusters/door-lock-server/door-lock-server-callback.cpp

@@ -242,15 +242,3 @@ emberAfPluginDoorLockGetFaceCredentialLengthConstraints(chip::EndpointId endpoin
 {
     return false;
 }
-
-bool __attribute__((weak))
-emberAfPluginDoorLockSetAliroReaderConfig(EndpointId endpointId, const ByteSpan & signingKey, const ByteSpan & verificationKey,
-                                          const ByteSpan & groupIdentifier, const Optional<ByteSpan> groupResolvingKey)
-{
-    return false;
-}
-
-bool __attribute__((weak)) emberAfPluginDoorLockClearAliroReaderConfig(chip::EndpointId endpointId)
-{
-    return false;
-}

src/app/clusters/door-lock-server/door-lock-server.cpp

@@ -90,7 +90,7 @@ class DoorLockClusterFabricDelegate : public chip::FabricTable::Delegate
 {
     void OnFabricRemoved(const FabricTable & fabricTable, FabricIndex fabricIndex) override
     {
-        for (auto endpointId : EnabledEndpointsWithServerCluster(chip::app::Clusters::DoorLock::Id))
+        for (auto endpointId : EnabledEndpointsWithServerCluster(Clusters::DoorLock::Id))
         {
             if (!DoorLockServer::Instance().OnFabricRemoved(endpointId, fabricIndex))
             {
@@ -3641,7 +3641,7 @@ void DoorLockServer::SendEvent(chip::EndpointId endpointId, T & event)
 
 template <typename T>
 bool DoorLockServer::GetAttribute(chip::EndpointId endpointId, chip::AttributeId attributeId,
-                                  Status (*getFn)(chip::EndpointId endpointId, T * value), T & value) const
+                                  Status (*getFn)(chip::EndpointId endpointId, T * value), T & value)
 {
     Status status = getFn(endpointId, &value);
     bool success  = (Status::Success == status);
@@ -3927,12 +3927,26 @@ void DoorLockServer::setAliroReaderConfigCommandHandler(CommandHandler * command
     }
 
     Delegate * delegate = GetDelegate(endpointID);
-    VerifyOrReturn(delegate != nullptr, ChipLogError(Zcl, "Delegate is null"));
+    if (!delegate)
+    {
+        ChipLogError(Zcl, "Door Lock delegate is null");
+        commandObj->AddStatus(commandPath, Status::Failure);
+        return;
+    }
 
-    // If Aliro BLE UWB feature is supported and groupResolvingKey is not provided in the command, return INVALID_COMMAND.
-    if (SupportsAliroBLEUWB(endpointID) && !groupResolvingKey.HasValue())
+    // The GroupResolvingKey must be provided if and only if the Aliro BLE UWB
+    // feature is supported.  Otherwise, return INVALID_COMMAND
+    const bool supportsAliroBLEUWB = SupportsAliroBLEUWB(endpointID);
+    if (supportsAliroBLEUWB != groupResolvingKey.HasValue())
     {
-        ChipLogProgress(Zcl, "[SetAliroReaderConfig] Aliro BLE UWB supported but Group Resolving Key is not provided");
+        if (supportsAliroBLEUWB)
+        {
+            ChipLogError(Zcl, "[SetAliroReaderConfig] Aliro BLE UWB supported but Group Resolving Key is not provided");
+        }
+        else
+        {
+            ChipLogError(Zcl, "[SetAliroReaderConfig] Aliro BLE UWB not supported but Group Resolving Key is provided");
+        }
         commandObj->AddStatus(commandPath, Status::InvalidCommand);
         return;
     }
@@ -3959,19 +3973,27 @@ void DoorLockServer::setAliroReaderConfigCommandHandler(CommandHandler * command
     // INVALID_IN_STATE.
     if (err != CHIP_NO_ERROR || !readerVerificationKey.empty())
     {
-        ChipLogProgress(
-            Zcl, "[SetAliroReaderConfig] Aliro reader verification key was not read or is not null. Return INVALID_IN_STATE");
+        ChipLogError(Zcl, "[SetAliroReaderConfig] Aliro reader verification key was not read or is not null.");
         commandObj->AddStatus(commandPath, Status::InvalidInState);
         return;
     }
 
-    Status status = Status::Success;
-    if (!emberAfPluginDoorLockSetAliroReaderConfig(endpointID, signingKey, verificationKey, groupIdentifier, groupResolvingKey))
+    err = delegate->SetAliroReaderConfig(signingKey, verificationKey, groupIdentifier, groupResolvingKey);
+    if (err != CHIP_NO_ERROR)
     {
         ChipLogProgress(Zcl, "[SetAliroReaderConfig] Unable to set aliro reader config [endpointId=%d]", endpointID);
-        status = Status::Failure;
     }
-    sendClusterResponse(commandObj, commandPath, ClusterStatusCode(status));
+    else
+    {
+        // Various attributes changed; mark them dirty.
+        MatterReportingAttributeChangeCallback(endpointID, Clusters::DoorLock::Id, AliroReaderVerificationKey::Id);
+        MatterReportingAttributeChangeCallback(endpointID, Clusters::DoorLock::Id, AliroReaderGroupIdentifier::Id);
+        if (supportsAliroBLEUWB)
+        {
+            MatterReportingAttributeChangeCallback(endpointID, Clusters::DoorLock::Id, AliroGroupResolvingKey::Id);
+        }
+    }
+    sendClusterResponse(commandObj, commandPath, ClusterStatusCode(StatusIB(err).mStatus));
 }
 
 void DoorLockServer::clearAliroReaderConfigCommandHandler(CommandHandler * commandObj, const ConcreteCommandPath & commandPath)
@@ -3987,13 +4009,41 @@ void DoorLockServer::clearAliroReaderConfigCommandHandler(CommandHandler * comma
         return;
     }
 
-    Status status = Status::Success;
-    if (!emberAfPluginDoorLockClearAliroReaderConfig(endpointID))
+    Delegate * delegate = GetDelegate(endpointID);
+    if (!delegate)
     {
-        ChipLogProgress(Zcl, "[SetAliroReaderConfig] Unable to set aliro reader config [endpointId=%d]", endpointID);
-        status = Status::Failure;
+        ChipLogError(Zcl, "Door Lock delegate is null");
+        commandObj->AddStatus(commandPath, Status::Failure);
+        return;
+    }
+
+    uint8_t buffer[kAliroReaderVerificationKeySize];
+    MutableByteSpan readerVerificationKey(buffer);
+    CHIP_ERROR err = delegate->GetAliroReaderVerificationKey(readerVerificationKey);
+    if (err != CHIP_NO_ERROR && readerVerificationKey.empty())
+    {
+        // No reader config to start with.  Just return without marking any
+        // attributes as dirty.
+        commandObj->AddStatus(commandPath, Status::Success);
+        return;
+    }
+
+    err = delegate->ClearAliroReaderConfig();
+    if (err != CHIP_NO_ERROR)
+    {
+        ChipLogError(Zcl, "[SetAliroReaderConfig] Unable to set aliro reader config [endpointId=%d]", endpointID);
+    }
+    else
+    {
+        // Various attributes changed; mark them dirty.
+        MatterReportingAttributeChangeCallback(endpointID, Clusters::DoorLock::Id, AliroReaderVerificationKey::Id);
+        MatterReportingAttributeChangeCallback(endpointID, Clusters::DoorLock::Id, AliroReaderGroupIdentifier::Id);
+        if (SupportsAliroBLEUWB(endpointID))
+        {
+            MatterReportingAttributeChangeCallback(endpointID, Clusters::DoorLock::Id, AliroGroupResolvingKey::Id);
+        }
     }
-    sendClusterResponse(commandObj, commandPath, ClusterStatusCode(status));
+    sendClusterResponse(commandObj, commandPath, ClusterStatusCode(StatusIB(err).mStatus));
 }
 
 // =============================================================================

src/app/clusters/door-lock-server/door-lock-server.h

@@ -29,6 +29,7 @@
 #include <app/AttributeAccessInterface.h>
 #include <app/CommandHandler.h>
 #include <app/ConcreteCommandPath.h>
+#include <app/reporting/reporting.h>
 #include <app/util/attribute-storage.h>
 #include <app/util/config.h>
 #include <platform/CHIPDeviceConfig.h>
@@ -174,57 +175,60 @@ class DoorLockServer : public chip::app::AttributeAccessInterface
 
     bool SendLockAlarmEvent(chip::EndpointId endpointId, AlarmCodeEnum alarmCode);
 
-    chip::BitFlags<Feature> GetFeatures(chip::EndpointId endpointId);
+    static chip::BitFlags<Feature> GetFeatures(chip::EndpointId endpointId);
 
-    inline bool SupportsPIN(chip::EndpointId endpointId) { return GetFeatures(endpointId).Has(Feature::kPinCredential); }
+    static inline bool SupportsPIN(chip::EndpointId endpointId) { return GetFeatures(endpointId).Has(Feature::kPinCredential); }
 
-    inline bool SupportsRFID(chip::EndpointId endpointId) { return GetFeatures(endpointId).Has(Feature::kRfidCredential); }
+    static inline bool SupportsRFID(chip::EndpointId endpointId) { return GetFeatures(endpointId).Has(Feature::kRfidCredential); }
 
-    inline bool SupportsFingers(chip::EndpointId endpointId) { return GetFeatures(endpointId).Has(Feature::kFingerCredentials); }
+    static inline bool SupportsFingers(chip::EndpointId endpointId)
+    {
+        return GetFeatures(endpointId).Has(Feature::kFingerCredentials);
+    }
 
-    inline bool SupportsFace(chip::EndpointId endpointId) { return GetFeatures(endpointId).Has(Feature::kFaceCredentials); }
+    static inline bool SupportsFace(chip::EndpointId endpointId) { return GetFeatures(endpointId).Has(Feature::kFaceCredentials); }
 
-    inline bool SupportsWeekDaySchedules(chip::EndpointId endpointId)
+    static inline bool SupportsWeekDaySchedules(chip::EndpointId endpointId)
     {
         return GetFeatures(endpointId).Has(Feature::kWeekDayAccessSchedules);
     }
 
-    inline bool SupportsYearDaySchedules(chip::EndpointId endpointId)
+    static inline bool SupportsYearDaySchedules(chip::EndpointId endpointId)
     {
         return GetFeatures(endpointId).Has(Feature::kYearDayAccessSchedules);
     }
 
-    inline bool SupportsHolidaySchedules(chip::EndpointId endpointId)
+    static inline bool SupportsHolidaySchedules(chip::EndpointId endpointId)
     {
         return GetFeatures(endpointId).Has(Feature::kHolidaySchedules);
     }
 
-    inline bool SupportsAnyCredential(chip::EndpointId endpointId)
+    static inline bool SupportsAnyCredential(chip::EndpointId endpointId)
     {
         return GetFeatures(endpointId)
             .HasAny(Feature::kPinCredential, Feature::kRfidCredential, Feature::kFingerCredentials, Feature::kFaceCredentials,
                     Feature::kAliroProvisioning);
     }
 
-    inline bool SupportsCredentialsOTA(chip::EndpointId endpointId)
+    static inline bool SupportsCredentialsOTA(chip::EndpointId endpointId)
     {
         return GetFeatures(endpointId).Has(Feature::kCredentialsOverTheAirAccess);
     }
 
-    inline bool SupportsUSR(chip::EndpointId endpointId)
+    static inline bool SupportsUSR(chip::EndpointId endpointId)
     {
         // appclusters, 5.2.2: USR feature has conformance [PIN | RID | FGP | FACE]
         return GetFeatures(endpointId).Has(Feature::kUser) && SupportsAnyCredential(endpointId);
     }
 
-    inline bool SupportsUnbolt(chip::EndpointId endpointId) { return GetFeatures(endpointId).Has(Feature::kUnbolt); }
+    static bool SupportsUnbolt(chip::EndpointId endpointId) { return GetFeatures(endpointId).Has(Feature::kUnbolt); }
 
     /**
      * @brief Checks if Aliro Provisioning feature is supported on the given endpoint
      *
      * @param endpointId endpointId ID of the endpoint which contains the lock.
      */
-    inline bool SupportsAliroProvisioning(chip::EndpointId endpointId)
+    static inline bool SupportsAliroProvisioning(chip::EndpointId endpointId)
     {
         return GetFeatures(endpointId).Has(Feature::kAliroProvisioning);
     }
@@ -234,7 +238,10 @@ class DoorLockServer : public chip::app::AttributeAccessInterface
      *
      * @param endpointId endpointId ID of the endpoint which contains the lock.
      */
-    inline bool SupportsAliroBLEUWB(chip::EndpointId endpointId) { return GetFeatures(endpointId).Has(Feature::kAliroBLEUWB); }
+    static inline bool SupportsAliroBLEUWB(chip::EndpointId endpointId)
+    {
+        return GetFeatures(endpointId).Has(Feature::kAliroBLEUWB);
+    }
 
     /**
      * @brief Allows the application to register a custom callback which will be called after the default DoorLock
@@ -545,8 +552,8 @@ class DoorLockServer : public chip::app::AttributeAccessInterface
      * @return false        if attribute reading failed (value is kept unchanged)
      */
     template <typename T>
-    bool GetAttribute(chip::EndpointId endpointId, chip::AttributeId attributeId,
-                      chip::Protocols::InteractionModel::Status (*getFn)(chip::EndpointId endpointId, T * value), T & value) const;
+    static bool GetAttribute(chip::EndpointId endpointId, chip::AttributeId attributeId,
+                             chip::Protocols::InteractionModel::Status (*getFn)(chip::EndpointId endpointId, T * value), T & value);
 
     /**
      * @brief Set generic attribute value
@@ -1273,33 +1280,3 @@ bool emberAfPluginDoorLockGetFingerVeinCredentialLengthConstraints(chip::Endpoin
  * @return false on failure, true on success.
  */
 bool emberAfPluginDoorLockGetFaceCredentialLengthConstraints(chip::EndpointId endpointId, uint8_t & minLen, uint8_t & maxLen);
-
-/**
- * @brief This callback is called when Door Lock cluster needs to communicate the Aliro reader configuration to the door lock.
- *
- * @note This function is used for communicating the Aliro signing key, verification key, group identifier and group resolving key
- *       to the lock.
- *
- * @param endpointId ID of the endpoint which contains the lock.
- * @param[in] signingKey Signing key component of the Reader's key pair.
- * @param[in] verificationKey Verification key component of the Reader's key pair.
- * @param[in] groupIdentifier Reader group identifier for the lock.
- * @param[in] groupResolvingKey Group resolving key for the lock if Aliro BLE UWB feature is supported
- *
- * @retval true, if the Aliro reader config was successfully communicated to the door lock.
- * @retval false, if error occurred while communicating the Aliro reader config.
- */
-bool emberAfPluginDoorLockSetAliroReaderConfig(chip::EndpointId endpointId, const chip::ByteSpan & signingKey,
-                                               const chip::ByteSpan & verificationKey, const chip::ByteSpan & groupIdentifier,
-                                               const Optional<chip::ByteSpan> & groupResolvingKey);
-
-/**
- * @brief This callback is called when Door Lock cluster needs to clear an existing Aliro reader configuration from the door lock.
- *
- *
- * @param endpointId ID of the endpoint which contains the lock.
- *
- * @retval true, if the Aliro reader config was successfully cleared from the door lock.
- * @retval false, if error occurred while clearing the Aliro reader config.
- */
-bool emberAfPluginDoorLockClearAliroReaderConfig(chip::EndpointId endpointId);