Skip to content

Conversation

@arun-silabs
Copy link
Contributor

Summary

This PR adds the logic to handle the hardware actions in Oven app.The actions mainly are : Updating LED, updating LCD and handling the oven-app behavior on button press.

Related issues

MATTER-5575

Testing

Built oven-app locally and tested the LCD, LED updates by sending onoff command and verified the button 1 implementation locally.

Readability checklist

The checklist below will help the reviewer finish PR review in time and keep the
code readable:

  • PR title is
    descriptive
  • Apply the
    “When in Rome…”
    rule (coding style)
  • PR size is short
  • Try to avoid "squashing" and "force-update" in commit history
  • CI time didn't increase

See: Pull Request Guidelines

@arun-silabs arun-silabs requested a review from a team as a code owner November 4, 2025 04:39
#include "AppEvent.h"
#include "LEDWidget.h"
#include "OvenBindingHandler.h"
#include "OvenUI.h"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this intended? Seems like double inclusion

@Sarthak-Shaha Sarthak-Shaha requested a review from Copilot November 4, 2025 15:12
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR implements hardware UI functionality for the Silabs oven application, including LCD display management, LED control, and button input handling. The implementation adds visual feedback for cooktop state and oven mode, along with button-triggered control of the cooktop endpoint.

Key changes:

  • New OvenUI class with LCD rendering methods for cooktop state and oven mode display
  • LED control integration with cooktop on/off state
  • Button handler for toggling cooktop and triggering binding events

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
examples/oven-app/silabs/src/OvenUI.cpp New file implementing LCD UI rendering for oven state visualization
examples/oven-app/silabs/include/OvenUI.h Header defining OvenUI class interface for LCD drawing functions
examples/oven-app/silabs/src/OvenManager.cpp Adds initialization of cooktop state and oven mode from cluster attributes, updates LCD on attribute changes
examples/oven-app/silabs/include/OvenManager.h Adds getter methods for cooktop state, oven mode, and endpoint IDs
examples/oven-app/silabs/src/AppTask.cpp Implements button handling, LED updates, and LCD refresh on state changes
examples/oven-app/silabs/include/AppTask.h Adds UpdateLED/UpdateLCD methods and OvenButtonHandler declaration
examples/oven-app/silabs/BUILD.gn Adds OvenUI.cpp to build sources

#include "OvenUI.h"

#ifdef DISPLAY_ENABLED
#include "OvenUI.h"
Copy link

Copilot AI Nov 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Duplicate include of 'OvenUI.h' - this header is already included at line 25. Remove this duplicate inclusion.

Suggested change
#include "OvenUI.h"

Copilot uses AI. Check for mistakes.
: OvenManager::ON_ACTION;

// Toggle CookTop
chip::DeviceLayer::PlatformMgr().ScheduleWork(UpdateClusterState, reinterpret_cast<intptr_t>(nullptr));
Copy link

Copilot AI Nov 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using reinterpret_cast<intptr_t>(nullptr) is unnecessarily complex. Since the UpdateClusterState function doesn't use the context parameter, pass 0 directly or use static_cast<intptr_t>(0) for clarity.

Suggested change
chip::DeviceLayer::PlatformMgr().ScheduleWork(UpdateClusterState, reinterpret_cast<intptr_t>(nullptr));
chip::DeviceLayer::PlatformMgr().ScheduleWork(UpdateClusterState, 0);

Copilot uses AI. Check for mistakes.
Comment on lines +227 to +231
ChipLogProgress(AppServer, "Updating cooktop OnOff cluster state to %s", currentState ? "On" : "Off");

// Set the OnOff attribute value for the cooktop endpoint
Protocols::InteractionModel::Status status =
OnOffServer::Instance().setOnOffValue(OvenManager::GetCookTopEndpoint(), currentState, false);
Copy link

Copilot AI Nov 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The OnOff cluster is being set to the current state rather than the new toggled state. Line 225 reads the current state, but the toggle logic should invert this value before setting it. The state should be !currentState to achieve the toggle behavior.

Suggested change
ChipLogProgress(AppServer, "Updating cooktop OnOff cluster state to %s", currentState ? "On" : "Off");
// Set the OnOff attribute value for the cooktop endpoint
Protocols::InteractionModel::Status status =
OnOffServer::Instance().setOnOffValue(OvenManager::GetCookTopEndpoint(), currentState, false);
ChipLogProgress(AppServer, "Updating cooktop OnOff cluster state to %s", !currentState ? "On" : "Off");
// Set the OnOff attribute value for the cooktop endpoint
Protocols::InteractionModel::Status status =
OnOffServer::Instance().setOnOffValue(OvenManager::GetCookTopEndpoint(), !currentState, false);

Copilot uses AI. Check for mistakes.

/**
* @brief PB0 Button event processing function
* Press and hold will trigger a factory reset timer start
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this not apply anymore?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants