Clean-up the command handling logic

Author: arun-silabs

examples/oven-app/oven-app-common/include/CookSurfaceEndpoint.h

@@ -38,9 +38,8 @@ class CookSurfaceEndpoint
     CHIP_ERROR Init();
 
     /**
-     * @brief Returns the cached current On/Off state without querying attributes.
+     * @brief Returns the current On/Off state.
      */
-    bool CurrentState() const { return currentOnOffState; }
 
     bool GetOnOffState();
 
@@ -50,7 +49,6 @@ class CookSurfaceEndpoint
     void SetOnOffState(bool state);
 
 private:
-    bool currentOnOffState = false;
     EndpointId mEndpointId = kInvalidEndpointId;
 };
 

examples/oven-app/oven-app-common/src/CookSurfaceEndpoint.cpp

@@ -28,7 +28,6 @@ CHIP_ERROR CookSurfaceEndpoint::Init()
 {
     bool state = false;
     OnOffServer::Instance().getOnOffValue(mEndpointId, &state);
-    currentOnOffState = state;
     return CHIP_NO_ERROR;
 }
 
@@ -43,8 +42,8 @@ void CookSurfaceEndpoint::SetOnOffState(bool state)
 {
     CommandId commandId = state ? OnOff::Commands::On::Id : OnOff::Commands::Off::Id;
     auto status = OnOffServer::Instance().setOnOffValue(mEndpointId, commandId, false);
-    if (status == chip::Protocols::InteractionModel::Status::Success)
+    if (status != Protocols::InteractionModel::Status::Success)
     {
-        currentOnOffState = state;
+        ChipLogError(AppServer, "ERR: updating on/off %x", to_underlying(status));
     }
 }

examples/oven-app/silabs/BUILD.gn

@@ -76,7 +76,6 @@ if (wifi_soc) {
       "${examples_plat_dir}",
       "${chip_root}/src/lib",
       "${examples_common_plat_dir}",
-      # Added for oven app Option A: expose shared oven-app-common headers to all upstream targets
       "${example_oven_dir}/oven-app-common/include",
     ]
 

examples/oven-app/silabs/include/AppConfig.h

@@ -26,7 +26,3 @@
 #define APP_TASK_NAME "Oven"
 
 #define BLE_DEV_NAME "Silabs-Oven"
-
-// Time it takes in ms for the simulated actuator to move from one
-// state to another.
-#define ACTUATOR_MOVEMENT_PERIOD_MS 10

examples/oven-app/silabs/include/OvenManager.h

@@ -27,7 +27,6 @@
 #pragma once
 
 #include "AppSupportedTemperatureLevelsDelegate.h"
-// Corrected relative paths (silabs/include -> go up two levels to oven-app root)
 #include "OvenEndpoint.h"
 #include "CookTopEndpoint.h"
 #include "CookSurfaceEndpoint.h"
@@ -76,8 +75,34 @@ class OvenManager
      */
     static OvenManager & GetInstance() { return sOvenMgr; }
 
+    /**
+     * @brief Handles temperature control attribute changes.
+     *
+     * @param endpointId The ID of the endpoint.
+     * @param attributeId The ID of the attribute.
+     * @param value Pointer to the new value.
+     * @param size Size of the new value.
+     */
     void TempCtrlAttributeChangeHandler(chip::EndpointId endpointId, chip::AttributeId attributeId, uint8_t * value, uint16_t size);
+    
+    /**
+     * @brief Handles on/off attribute changes.
+     *
+     * @param endpointId The ID of the endpoint.
+     * @param attributeId The ID of the attribute.
+     * @param value Pointer to the new value.
+     * @param size Size of the new value.
+     */
     void OnOffAttributeChangeHandler(chip::EndpointId endpointId, chip::AttributeId attributeId, uint8_t * value, uint16_t size);
+
+    /**
+     * @brief Handles oven mode attribute changes.
+     *
+     * @param endpointId The ID of the endpoint.
+     * @param attributeId The ID of the attribute.
+     * @param value Pointer to the new value.
+     * @param size Size of the new value.
+     */
     void OvenModeAttributeChangeHandler(chip::EndpointId endpointId, chip::AttributeId attributeId, uint8_t * value, uint16_t size);
 
     /**

examples/oven-app/silabs/src/AppTask.cpp

@@ -132,11 +132,10 @@ void AppTask::ButtonEventHandler(uint8_t button, uint8_t btnAction)
 
 void AppTask::ActionInitiated(OvenManager::Action_t aAction, int32_t aActor, uint8_t * aValue)
 {
-    // Action initiated
     bool lightOn = aAction == OvenManager::ON_ACTION;
     SILABS_LOG("Turning light %s", (lightOn) ? "On" : "Off");
 
-    //TODO: Update LED state
+    // TODO: Update LED state
 
 #ifdef DISPLAY_ENABLED
     sAppTask.GetLCD().WriteDemoUI(lightOn);
@@ -150,20 +149,19 @@ void AppTask::ActionInitiated(OvenManager::Action_t aAction, int32_t aActor, uin
 
 void AppTask::ActionCompleted(OvenManager::Action_t aAction)
 {
-    // action has been completed on the oven
+    // Action has been completed on the oven
     if (aAction == OvenManager::ON_ACTION)
     {
-        SILABS_LOG("Oven ON")
+        SILABS_LOG("Oven ON");
     }
     else if (aAction == OvenManager::OFF_ACTION)
     {
-        SILABS_LOG("Oven OFF")
+        SILABS_LOG("Oven OFF");
     }
 
     if (sAppTask.mSyncClusterToButtonAction)
     {
-        // chip::DeviceLayer::PlatformMgr().ScheduleWork(UpdateClusterState, reinterpret_cast<intptr_t>(nullptr));
-        // Schedule work to Update CookTop and CookSurfaceEndpoints (turn on/off)
+        // TODO: Schedule work to Update CookTop and CookSurfaceEndpoints (turn on/off)
         sAppTask.mSyncClusterToButtonAction = false;
     }
 }

examples/oven-app/silabs/src/DataModelCallbacks.cpp

@@ -48,7 +48,6 @@ void MatterPostAttributeChangeCallback(const app::ConcreteAttributePath & attrib
     case app::Clusters::OnOff::Id:
         ChipLogProgress(Zcl, "OnOff cluster ID: " ChipLogFormatMEI " Type: %u Value: %u, length %u",
                         ChipLogValueMEI(attributeId), type, *value, size);
-        ChipLogProgress(Zcl, "OnOff received for endpoint: %d", attributePath.mEndpointId);
         OvenManager::GetInstance().OnOffAttributeChangeHandler(attributePath.mEndpointId, attributeId, value, size);
         break;
     case app::Clusters::TemperatureControl::Id:

examples/oven-app/silabs/src/OvenManager.cpp

@@ -17,7 +17,6 @@
  */
 
 #include "OvenManager.h"
-// Corrected relative paths to shared oven-app-common headers (src/ -> ../../)
 #include "CookSurfaceEndpoint.h"
 #include "CookTopEndpoint.h"
 #include "OvenEndpoint.h"
@@ -110,21 +109,9 @@ void OvenManager::Init()
 
 void OvenManager::TempCtrlAttributeChangeHandler(EndpointId endpointId, AttributeId attributeId, uint8_t * value, uint16_t size)
 {
-    if (endpointId == kCookSurfaceEndpoint4 || endpointId == kCookSurfaceEndpoint5)
+    if (endpointId == kTemperatureControlledCabinetEndpoint2)
     {
-        // Handle temperature control attribute changes for the cook surface endpoints
-        if (*value == 0) // low
-        {
-            // TODO: Adjust the temperature-setpoint value in TemperatureControl Cluster accordingly
-        }
-        else if (*value == 1) // medium
-        {
-            // TODO: Adjust the temperature-setpoint value in TemperatureControl Cluster accordingly
-        }
-        else if (*value == 2) // high
-        {
-            // TODO: Adjust the temperature-setpoint value in TemperatureControl Cluster accordingly
-        }
+        // TODO: Update the LCD with the new Temperature Control attribute value
     }
     return;
 }
@@ -133,7 +120,6 @@ void OvenManager::OnOffAttributeChangeHandler(EndpointId endpointId, AttributeId
 {
     if (endpointId == kCookTopEndpoint3)
     {
-        // Handle LCD and LED Actions
         InitiateAction(AppEvent::kEventType_Oven, *value ? OvenManager::ON_ACTION : OvenManager::OFF_ACTION, value);
 
         // Update CookSurface states accordingly
@@ -174,27 +160,24 @@ bool OvenManager::InitiateAction(int32_t aActor, Action_t aAction, uint8_t * aVa
     State_t new_state;
 
     // Initiate Turn On/Off Action only when the previous one is complete.
-    // if (((mState == kState_OffCompleted) || mOffEffectArmed) && aAction == ON_ACTION)
     if (mState == kState_OffCompleted && aAction == ON_ACTION)
     {
         action_initiated = true;
-
-        new_state = kState_OnInitiated;
+        new_state        = kState_OnInitiated;
     }
     else if (mState == kState_OnCompleted && aAction == OFF_ACTION)
     {
         action_initiated = true;
-
-        new_state = kState_OffInitiated;
+        new_state        = kState_OffInitiated;
     }
 
     if (action_initiated && (aAction == ON_ACTION || aAction == OFF_ACTION))
     {
         mState = new_state;
 
         AppEvent event;
-        event.Type               = AppEvent::kEventType_Oven; // Assuming a distinct action type exists; fallback to Timer if not.
-        event.OvenEvent.Context = this; // Reuse Context field; adjust if a specific union member exists for action.
+        event.Type               = AppEvent::kEventType_Oven;
+        event.OvenEvent.Context  = this;
         event.Handler            = ActuatorMovementHandler;
         AppTask::GetAppTask().PostEvent(&event);
     }
@@ -211,17 +194,16 @@ void OvenManager::ActuatorMovementHandler(AppEvent * aEvent)
 {
     Action_t actionCompleted = INVALID_ACTION;
 
-    // Correct union member: event posted stored context in OvenEvent.Context, not TimerEvent.Context.
     OvenManager * oven = static_cast<OvenManager *>(aEvent->OvenEvent.Context);
 
     if (oven->mState == kState_OffInitiated)
     {
-        oven->mState   = kState_OffCompleted;
+        oven->mState    = kState_OffCompleted;
         actionCompleted = OFF_ACTION;
     }
     else if (oven->mState == kState_OnInitiated)
     {
-        oven->mState   = kState_OnCompleted;
+        oven->mState    = kState_OnCompleted;
         actionCompleted = ON_ACTION;
     }
 
@@ -270,15 +252,15 @@ void OvenManager::ProcessOvenModeChange(chip::EndpointId endpointId, uint8_t new
     using chip::Protocols::InteractionModel::Status;
     ChipLogProgress(AppServer, "OvenManager::ProcessOvenModeChange ep=%u newMode=%u", endpointId, newMode);
 
-    // 1. Verify newMode is among supported modes
+    // Verify newMode is among supported modes
     bool supported = TemperatureControlledCabinet::OvenModeDelegate::IsSupportedMode(newMode);
     if (!supported)
     {
         response.status = to_underlying(ModeBase::StatusCode::kUnsupportedMode);
         return;
     }
 
-    // 2. Read current mode
+    // Read Current Oven Mode
     uint8_t currentMode;
     Status attrStatus   = OvenMode::Attributes::CurrentMode::Get(endpointId, &currentMode);
     if (attrStatus != Status::Success)
@@ -289,14 +271,14 @@ void OvenManager::ProcessOvenModeChange(chip::EndpointId endpointId, uint8_t new
         return;
     }
 
-    // 3. No-op
+    // No action needed if current mode is the same as new mode
     if (currentMode == newMode)
     {
         response.status = to_underlying(ModeBase::StatusCode::kSuccess);
         return;
     }
 
-    // 4. Policy check
+    // Check if the mode transition is possible
     if (IsTransitionBlocked(currentMode, newMode))
     {
         ChipLogProgress(AppServer, "OvenManager: Blocked transition %u -> %u", currentMode, newMode);
@@ -305,7 +287,7 @@ void OvenManager::ProcessOvenModeChange(chip::EndpointId endpointId, uint8_t new
         return;
     }
 
-    // 5. Write new mode
+    // Write new mode
     Status writeStatus = OvenMode::Attributes::CurrentMode::Set(endpointId, newMode);
     if (writeStatus != Status::Success)
     {