chore: Improve Sonos Memory Usage

@NoahCornell did some analysis of driver heap usage, and Sonos was
disproportionately above the mean for the heap-space-per-device line fit
on the dataset.

This PR improves memory usage by limiting the information we keep resident
from API responses to the fields that we actually use in regular operation.

Additional background:

One source of heap usage was unbounded memory growth due to task spawning
on a certain error pathway, which he already fixed in a different PR.

Another source was the fact that we were cacheing the entire API
response for the Player and Group information for the LAN's Sonos topology.
These API payloads are pretty large, and have actually gotten larger over time.

This has been the case for this driver for a very long time; the
decision to store the whole response object was made so that we would
have the information available if we needed it in the future for bug
fixes or enhancements.

It turns out that the information we're utilizing hasn't really changed
much over the last few years, so I'm feeling quite comfortable about
excising the majority of the payload information at this point. We see
pretty signifcant memory savings with these changes, and the savings
should scale appreciably with device count, which is a big win.

Author: dljsjr

drivers/SmartThings/sonos/src/api/rest.lua

@@ -74,7 +74,7 @@ local SonosRestApi = {}
 --- Query a Sonos Group IP address for individual player info
 ---@param url table a URL table created by `net_url`
 ---@param headers table<string,string>?
----@return SonosDiscoveryInfo|SonosErrorResponse|nil
+---@return SonosDiscoveryInfoObject|SonosErrorResponse|nil
 ---@return string|nil error
 function SonosRestApi.get_player_info(url, headers)
   url.path = "/api/v1/players/local/info"

drivers/SmartThings/sonos/src/api/sonos_connection.lua

@@ -169,7 +169,7 @@ local function _open_coordinator_socket(sonos_conn, household_id, self_player_id
     _, err = Router.open_socket_for_player(
       household_id,
       coordinator_id,
-      coordinator.player.websocketUrl,
+      coordinator.player.websocket_url,
       api_key
     )
     if err ~= nil then
@@ -406,7 +406,7 @@ function SonosConnection.new(driver, device)
             return
           end
           local group = household.groups[header.groupId] or { playerIds = {} }
-          for _, player_id in ipairs(group.playerIds) do
+          for _, player_id in ipairs(group.player_ids) do
             local device_for_player = self.driver:device_for_player(header.householdId, player_id)
             --- we've seen situations where these messages can be processed while a device
             --- is being deleted so we check for the presence of emit event as a proxy for
@@ -430,7 +430,7 @@ function SonosConnection.new(driver, device)
           return
         end
         local group = household.groups[header.groupId] or { playerIds = {} }
-        for _, player_id in ipairs(group.playerIds) do
+        for _, player_id in ipairs(group.player_ids) do
           local device_for_player = self.driver:device_for_player(header.householdId, player_id)
           --- we've seen situations where these messages can be processed while a device
           --- is being deleted so we check for the presence of emit event as a proxy for
@@ -453,7 +453,7 @@ function SonosConnection.new(driver, device)
           return
         end
         local group = household.groups[header.groupId] or { playerIds = {} }
-        for _, player_id in ipairs(group.playerIds) do
+        for _, player_id in ipairs(group.player_ids) do
           local device_for_player = self.driver:device_for_player(header.householdId, player_id)
           --- we've seen situations where these messages can be processed while a device
           --- is being deleted so we check for the presence of emit event as a proxy for
@@ -484,7 +484,7 @@ function SonosConnection.new(driver, device)
               return
             end
 
-            local url_ip = lb_utils.force_url_table(coordinator_player.player.websocketUrl).host
+            local url_ip = lb_utils.force_url_table(coordinator_player.player.websocket_url).host
             local base_url = lb_utils.force_url_table(
               string.format("https://%s:%s", url_ip, SonosApi.DEFAULT_SONOS_PORT)
             )
@@ -510,7 +510,7 @@ function SonosConnection.new(driver, device)
               end
               self.driver.sonos:update_household_favorites(header.householdId, new_favorites)
 
-              for _, player_id in ipairs(group.playerIds) do
+              for _, player_id in ipairs(group.player_ids) do
                 local device_for_player =
                   self.driver:device_for_player(header.householdId, player_id)
                 --- we've seen situations where these messages can be processed while a device

drivers/SmartThings/sonos/src/api/sonos_ssdp_discovery.lua

@@ -9,7 +9,140 @@ local utils = require "utils"
 local SonosApi = require "api"
 local SSDP_SCAN_INTERVAL_SECONDS = 600
 
+local SONOS_DEFAULT_PORT = 1443
+local SONOS_DEFAULT_WSS_PATH = "websocket/api"
+local SONOS_DEFAULT_REST_PATH = "api/v1"
+
+--- Cached information gathered during discovery scanning, created from a subset of the
+--- found [SonosSSDPInfo](lua://SonosSSDPInfo) and [SonosDiscoveryInfoObject](lua://SonosDiscoveryInfoObject)
+---
+--- @class SpeakerDiscoveryInfo
+--- @field public unique_key UniqueKey
+--- @field public mac_addr string
+--- @field public expires_at integer
+--- @field public ipv4 string
+--- @field public port integer
+--- @field public household_id HouseholdId
+--- @field public player_id PlayerId
+--- @field public name string
+--- @field public model string
+--- @field public model_display_name string
+--- @field public sw_gen integer
+--- @field public wss_url table
+--- @field public rest_url table
+--- @field public is_group_coordinator boolean
+--- @field public group_name string? nil if a speaker is the non-primary in a bonded set
+--- @field public group_id GroupId? nil if a speaker is the non-primary in a bonded set
+--- @field package wss_path string? nil if equivalent to the default value; does not include leading slash!
+--- @field package rest_path string? nil if equivalent to the default value; does not include leading slash!
+local SpeakerDiscoveryInfo = {}
+
+---@param ssdp_info SonosSSDPInfo
+---@param discovery_info SonosDiscoveryInfoObject
+---@return SpeakerDiscoveryInfo info
+function SpeakerDiscoveryInfo.new(ssdp_info, discovery_info)
+  local mac_addr = utils.extract_mac_addr(discovery_info.device)
+  local port, rest_path = string.match(discovery_info.restUrl, "^.*:(%d*)/(.*)$")
+  local _, wss_path = string.match(discovery_info.websocketUrl, "^.*:(%d*)/(.*)$")
+  port = tonumber(port) or SONOS_DEFAULT_PORT
+
+  local ret = {
+    unique_key = utils.sonos_unique_key_from_ssdp(ssdp_info),
+    expires_at = ssdp_info.expires_at,
+    ipv4 = ssdp_info.ip,
+    port = port,
+    mac_addr = mac_addr,
+    household_id = ssdp_info.household_id,
+    player_id = ssdp_info.player_id,
+    name = discovery_info.device.name,
+    model = discovery_info.device.model,
+    model_display_name = discovery_info.device.modelDisplayName,
+    sw_gen = discovery_info.device.swGen,
+    is_group_coordinator = ssdp_info.is_group_coordinator,
+  }
+
+  if type(ssdp_info.group_name) == "string" and #ssdp_info.group_name > 0 then
+    ret.group_name = ssdp_info.group_name
+  end
+
+  if type(ssdp_info.group_id) == "string" and #ssdp_info.group_id > 0 then
+    ret.group_id = ssdp_info.group_id
+  end
+
+  if type(wss_path) == "string" and #wss_path > 0 and wss_path ~= SONOS_DEFAULT_WSS_PATH then
+    ret.wss_path = wss_path
+  end
+
+  if type(rest_path) == "string" and #rest_path > 0 and rest_path ~= SONOS_DEFAULT_REST_PATH then
+    ret.rest_path = rest_path
+  end
+
+  local proxy_index = function(_, k)
+    if k == "rest_url" and not rawget(ret, "rest_url") then
+      rawset(
+        ret,
+        "rest_url",
+        net_url.parse(
+          string.format(
+            "https://%s:%s/%s",
+            ret.ipv4,
+            ret.port,
+            ret.rest_path or SONOS_DEFAULT_REST_PATH
+          )
+        )
+      )
+    end
+
+    if k == "wss_url" and not rawget(ret, "wss_url") then
+      rawset(
+        ret,
+        "wss_url",
+        net_url.parse(
+          string.format(
+            "https://%s:%s/%s",
+            ret.ipv4,
+            ret.port,
+            ret.wss_path or SONOS_DEFAULT_WSS_PATH
+          )
+        )
+      )
+    end
+
+    return rawget(ret, k)
+  end
+
+  local proxy_newindex = function(_, _, _)
+    error("attempt to index a read-only table", 2)
+  end
+
+  for k, v in pairs(SpeakerDiscoveryInfo) do
+    rawset(ret, k, v)
+  end
+
+  return setmetatable(ret, { __index = proxy_index, __newindex = proxy_newindex })
+end
+
+function SpeakerDiscoveryInfo:is_bonded()
+  return (self.group_id == nil)
+end
+
+---@return SonosSSDPInfo
+function SpeakerDiscoveryInfo:as_ssdp_info()
+  ---@type SonosSSDPInfo
+  return {
+    ip = self.ipv4,
+    group_id = self.group_id or "",
+    group_name = self.group_name or "",
+    expires_at = self.expires_at,
+    player_id = self.player_id,
+    wss_url = self.wss_url:build(),
+    household_id = self.household_id,
+    is_group_coordinator = self.is_group_coordinator,
+  }
+end
+
 local sonos_ssdp = {}
+sonos_ssdp.SpeakerDiscoveryInfo = SpeakerDiscoveryInfo
 
 ---@module 'luncheon.headers'
 
@@ -160,8 +293,8 @@ end
 
 ---@class SonosPersistentSsdpTask
 ---@field package ssdp_search_handle SsdpSearchHandle
----@field package player_info_by_sonos_ids table<UniqueKey, { ssdp_info: SonosSSDPInfo, discovery_info: SonosDiscoveryInfo }>
----@field package player_info_by_mac_addrs table<string, { ssdp_info: SonosSSDPInfo, discovery_info: SonosDiscoveryInfo }>
+---@field package player_info_by_sonos_ids table<UniqueKey, SpeakerDiscoveryInfo>
+---@field package player_info_by_mac_addrs table<string, SpeakerDiscoveryInfo>
 ---@field package waiting_for_unique_key table<UniqueKey, table[]>
 ---@field package waiting_for_mac_addr table<string, table[]>
 ---@field package control_tx table
@@ -217,7 +350,7 @@ function SonosPersistentSsdpTask:get_player_info(reply_tx, ...)
   end
 
   local maybe_existing = lookup_table[lookup_key]
-  if maybe_existing and maybe_existing.ssdp_info.expires_at > os.time() then
+  if maybe_existing and maybe_existing.expires_at > os.time() then
     reply_tx:send(maybe_existing)
     return
   end
@@ -267,11 +400,12 @@ function sonos_ssdp.spawn_persistent_ssdp_task()
     local maybe_known = task_handle.player_info_by_sonos_ids[unique_key]
     local is_new_information = not (
       maybe_known
-      and maybe_known.ssdp_info.expires_at > os.time()
-      and sonos_ssdp.ssdp_info_eq(maybe_known.ssdp_info, sonos_ssdp_info)
+      and maybe_known.expires_at > os.time()
+      and sonos_ssdp.ssdp_info_eq(maybe_known:as_ssdp_info(), sonos_ssdp_info)
     )
 
-    local info_to_send
+    local speaker_info
+    local event_bus_msg
 
     if is_new_information then
       local headers = SonosApi.make_headers()
@@ -283,30 +417,21 @@ function sonos_ssdp.spawn_persistent_ssdp_task()
       )
       if not discovery_info then
         log.error(string.format("Error getting discovery info from SSDP response: %s", err))
+      elseif discovery_info._objectType == "globalError" then
+        log.error(string.format("Error message in discovery info: %s", discovery_info.errorCode))
       else
-        local unified_info =
-          { ssdp_info = sonos_ssdp_info, discovery_info = discovery_info, force_refresh = true }
-        task_handle.player_info_by_sonos_ids[unique_key] = unified_info
-        info_to_send = unified_info
+        speaker_info = SpeakerDiscoveryInfo.new(sonos_ssdp_info, discovery_info)
+        task_handle.player_info_by_sonos_ids[unique_key] = speaker_info
+        event_bus_msg = { speaker_info = speaker_info, force_refresh = true }
       end
     else
-      info_to_send = {
-        ssdp_info = maybe_known.ssdp_info,
-        discovery_info = maybe_known.discovery_info,
-        force_refresh = false,
-      }
+      speaker_info = maybe_known
+      event_bus_msg = { speaker_info = speaker_info, force_refresh = false }
     end
 
-    if info_to_send then
-      if not (info_to_send.discovery_info and info_to_send.discovery_info.device) then
-        log.error_with(
-          { hub_logs = false },
-          st_utils.stringify_table(info_to_send, "Sonos Discovery Info has unexpected structure")
-        )
-        return
-      end
-      event_bus:send(info_to_send)
-      local mac_addr = utils.extract_mac_addr(info_to_send.discovery_info.device)
+    if speaker_info then
+      event_bus:send(event_bus_msg)
+      local mac_addr = speaker_info.mac_addr
       local waiting_handles = task_handle.waiting_for_unique_key[unique_key] or {}
 
       log.debug(st_utils.stringify_table(waiting_handles, "waiting for unique keys", true))
@@ -318,7 +443,7 @@ function sonos_ssdp.spawn_persistent_ssdp_task()
         st_utils.stringify_table(waiting_handles, "waiting for unique keys and mac addresses", true)
       )
       for _, reply_tx in ipairs(waiting_handles) do
-        reply_tx:send(info_to_send)
+        reply_tx:send(speaker_info)
       end
 
       task_handle.waiting_for_unique_key[unique_key] = {}

drivers/SmartThings/sonos/src/lifecycle_handlers.lua

@@ -68,7 +68,7 @@ function SonosDriverLifecycleHandlers.initialize_device(driver, device)
             if not info then
               device.log.warn(string.format("error receiving device info: %s", recv_err))
             else
-              ---@cast info { ssdp_info: SonosSSDPInfo, discovery_info: SonosDiscoveryInfo, force_refresh: boolean }
+              ---@cast info SpeakerDiscoveryInfo
               local auth_success, api_key_or_err = driver:check_auth(info)
               if not auth_success then
                 device:offline()