Merge pull request #986 from SmartThingsCommunity/matter-switch-fix-e…

…ndpoint-mapping

Matter Switch: fix endpoint to component mapping

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

@@ -29,16 +29,22 @@ local COLOR_TEMPERATURE_KELVIN_MIN = 1
 local COLOR_TEMPERATURE_MIRED_MAX = CONVERSION_CONSTANT/COLOR_TEMPERATURE_KELVIN_MIN
 local COLOR_TEMPERATURE_MIRED_MIN = CONVERSION_CONSTANT/COLOR_TEMPERATURE_KELVIN_MAX
 
+local ENDPOINT_TO_COMPONENT_MAP = "__endpoint_to_component_map"
+-- New profiles need to be added for devices that have more switch endpoints
+local MAX_MULTI_SWITCH_EPS = 7
+
 local function convert_huesat_st_to_matter(val)
   return math.floor((val * 0xFE) / 100.0 + 0.5)
 end
 
 --- component_to_endpoint helper function to handle situations where
 --- device does not have endpoint ids in sequential order from 1
 --- In this case the function returns the lowest endpoint value that isn't 0
+--- and supports the OnOff cluster. This is done to bypass the
+--- BRIDGED_NODE_DEVICE_TYPE on bridged devices
 local function find_default_endpoint(device, component)
   local res = device.MATTER_DEFAULT_ENDPOINT
-  local eps = device:get_endpoints(nil)
+  local eps = device:get_endpoints(clusters.OnOff.ID)
   table.sort(eps)
   for _, v in ipairs(eps) do
     if v ~= 0 then --0 is the matter RootNode endpoint
@@ -49,23 +55,61 @@ local function find_default_endpoint(device, component)
   return res
 end
 
-local function component_to_endpoint(device, component_id)
-  -- Assumes matter endpoint layout is sequentional starting at 1.
-  local ep_num = component_id:match("switch(%d)")
-  return ep_num and tonumber(ep_num) or find_default_endpoint(device, component_id)
+local function initialize_switch(device)
+  local switch_eps = device:get_endpoints(clusters.OnOff.ID)
+  table.sort(switch_eps)
+  local endpoint_map = {}
+
+  -- For switch devices, the profile components follow the naming convention "switch%d",
+  -- with the exception of "main" being the first component. Each component will then map
+  -- to the next lowest endpoint that hasn't been mapped yet.
+  for i, ep in ipairs(switch_eps) do
+    if i == 1 then
+      endpoint_map[ep] = "main"
+    else
+      endpoint_map[ep] = string.format("switch%d", i)
+    end
+  end
+
+  device:set_field(ENDPOINT_TO_COMPONENT_MAP, endpoint_map, {persist = true})
+  -- Note: This profile switching is needed because of shortcoming in the generic fingerprints
+  -- where devices with multiple endpoints with the same device type cannot be detected
+  local num_switch_eps = #switch_eps
+  if num_switch_eps > 1 then
+    device:try_update_metadata({profile = string.format("switch-%d", math.min(num_switch_eps, MAX_MULTI_SWITCH_EPS))})
+  end
+  if num_switch_eps > MAX_MULTI_SWITCH_EPS then
+    error(string.format(
+      "Matter multi switch device will have limited function. Profile doesn't exist with %d components, max is %d",
+      num_switch_eps,
+      MAX_MULTI_SWITCH_EPS
+    ))
+  end
+end
+
+local function component_to_endpoint(device, component_name)
+  local map = device:get_field(ENDPOINT_TO_COMPONENT_MAP) or {}
+  for ep, component in pairs(map) do
+    if component == component_name then return ep end
+  end
+  return find_default_endpoint(device, component_name)
 end
 
 local function endpoint_to_component(device, ep)
-  local switch_comp = string.format("switch%d", ep)
-  if device.profile.components[switch_comp] ~= nil then
-    return switch_comp
-  else
-    return "main"
+  local map = device:get_field(ENDPOINT_TO_COMPONENT_MAP) or {}
+  if map[ep] and device.profile.components[map[ep]] then
+    return map[ep]
   end
+  log.warn_with({hub_logs = true}, string.format("Device has more than supported number of switches (max %d), mapping excess endpoint to main component", MAX_MULTI_SWITCH_EPS))
+  return "main"
 end
 
 local function device_init(driver, device)
   log.info_with({hub_logs=true}, "device init")
+  if not device:get_field(ENDPOINT_TO_COMPONENT_MAP) then
+    -- create endpoint to component map and switch profile as needed
+    initialize_switch(device)
+  end
   device:set_component_to_endpoint_fn(component_to_endpoint)
   device:set_endpoint_to_component_fn(endpoint_to_component)
   device:subscribe()
@@ -75,28 +119,6 @@ local function device_removed(driver, device)
   log.info("device removed")
 end
 
-local function do_configure(driver, device)
-  -- New profiles need to be added for devices that have more switch endpoints
-  local MAX_MULTI_SWITCH_EPS = 7
-  -- Note: This profile switching is needed because of shortcoming in the generic fingerprints
-  -- where devices with multiple endpoints with the same device type cannot be detected
-  local switch_eps = device:get_endpoints(clusters.OnOff.ID)
-  local num_switch_eps = #switch_eps
-  table.sort(switch_eps)
-  --Default MCD switch handling depends on consecutive endpoint numbering
-  if num_switch_eps == switch_eps[num_switch_eps] then
-    if num_switch_eps > 1 then
-      device:try_update_metadata({profile = string.format("switch-%d", math.min(num_switch_eps, MAX_MULTI_SWITCH_EPS))})
-    end
-    if num_switch_eps > MAX_MULTI_SWITCH_EPS then
-      error(string.format(
-        "Matter multi switch device will not function. Profile doesn't exist with %d components",
-        num_switch_eps
-      ))
-    end
-  end
-end
-
 local function handle_switch_on(driver, device, cmd)
   local endpoint_id = device:component_to_endpoint(cmd.component)
   --TODO use OnWithRecallGlobalScene for devices with the LT feature
@@ -280,8 +302,7 @@ end
 local matter_driver_template = {
   lifecycle_handlers = {
     init = device_init,
-    removed = device_removed,
-    doConfigure = do_configure,
+    removed = device_removed
   },
   matter_handlers = {
     attr = {

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

@@ -99,7 +99,6 @@ local function test_init()
   end
   test.socket.matter:__expect_send({mock_device_no_hue_sat.id, subscribe_request})
   test.mock_device.add_test_device(mock_device_no_hue_sat)
-
 end
 test.set_test_init_function(test_init)
 

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

@@ -100,36 +100,86 @@ local mock_2switch = test.mock_device.build_test_matter_device({
   }
 })
 
+local mock_3switch_non_sequential = test.mock_device.build_test_matter_device({
+  profile = t_utils.get_profile_definition("light-binary.yml"),
+  manufacturer_info = {
+    vendor_id = 0x0000,
+    product_id = 0x0000,
+  },
+  endpoints = {
+    {
+      endpoint_id = 0,
+      clusters = {
+        {cluster_id = clusters.Basic.ID, cluster_type = "SERVER"},
+      },
+      device_types = {
+        device_type_id = 0x0016, device_type_revision = 1, -- RootNode
+      }
+    },
+    {
+      endpoint_id = 10,
+      clusters = {
+        {cluster_id = clusters.OnOff.ID, cluster_type = "SERVER"},
+      },
+      device_types = {
+        device_type_id = 0x0100, device_type_revision = 2, -- On/Off Light
+      }
+    },
+    {
+      endpoint_id = 14,
+      clusters = {
+        {cluster_id = clusters.OnOff.ID, cluster_type = "SERVER"},
+      },
+      device_types = {
+        device_type_id = 0x0100, device_type_revision = 2, -- On/Off Light
+      }
+    },
+    {
+      endpoint_id = 15,
+      clusters = {
+        {cluster_id = clusters.OnOff.ID, cluster_type = "SERVER"},
+      },
+      device_types = {
+        device_type_id = 0x0100, device_type_revision = 2, -- On/Off Light
+      }
+    },
+  }
+})
 
-local function test_init()
+local function test_init_mock_3switch()
   local cluster_subscribe_list = {
     clusters.OnOff.attributes.OnOff,
   }
   test.socket.matter:__set_channel_ordering("relaxed")
   local subscribe_request = cluster_subscribe_list[1]:subscribe(mock_3switch)
   test.socket.matter:__expect_send({mock_3switch.id, subscribe_request})
   test.mock_device.add_test_device(mock_3switch)
+  mock_3switch:expect_metadata_update({ profile = "switch-3" })
+end
+
+local function test_init_mock_2switch()
+  local cluster_subscribe_list = {
+    clusters.OnOff.attributes.OnOff,
+  }
+  test.socket.matter:__set_channel_ordering("relaxed")
   local subscribe_request = cluster_subscribe_list[1]:subscribe(mock_2switch)
   test.socket.matter:__expect_send({mock_2switch.id, subscribe_request})
   test.mock_device.add_test_device(mock_2switch)
+  mock_2switch:expect_metadata_update({ profile = "switch-2" })
 end
-test.set_test_init_function(test_init)
-
 
-test.register_coroutine_test(
-  "Profile change for 3 switch device", function()
-    test.socket.device_lifecycle:__queue_receive({ mock_3switch.id, "doConfigure" })
-    mock_3switch:expect_metadata_update({ profile = "switch-3" })
-    mock_3switch:expect_metadata_update({ provisioning_state = "PROVISIONED" })
-end)
-
-test.register_coroutine_test(
-  "Profile change for 2 switch device", function()
-    test.socket.device_lifecycle:__queue_receive({ mock_2switch.id, "doConfigure" })
-    mock_2switch:expect_metadata_update({ profile = "switch-2" })
-    mock_2switch:expect_metadata_update({ provisioning_state = "PROVISIONED" })
-end)
+local function test_init_mock_3switch_non_sequential()
+  local cluster_subscribe_list = {
+    clusters.OnOff.attributes.OnOff,
+  }
+  test.socket.matter:__set_channel_ordering("relaxed")
+  local subscribe_request = cluster_subscribe_list[1]:subscribe(mock_3switch_non_sequential)
+  test.socket.matter:__expect_send({mock_3switch_non_sequential.id, subscribe_request})
+  test.mock_device.add_test_device(mock_3switch_non_sequential)
+  mock_3switch_non_sequential:expect_metadata_update({ profile = "switch-3" })
+end
 
+-- The custom "test_init" function also checks that the appropriate profile is switched on init
 test.register_message_test(
   "On command to component switch should send the appropriate commands",
   {
@@ -149,7 +199,56 @@ test.register_message_test(
         clusters.OnOff.server.commands.On(mock_3switch, 2)
       }
     }
-  }
+  },
+  { test_init = test_init_mock_3switch }
+)
+
+-- The custom "test_init" function also checks that the appropriate profile is switched on init
+test.register_message_test(
+  "On command to main component should send the appropriate commands",
+  {
+    {
+      channel = "capability",
+      direction = "receive",
+      message = {
+        mock_2switch.id,
+        { capability = "switch", component = "main", command = "on", args = { } }
+      }
+    },
+    {
+      channel = "matter",
+      direction = "send",
+      message = {
+        mock_2switch.id,
+        clusters.OnOff.server.commands.On(mock_2switch, 1)
+      }
+    }
+  },
+  { test_init = test_init_mock_2switch }
+)
+
+-- The custom "test_init" function also checks that the appropriate profile is switched on init
+test.register_message_test(
+  "On command to component switch should send the appropriate commands for devices with non-sequential endpoints",
+  {
+    {
+      channel = "capability",
+      direction = "receive",
+      message = {
+        mock_3switch_non_sequential.id,
+        { capability = "switch", component = "switch3", command = "on", args = { } }
+      }
+    },
+    {
+      channel = "matter",
+      direction = "send",
+      message = {
+        mock_3switch_non_sequential.id,
+        clusters.OnOff.server.commands.On(mock_3switch_non_sequential, 15) -- switch 3 is on endpoint 15
+      }
+    }
+  },
+  { test_init = test_init_mock_3switch_non_sequential }
 )
 
 test.run_registered_tests()