Simplify and future-proof enum range checks in door locks. (#24879)

* Simplify and future-proof enum range checks in door locks.

Instead of explicitly checking for the min/max values, check for the "unknown
value" value that decoding will decode to if the value is out of range.

Fixes #20936

* Address review commment.

* Address more review comments.

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

@@ -380,11 +380,10 @@ void DoorLockServer::setUserCommandHandler(chip::app::CommandHandler * commandOb
         return;
     }
 
-    if (!userType.IsNull() &&
-        (userType.Value() < UserTypeEnum::kUnrestrictedUser || userType.Value() > UserTypeEnum::kRemoteOnlyUser))
+    if (userType == UserTypeEnum::kUnknownEnumValue)
     {
         emberAfDoorLockClusterPrintln(
-            "[SetUser] Unable to set the user: user type is out of range [endpointId=%d,userIndex=%d,userType=%u]",
+            "[SetUser] Unable to set the user: user type is unknown [endpointId=%d,userIndex=%d,userType=%u]",
             commandPath.mEndpointId, userIndex, to_underlying(userType.Value()));
 
         sendClusterResponse(commandObj, commandPath, EMBER_ZCL_STATUS_INVALID_COMMAND);
@@ -694,10 +693,9 @@ void DoorLockServer::setCredentialCommandHandler(
         return;
     }
 
-    if (!userType.IsNull() &&
-        (userType.Value() < UserTypeEnum::kUnrestrictedUser || userType.Value() > UserTypeEnum::kRemoteOnlyUser))
+    if (userType == UserTypeEnum::kUnknownEnumValue)
     {
-        emberAfDoorLockClusterPrintln("[SetCredential] Unable to set the credential: user type is out of range "
+        emberAfDoorLockClusterPrintln("[SetCredential] Unable to set the credential: user type is unknown "
                                       "[endpointId=%d,credentialIndex=%d,userType=%u]",
                                       commandPath.mEndpointId, credentialIndex, to_underlying(userType.Value()));
         sendSetCredentialResponse(commandObj, commandPath, DlStatus::kInvalidField, 0, nextAvailableCredentialSlot);
@@ -3113,9 +3111,9 @@ void DoorLockServer::setHolidayScheduleCommandHandler(chip::app::CommandHandler
         return;
     }
 
-    if (operatingMode > OperatingModeEnum::kPassage || operatingMode < OperatingModeEnum::kNormal)
+    if (operatingMode == OperatingModeEnum::kUnknownEnumValue)
     {
-        emberAfDoorLockClusterPrintln("[SetHolidaySchedule] Unable to add schedule - operating mode is out of range"
+        emberAfDoorLockClusterPrintln("[SetHolidaySchedule] Unable to add schedule - operating mode is unknown"
                                       "[endpointId=%d,scheduleIndex=%d,localStarTime=%" PRIu32 ",localEndTime=%" PRIu32
                                       ", operatingMode=%d]",
                                       endpointId, holidayIndex, localStartTime, localEndTime, to_underlying(operatingMode));

src/app/data-model/Nullable.h

@@ -83,7 +83,9 @@ struct Nullable : protected Optional<T>
     static constexpr bool kIsFabricScoped = false;
 
     bool operator==(const Nullable & other) const { return Optional<T>::operator==(other); }
-    bool operator!=(const Nullable & other) const { return !(*this == other); }
+    bool operator!=(const Nullable & other) const { return Optional<T>::operator!=(other); }
+    bool operator==(const T & other) const { return Optional<T>::operator==(other); }
+    bool operator!=(const T & other) const { return Optional<T>::operator!=(other); }
 };
 
 template <class T>

src/lib/core/Optional.h

@@ -184,6 +184,8 @@ class Optional
         return (mHasValue == other.mHasValue) && (!other.mHasValue || (mValue.mData == other.mValue.mData));
     }
     bool operator!=(const Optional & other) const { return !(*this == other); }
+    bool operator==(const T & other) const { return HasValue() && Value() == other; }
+    bool operator!=(const T & other) const { return !(*this == other); }
 
     /** Convenience method to create an optional without a valid value. */
     static Optional<T> Missing() { return Optional<T>(); }

src/lib/core/tests/TestOptional.cpp

@@ -45,6 +45,8 @@ struct Count
     Count(Count && o) : m(o.m) { ++created; }
     Count & operator=(Count &&) = default;
 
+    bool operator==(const Count & o) const { return m == o.m; }
+
     int m;
 
     static void ResetCounter()
@@ -74,26 +76,43 @@ int Count::destroyed;
 
 static void TestBasic(nlTestSuite * inSuite, void * inContext)
 {
+    // Set up our test Count objects, which will mess with counts, before we reset the
+    // counts.
+    Count c100(100), c101(101), c102(102);
+
     Count::ResetCounter();
 
     {
         auto testOptional = Optional<Count>::Value(100);
         NL_TEST_ASSERT(inSuite, Count::created == 1 && Count::destroyed == 0);
         NL_TEST_ASSERT(inSuite, testOptional.HasValue() && testOptional.Value().m == 100);
+        NL_TEST_ASSERT(inSuite, testOptional == c100);
+        NL_TEST_ASSERT(inSuite, testOptional != c101);
+        NL_TEST_ASSERT(inSuite, testOptional != c102);
 
         testOptional.ClearValue();
         NL_TEST_ASSERT(inSuite, Count::created == 1 && Count::destroyed == 1);
         NL_TEST_ASSERT(inSuite, !testOptional.HasValue());
+        NL_TEST_ASSERT(inSuite, testOptional != c100);
+        NL_TEST_ASSERT(inSuite, testOptional != c101);
+        NL_TEST_ASSERT(inSuite, testOptional != c102);
 
         testOptional.SetValue(Count(101));
         NL_TEST_ASSERT(inSuite, Count::created == 3 && Count::destroyed == 2);
         NL_TEST_ASSERT(inSuite, testOptional.HasValue() && testOptional.Value().m == 101);
+        NL_TEST_ASSERT(inSuite, testOptional != c100);
+        NL_TEST_ASSERT(inSuite, testOptional == c101);
+        NL_TEST_ASSERT(inSuite, testOptional != c102);
 
         testOptional.Emplace(102);
         NL_TEST_ASSERT(inSuite, Count::created == 4 && Count::destroyed == 3);
         NL_TEST_ASSERT(inSuite, testOptional.HasValue() && testOptional.Value().m == 102);
+        NL_TEST_ASSERT(inSuite, testOptional != c100);
+        NL_TEST_ASSERT(inSuite, testOptional != c101);
+        NL_TEST_ASSERT(inSuite, testOptional == c102);
     }
 
+    // Our test Count objects are still in scope here.
     NL_TEST_ASSERT(inSuite, Count::created == 4 && Count::destroyed == 4);
 }