Skip to content

[SL-TEMP] MATTER-5950: Multi sensor app refactor (#1033) - #1260

Merged
Michael Duggan (miduggan24) merged 3 commits into
release_2.10-1.6.1from
cp/multi_sensor
Oct 20, 2026
Merged

Michael Duggan (miduggan24) merged 3 commits into
release_2.10-1.6.1from
cp/multi_sensor

Conversation

@miduggan24

@miduggan24 Michael Duggan (miduggan24) commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor

cherry-pick #1033

Summary

  • Merge SensorManager and DataModelCallbacks into AppTask
  • Create AppTaskImpl as CRTP base
  • Streamline APIs per class diagram from design doc
  • Add SensorConfig.h for user configurable endpoints

Related issues

MATTER-5950

Testing

CI (full run with smoke)
Local testing of overrides

@miduggan24
Michael Duggan (miduggan24) requested a review from a team as a code owner October 8, 2026 19:05
@cursor

cursor Bot commented Oct 8, 2026 •

Copy link
Copy Markdown

PR Summary

Medium Risk
Large structural move of Matter cluster updates and attribute-change handling into AppTask; behavior should match prior SensorManager logic but regressions in reporting, occupancy toggle, or customer overrides are possible.

Overview
Refactors the Silabs multi-sensor example so sensor behavior lives in AppTask instead of separate SensorManager / DataModelCallbacks modules, and wires the app into the shared CustomerAppTask + AppTaskImpl CRTP override pattern.

AppTask now owns initialization, periodic reads, occupancy button handling, AttributeChangeListener posting for occupancy/temp/humidity, cluster getters, and the former Identify logging callback. The build drops SensorManager.cpp / DataModelCallbacks.cpp, adds CustomerAppTask.cpp, and exposes SensorConfig.h so occupancy, temperature, and humidity cluster endpoints are user-configurable. CHIP_SILABS_APP_USE_CUSTOMER_APP_TASK enables the customer task path; LCD code reads measurements via AppTask::GetAppTask() instead of SensorManager.

Reviewed by Cursor Bugbot for commit c10219e. Configure here.

@Sarthak-Shaha sarthak shaha (Sarthak-Shaha) left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should this be SL-TEMP in order to avoid merging it back to main?

@cursor cursor Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes using default effort and found 2 potential issues.

Fix All in Cursor

Bugbot Autofix prepared fixes for both issues found in the latest run.

  • ✅ Fixed: Wrong display enable macro used
    • Replaced all instances of #ifdef DISPLAY_ENABLED with #if SL_MATTER_DISPLAY_ENABLED to properly gate LCD code compilation.
  • ✅ Fixed: Stale copyright year on new files
    • Updated copyright year from 2024 to 2026 in AppTaskImpl.h and SensorConfig.h to reflect their creation year.

Create PR

Or push these changes by commenting:

@cursor push ee1826a976
Preview (ee1826a976)
diff --git a/examples/multi-sensor-app/silabs/include/AppTask.h b/examples/multi-sensor-app/silabs/include/AppTask.h
--- a/examples/multi-sensor-app/silabs/include/AppTask.h
+++ b/examples/multi-sensor-app/silabs/include/AppTask.h
@@ -142,7 +142,7 @@
      */
     bool IsOccupancyDetected();
 
-#ifdef DISPLAY_ENABLED
+#if SL_MATTER_DISPLAY_ENABLED
     /**
      * @brief LCD callback function to cycle LCD display.
      *        Function switch to the next LCD UI and update the show values.

diff --git a/examples/multi-sensor-app/silabs/include/AppTaskImpl.h b/examples/multi-sensor-app/silabs/include/AppTaskImpl.h
--- a/examples/multi-sensor-app/silabs/include/AppTaskImpl.h
+++ b/examples/multi-sensor-app/silabs/include/AppTaskImpl.h
@@ -1,6 +1,6 @@
 /*
  *
- *    Copyright (c) 2024 Project CHIP Authors
+ *    Copyright (c) 2026 Project CHIP Authors
  *    All rights reserved.
  *
  *    Licensed under the Apache License, Version 2.0 (the "License");

diff --git a/examples/multi-sensor-app/silabs/include/SensorConfig.h b/examples/multi-sensor-app/silabs/include/SensorConfig.h
--- a/examples/multi-sensor-app/silabs/include/SensorConfig.h
+++ b/examples/multi-sensor-app/silabs/include/SensorConfig.h
@@ -1,6 +1,6 @@
 /*
  *
- *    Copyright (c) 2024 Project CHIP Authors
+ *    Copyright (c) 2026 Project CHIP Authors
  *    All rights reserved.
  *
  *    Licensed under the Apache License, Version 2.0 (the "License");

diff --git a/examples/multi-sensor-app/silabs/src/AppTask.cpp b/examples/multi-sensor-app/silabs/src/AppTask.cpp
--- a/examples/multi-sensor-app/silabs/src/AppTask.cpp
+++ b/examples/multi-sensor-app/silabs/src/AppTask.cpp
@@ -54,7 +54,7 @@
 #include <crypto/RandUtils.h>
 #endif // defined(SL_MATTER_USE_SI70XX_SENSOR) && SL_MATTER_USE_SI70XX_SENSOR
 
-#if defined(DISPLAY_ENABLED) && DISPLAY_ENABLED
+#if SL_MATTER_DISPLAY_ENABLED
 #include <SensorsUI.h>
 #endif
 
@@ -225,7 +225,7 @@
         break;
     }
 }
-#endif // DISPLAY_ENABLED
+#endif // SL_MATTER_DISPLAY_ENABLED
 
 void AppTask::ButtonEventHandler(uint8_t button, uint8_t btnAction)
 {
@@ -302,9 +302,9 @@
     VerifyOrReturn(aEvent != nullptr);
     VerifyOrReturn(aEvent->Type == AppEvent::kEventType_SensorAttributeUpdate);
 
-#ifdef DISPLAY_ENABLED
+#if SL_MATTER_DISPLAY_ENABLED
     GetAppTask().UpdateSensorDisplay();
-#endif // DISPLAY_ENABLED
+#endif // SL_MATTER_DISPLAY_ENABLED
 }
 
 void AppTask::OccupancyAttributeUpdateEvent(AppEvent * aEvent)
@@ -314,9 +314,9 @@
 
     sOccupancyLed.Set(aEvent->OccupancyEvent.occupancyDetected);
 
-#ifdef DISPLAY_ENABLED
+#if SL_MATTER_DISPLAY_ENABLED
     GetAppTask().UpdateSensorDisplay();
-#endif // DISPLAY_ENABLED
+#endif // SL_MATTER_DISPLAY_ENABLED
 }
 
 void AppTask::TriggerSensorAction(chip::System::Layer * aLayer, void * aAppState)

You can send follow-ups to the cloud agent here.

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit c10219e. Configure here.

Comment thread examples/multi-sensor-app/silabs/include/AppTask.h
Comment thread examples/multi-sensor-app/silabs/include/AppTaskImpl.h
@miduggan24 Michael Duggan (miduggan24) changed the title [SL-ONLY] MATTER-5950: Multi sensor app refactor (#1033) [SL-TEMP] MATTER-5950: Multi sensor app refactor (#1033) Oct 8, 2026
@miduggan24

Michael Duggan (miduggan24) commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor Author

Should this be SL-TEMP in order to avoid merging it back to main?

good point, updated this and oven accordingly

@Sarthak-Shaha sarthak shaha (Sarthak-Shaha) added the sl-temp This TAG indicates that this commit is silabs-specific that will not be upstreamed or carried over. label Oct 8, 2026
* initial consolidation into AppTask

* implement CRTP architecture

* Restyled by whitespace

* fix drift for sensor manager initalization

* add missing include

* SensorActionTriggered -> TriggerSensorAction

* consolidate repeated code into helper functions

* register OnAttributeChanged in AppTask

* wrap simulated value on kAttributeChangeReportThreshold < 3

---------

Co-authored-by: Restyled.io <commits@restyled.io>
- Replace all instances of #ifdef DISPLAY_ENABLED with #if SL_MATTER_DISPLAY_ENABLED
  to properly gate LCD/display code (DISPLAY_ENABLED=0 would still compile with #ifdef)
- Update copyright year from 2024 to 2026 in AppTaskImpl.h and SensorConfig.h
  (new files created in 2026)

Applied via @cursor push command
@miduggan24
Michael Duggan (miduggan24) merged commit c55d6a5 into release_2.10-1.6.1 Oct 20, 2026
85 checks passed
@miduggan24
Michael Duggan (miduggan24) deleted the cp/multi_sensor branch October 20, 2026 17:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

sl-temp This TAG indicates that this commit is silabs-specific that will not be upstreamed or carried over.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants