Use single component rather than MCD

This commit changes the fan/light configuration to be a single component
rather than MCD, for reasons layed out in the review discussion.

Author: nickolas-deboom

drivers/SmartThings/matter-switch/profiles/light-color-level-fan.yml

@@ -18,17 +18,13 @@ components:
               range: [ 2200, 6500 ]
       - id: colorControl
         version: 1
+      - id: fanMode
+        version: 1
+      - id: fanSpeedPercent
+        version: 1
       - id: firmwareUpdate
         version: 1
       - id: refresh
         version: 1
     categories:
       - name: Light
-  - id: fan
-    capabilities:
-      - id: fanMode
-        version: 1
-      - id: fanSpeedPercent
-        version: 1
-    categories:
-      - name: Fan

drivers/SmartThings/matter-switch/src/init.lua

@@ -51,9 +51,12 @@ local SWITCH_INITIALIZED = "__switch_intialized"
 -- in the device table for devices that joined prior to this transition, and it
 -- will not be set for new devices.
 local COMPONENT_TO_ENDPOINT_MAP = "__component_to_endpoint_map"
--- COMPONENT_TO_ENDPOINT_MAP_NEW_DEVICES is for new devices that can be supported
--- by a MCD configuration.
-local COMPONENT_TO_ENDPOINT_MAP_NEW_DEVICES = "__component_to_endpoint_map_button"
+-- COMPONENT_TO_ENDPOINT_MAP_BUTTON is for devices with button endpoints, to
+-- preserve the MCD functionality for button devices from the matter-button
+-- driver after it was merged into the matter-switch driver. Note that devices
+-- containing both button endpoints and switch endpoints will use this field
+-- rather than COMPONENT_TO_ENDPOINT_MAP.
+local COMPONENT_TO_ENDPOINT_MAP_BUTTON = "__component_to_endpoint_map_button"
 local ENERGY_MANAGEMENT_ENDPOINT = "__energy_management_endpoint"
 local IS_PARENT_CHILD_DEVICE = "__is_parent_child_device"
 local COLOR_TEMP_BOUND_RECEIVED_KELVIN = "__colorTemp_bound_received_kelvin"
@@ -453,15 +456,15 @@ local function find_default_endpoint(device)
 end
 
 local function component_to_endpoint(device, component)
-  local map = device:get_field(COMPONENT_TO_ENDPOINT_MAP_NEW_DEVICES) or device:get_field(COMPONENT_TO_ENDPOINT_MAP) or {}
+  local map = device:get_field(COMPONENT_TO_ENDPOINT_MAP_BUTTON) or device:get_field(COMPONENT_TO_ENDPOINT_MAP) or {}
   if map[component] then
     return map[component]
   end
   return find_default_endpoint(device)
 end
 
 local function endpoint_to_component(device, ep)
-  local map = device:get_field(COMPONENT_TO_ENDPOINT_MAP_NEW_DEVICES) or device:get_field(COMPONENT_TO_ENDPOINT_MAP) or {}
+  local map = device:get_field(COMPONENT_TO_ENDPOINT_MAP_BUTTON) or device:get_field(COMPONENT_TO_ENDPOINT_MAP) or {}
   for component, endpoint in pairs(map) do
     if endpoint == ep then
       return component
@@ -584,43 +587,32 @@ local function find_child(parent, ep_id)
 end
 
 local function build_component_map(device, main_endpoint)
-  local component_name, endpoints
+  -- create component mapping on the main profile button endpoints
   local button_eps = device:get_endpoints(clusters.Switch.ID, {feature_bitmap=clusters.Switch.types.SwitchFeature.MOMENTARY_SWITCH})
-  local fan_eps = device:get_endpoints(clusters.FanControl.ID)
-  table.sort(button_eps)
-  table.sort(fan_eps)
-
-  if #button_eps > 0 and STATIC_BUTTON_PROFILE_SUPPORTED[#button_eps] then
-    component_name = "button"
-    endpoints = button_eps
-  elseif #fan_eps > 0 then
-    component_name = "fan"
-    endpoints = fan_eps
-  else
-    return
-  end
-
-  local component_map = {}
-  component_map["main"] = main_endpoint
-  for component_num, ep in ipairs(endpoints) do
-    if ep ~= main_endpoint then
-      local component = component_name
-      if #endpoints > 1 then
-        component = component .. component_num
+  if tbl_contains(STATIC_BUTTON_PROFILE_SUPPORTED, #button_eps) then
+    table.sort(button_eps)
+    local component_map = {}
+    component_map["main"] = main_endpoint
+    for component_num, ep in ipairs(button_eps) do
+      if ep ~= main_endpoint then
+        local button_component = "button"
+        if #button_eps > 1 then
+          button_component = button_component .. component_num
+        end
+        component_map[button_component] = ep
       end
-      component_map[component] = ep
     end
+    device:set_field(COMPONENT_TO_ENDPOINT_MAP_BUTTON, component_map, {persist = true})
   end
-  device:set_field(COMPONENT_TO_ENDPOINT_MAP_NEW_DEVICES, component_map, {persist = true})
 end
 
-local function build_mcd_profile(device, main_endpoint)
+local function build_profile(device, main_endpoint)
   local button_eps = device:get_endpoints(clusters.Switch.ID, {feature_bitmap=clusters.Switch.types.SwitchFeature.MOMENTARY_SWITCH})
   local fan_eps = device:get_endpoints(clusters.FanControl.ID)
   local profile_name
   local battery_supported
 
-  if #button_eps > 0 and STATIC_BUTTON_PROFILE_SUPPORTED[#button_eps] then
+  if tbl_contains(STATIC_BUTTON_PROFILE_SUPPORTED, #button_eps) then
     profile_name = string.gsub(#button_eps .. "-button", "1%-", "") -- remove the "1-" in a device with 1 button ep
     if device_type_supports_button_switch_combination(device, main_endpoint) then
       profile_name = "light-level-" .. profile_name
@@ -632,7 +624,7 @@ local function build_mcd_profile(device, main_endpoint)
       device:send(attribute_list_read)
     end
   elseif #fan_eps > 0 then
-    profile_name = "light-color-level-fan"
+    profile_name = "light-color-level-fan" -- currently the only supported fan+light configuration
   else
     return
   end
@@ -704,7 +696,7 @@ local function handle_light_switch_with_onOff_server_clusters(device, main_endpo
 end
 
 local function initialize_switch(driver, device, main_endpoint)
-  build_mcd_profile(device, main_endpoint)
+  build_profile(device, main_endpoint)
   build_component_map(device, main_endpoint)
   configure_buttons(device)
 

drivers/SmartThings/matter-switch/src/test/test_matter_light_fan.lua

@@ -16,6 +16,7 @@ local capabilities = require "st.capabilities"
 local clusters = require "st.matter.generated.zap_clusters"
 local t_utils = require "integration_test.utils"
 local test = require "integration_test"
+local version = require "version"
 
 local TRANSITION_TIME = 0
 local OPTIONS_MASK = 0x01
@@ -94,43 +95,44 @@ end
 
 test.set_test_init_function(test_init)
 
-test.register_message_test(
-  "Main switch component: switch capability should send the appropriate commands",
-  {
-    {
-      channel = "capability",
-      direction = "receive",
-      message = {
+test.register_coroutine_test(
+  "Switch capability should send the appropriate commands", function()
+    test.socket.capability:__queue_receive(
+      {
         mock_device.id,
         { capability = "switch", component = "main", command = "on", args = { } }
       }
-    },
-    {
-      channel = "matter",
-      direction = "send",
-      message = {
+    )
+    if version.api >= 11 then
+      test.socket.devices:__expect_send(
+        {
+          "register_native_capability_cmd_handler",
+          { device_uuid = mock_device.id, capability_id = "switch", capability_cmd_id = "on" }
+        }
+      )
+    end
+    test.socket.matter:__expect_send(
+      {
         mock_device.id,
-        clusters.OnOff.server.commands.On(mock_device, mock_device_ep1)
-      },
-    },
-    {
-      channel = "matter",
-      direction = "receive",
-      message = {
+        clusters.OnOff.server.commands.On(mock_device, 1)
+      }
+    )
+    test.socket.matter:__queue_receive(
+      {
         mock_device.id,
         clusters.OnOff.attributes.OnOff:build_test_report_data(mock_device, mock_device_ep1, true)
       }
-    },
-    {
-      channel = "capability",
-      direction = "send",
-      message = mock_device:generate_test_message("main", capabilities.switch.switch.on())
-    }
-  }
+    )
+    test.socket.capability:__expect_send(
+      mock_device:generate_test_message(
+        "main", capabilities.switch.switch.on()
+      )
+    )
+  end
 )
 
 test.register_message_test(
-  "Main switch component: Set color temperature should send the appropriate commands",
+  "Set color temperature should send the appropriate commands",
   {
     {
       channel = "capability",
@@ -187,7 +189,7 @@ test.register_message_test(
     {
       channel = "capability",
       direction = "send",
-      message = mock_device:generate_test_message("fan", capabilities.fanMode.fanMode.auto())
+      message = mock_device:generate_test_message("main", capabilities.fanMode.fanMode.auto())
     },
     {
       channel = "matter",
@@ -200,7 +202,7 @@ test.register_message_test(
     {
       channel = "capability",
       direction = "send",
-      message = mock_device:generate_test_message("fan", capabilities.fanMode.fanMode.auto())
+      message = mock_device:generate_test_message("main", capabilities.fanMode.fanMode.auto())
     },
     {
       channel = "matter",
@@ -213,7 +215,7 @@ test.register_message_test(
     {
       channel = "capability",
       direction = "send",
-      message = mock_device:generate_test_message("fan", capabilities.fanMode.fanMode("medium"))
+      message = mock_device:generate_test_message("main", capabilities.fanMode.fanMode("medium"))
     }
   }
 )
@@ -233,7 +235,7 @@ test.register_message_test(
     {
       channel = "capability",
       direction = "send",
-      message = mock_device:generate_test_message("fan", capabilities.fanMode.supportedFanModes({"off", "high"}, {visibility={displayed=false}}))
+      message = mock_device:generate_test_message("main", capabilities.fanMode.supportedFanModes({"off", "high"}, {visibility={displayed=false}}))
     },
     {
       channel = "matter",
@@ -246,7 +248,7 @@ test.register_message_test(
     {
       channel = "capability",
       direction = "send",
-      message = mock_device:generate_test_message("fan", capabilities.fanMode.supportedFanModes({"off", "low", "medium", "high", "auto"}, {visibility={displayed=false}}))
+      message = mock_device:generate_test_message("main", capabilities.fanMode.supportedFanModes({"off", "low", "medium", "high", "auto"}, {visibility={displayed=false}}))
     },
   }
 )