-
Notifications
You must be signed in to change notification settings - Fork 2.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Modify the value that checks for invalid values #31596
Conversation
Signed-off-by: Hunsup Jung <[email protected]>
PR #31596: Size comparison from ed22840 to 965989b Increases (1 build for esp32)
Full report (73 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, psoc6, qpg, stm32, telink)
|
@@ -442,7 +442,7 @@ MatterThermostatClusterServerPreAttributeChangedCallback(const app::ConcreteAttr | |||
} | |||
auto RequestedSystemMode = static_cast<SystemModeEnum>(*value); | |||
if (ControlSequenceOfOperation > ControlSequenceOfOperationEnum::kCoolingAndHeatingWithReheat || | |||
RequestedSystemMode > SystemModeEnum::kFanOnly) | |||
RequestedSystemMode > SystemModeEnum::kSleep) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we use EnusureKnownEnumValue(RequestedSystemMode) != SystemModeEnum::kUnknownValue
?
We cannot rely on ordering of enums or kSleep being the largest.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your reviewing :) I'll modify it.
…Value function Signed-off-by: Hunsup Jung <[email protected]>
PR #31596: Size comparison from ed22840 to b292351 Decreases (13 builds for cc13x4_26x4, cc32xx, k32w, mbed, stm32)
Full report (24 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, k32w, mbed, stm32)
|
@@ -442,7 +442,7 @@ MatterThermostatClusterServerPreAttributeChangedCallback(const app::ConcreteAttr | |||
} | |||
auto RequestedSystemMode = static_cast<SystemModeEnum>(*value); | |||
if (ControlSequenceOfOperation > ControlSequenceOfOperationEnum::kCoolingAndHeatingWithReheat || | |||
RequestedSystemMode > SystemModeEnum::kFanOnly) | |||
EnsureKnownEnumValue(RequestedSystemMode) != SystemModeEnum::kUnknownValue) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This check seems backwards. Was this tested? Given that it fails to compile, I assume not....
Please add a test that would have caught the old (incorrect) code.
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. |
Old PR with changes requested. Closing as stale. |
#36048 created to do the right check, with test. |
No description provided.