Merge pull request #2170 from SmartThingsCommunity/fix/sonos-backwards-compat-crashes

dljsjr · web-flow · commit 3f04c239f3b6 · 2025-06-04T20:50:45.000-05:00
diff --git a/drivers/SmartThings/sonos/src/api/sonos_connection.lua b/drivers/SmartThings/sonos/src/api/sonos_connection.lua
@@ -1,3 +1,4 @@
+local api_version = require "version".api
 local cosock = require "cosock"
 local json = require "st.json"
 local log = require "log"
@@ -74,7 +75,13 @@ local _update_subscriptions_helper = function(
       {},
     }
     local payload = json.encode(payload_table)
-    Router.send_message_to_player(utils.sonos_unique_key(householdId, playerId), payload, reply_tx)
+    local unique_key, bad_key_part = utils.sonos_unique_key(householdId, playerId)
+    if not unique_key then
+      local err_msg = string.format("Invalid Sonos Unique Key Part: %s", bad_key_part)
+      reply_tx:send(table.pack(nil, err_msg))
+      return
+    end
+    Router.send_message_to_player(unique_key, payload, reply_tx)
   end
 end
 
@@ -168,10 +175,13 @@ local function _open_coordinator_socket(sonos_conn, household_id, self_player_id
     end
 
     local listener_id
-    listener_id, err = Router.register_listener_for_socket(
-      sonos_conn,
-      utils.sonos_unique_key(household_id, coordinator_id)
-    )
+    local unique_key, bad_key_part = utils.sonos_unique_key(household_id, coordinator_id)
+
+    if not unique_key then
+      log.error(string.format("Invalid Sonos Unique Key Part: %s", bad_key_part))
+    end
+
+    listener_id, err = Router.register_listener_for_socket(sonos_conn, unique_key)
     if err ~= nil or not listener_id then
       log.error(err)
     else
@@ -208,7 +218,22 @@ local function backoff_builder(max, inc, rand)
 end
 
 ---@param sonos_conn SonosConnection
-local function _spawn_reconnect_task(sonos_conn)
+local function _legacy_reconnect_task(sonos_conn)
+  log.debug("Spawning reconnect task for ", sonos_conn.device.label)
+  cosock.spawn(function()
+    local backoff = backoff_builder(60, 1, 0.1)
+    while not sonos_conn:is_running() do
+      local start_success = sonos_conn:start()
+      if start_success then
+        return
+      end
+      cosock.socket.sleep(backoff())
+    end
+  end, string.format("%s Reconnect Task", sonos_conn.device.label))
+end
+
+---@param sonos_conn SonosConnection
+local function _oauth_reconnect_task(sonos_conn)
   log.debug("Spawning reconnect task for ", sonos_conn.device.label)
   if not sonos_conn.driver:is_waiting_for_oauth_token() then
     sonos_conn.driver:request_oauth_token()
@@ -243,6 +268,15 @@ local function _spawn_reconnect_task(sonos_conn)
   end, string.format("%s Reconnect Task", sonos_conn.device.label))
 end
 
+---@param sonos_conn SonosConnection
+local function _spawn_reconnect_task(sonos_conn)
+  if type(api_version) == "number" and api_version >= 14 then
+    _oauth_reconnect_task(sonos_conn)
+  else
+    _legacy_reconnect_task(sonos_conn)
+  end
+end
+
 --- Create a new Sonos connection to manage the given device
 --- @param driver SonosDriver
 --- @param device SonosDevice
@@ -276,15 +310,29 @@ function SonosConnection.new(driver, device)
       local header, body = table.unpack(table.unpack(json_result))
       if header.type == "globalError" then
         if body.errorCode == "ERROR_NOT_AUTHORIZED" then
-          local household_id, player_id = driver.sonos:get_player_for_device(device)
-          device.log.warn(string.format("WebSocket connection no longer authorized, disconnecting"))
-          local _, security_err = driver:request_oauth_token()
-          if security_err then
-            log.warn(string.format("Error during request for oauth token: %s", security_err))
+          if api_version < 14 then
+            log.error(
+              "Unable to authenticate Sonos WebSocket Connection, and current API does not support Sonos OAuth"
+            )
+            self:stop()
+          else
+            local household_id, player_id = driver.sonos:get_player_for_device(device)
+            device.log.warn(
+              string.format("WebSocket connection no longer authorized, disconnecting")
+            )
+            local _, security_err = driver:request_oauth_token()
+            if security_err then
+              log.warn(string.format("Error during request for oauth token: %s", security_err))
+            end
+            -- closing the socket directly without calling `:stop()` triggers the reconnect loop,
+            -- which is where we wait for the token to come in.
+            local unique_key, bad_key_part = utils.sonos_unique_key(household_id, player_id)
+            if not unique_key then
+              log.error(string.format("Invalid Sonos Unique Key Part: %s", bad_key_part))
+            else
+              Router.close_socket_for_player(unique_key)
+            end
           end
-          -- closing the socket directly without calling `:stop()` triggers the reconnect loop,
-          -- which is where we wait for the token to come in.
-          Router.close_socket_for_player(utils.sonos_unique_key(household_id, player_id))
         end
       elseif header.type == "groups" then
         log.trace(string.format("Groups type message for %s", device_name))
@@ -494,15 +542,32 @@ end
 function SonosConnection:self_running()
   local household_id = self.device:get_field(PlayerFields.HOUSEHOLD_ID)
   local player_id = self.device:get_field(PlayerFields.PLAYER_ID)
-  return Router.is_connected(utils.sonos_unique_key(household_id, player_id)) and self._initialized
+  local unique_key, bad_key_part = utils.sonos_unique_key(household_id, player_id)
+  if bad_key_part then
+    self.device.log.warn(
+      string.format(
+        "Bad Unique Key Part While Inspecting Connections in 'self_running': %s",
+        bad_key_part
+      )
+    )
+  end
+  return type(unique_key) == "string" and Router.is_connected(unique_key) and self._initialized
 end
 
 --- Whether or not the connection has a live websocket connection to its coordinator
 --- @return boolean
 function SonosConnection:coordinator_running()
   local household_id, coordinator_id = self.driver.sonos:get_coordinator_for_device(self.device)
-  return Router.is_connected(utils.sonos_unique_key(household_id, coordinator_id))
-    and self._initialized
+  local unique_key, bad_key_part = utils.sonos_unique_key(household_id, coordinator_id)
+  if bad_key_part then
+    self.device.log.warn(
+      string.format(
+        "Bad Unique Key Part While Inspecting Connections in 'coordinator_running': %s",
+        bad_key_part
+      )
+    )
+  end
+  return unique_key and Router.is_connected(unique_key) and self._initialized
 end
 
 function SonosConnection:refresh_subscriptions(maybe_reply_tx)
@@ -524,10 +589,12 @@ function SonosConnection:send_command(cmd)
   if err or not json_payload then
     log.error("Json encoding error: " .. err)
   else
-    Router.send_message_to_player(
-      utils.sonos_unique_key(household_id, coordinator_id),
-      json_payload
-    )
+    local unique_key, bad_key_part = utils.sonos_unique_key(household_id, coordinator_id)
+    if not unique_key then
+      self.device.log.error(string.format("Invalid Sonos Unique Key Part: %s", bad_key_part))
+      return
+    end
+    Router.send_message_to_player(unique_key, json_payload)
   end
 end
 
@@ -562,12 +629,17 @@ function SonosConnection:start()
       return false
     end
 
-    local listener_id, register_listener_err =
-      Router.register_listener_for_socket(self, utils.sonos_unique_key(household_id, player_id))
-    if register_listener_err ~= nil or not listener_id then
-      log.error(register_listener_err)
+    local unique_key, bad_key_part = utils.sonos_unique_key(household_id, player_id)
+    if bad_key_part then
+      self.device.log.error(string.format("Invalid Sonos Unique Key Part: %s", bad_key_part))
     else
-      self._self_listener_uuid = listener_id
+      local listener_id, register_listener_err =
+        Router.register_listener_for_socket(self, unique_key)
+      if register_listener_err ~= nil or not listener_id then
+        log.error(register_listener_err)
+      else
+        self._self_listener_uuid = listener_id
+      end
     end
   end
 
@@ -578,16 +650,39 @@ function SonosConnection:start()
 
   self:refresh_subscriptions()
   local coordinator_id = self.driver.sonos:get_coordinator_for_player(household_id, player_id)
+  local self_unique_key, self_bad_key_part = utils.sonos_unique_key(household_id, player_id)
+  local coordinator_unique_key, coordinator_bad_key_part =
+    utils.sonos_unique_key(household_id, coordinator_id)
   if
-    Router.is_connected(utils.sonos_unique_key(household_id, player_id))
-    and Router.is_connected(utils.sonos_unique_key(household_id, coordinator_id))
+    self_unique_key
+    and Router.is_connected(self_unique_key)
+    and coordinator_unique_key
+    and Router.is_connected(coordinator_unique_key)
   then
     self.device:online()
     self._initialized = true
     self._keepalive = true
     return true
   end
 
+  if self_bad_key_part then
+    self.device.log.error(
+      string.format(
+        "Invalid Unique Key Part for 'self' Player during connection bring-up: %s",
+        self_bad_key_part
+      )
+    )
+  end
+
+  if coordinator_bad_key_part then
+    self.device.log.error(
+      string.format(
+        "Invalid Unique Key Part for 'coordinator' Player during connection bring-up: %s",
+        coordinator_bad_key_part
+      )
+    )
+  end
+
   return false
 end
 
@@ -601,7 +696,12 @@ function SonosConnection:stop()
   local coordinator_id = self.driver.sonos:get_coordinator_for_group(household_id, group_id)
 
   if player_id ~= coordinator_id then
-    Router.close_socket_for_player(utils.sonos_unique_key(household_id, player_id))
+    local unique_key, bad_key_part = utils.sonos_unique_key(household_id, player_id)
+    if not unique_key then
+      self.device.log.error(string.format("Invalid Sonos Unique Key Part: %s", bad_key_part))
+      return
+    end
+    Router.close_socket_for_player(unique_key)
   end
 end
 
diff --git a/drivers/SmartThings/sonos/src/api/sonos_ssdp_discovery.lua b/drivers/SmartThings/sonos/src/api/sonos_ssdp_discovery.lua
@@ -200,16 +200,19 @@ function SonosPersistentSsdpTask:get_player_info(reply_tx, ...)
   local household_id_or_mac = select(1, ...)
   local player_id = select(2, ...)
 
-  local lookup_table, lookup_key
+  local lookup_table, lookup_key, bad_key_part
 
   if player_id ~= nil and type(player_id) == "string" then
     lookup_table = self.player_info_by_sonos_ids
-    lookup_key = utils.sonos_unique_key(household_id_or_mac, player_id)
+    lookup_key, bad_key_part = utils.sonos_unique_key(household_id_or_mac, player_id)
   else
     lookup_table = self.player_info_by_mac_addrs
     lookup_key = household_id_or_mac
   end
 
+  if not lookup_key and bad_key_part then
+    log.error(string.format("Invalid Unique Key Part: %s", bad_key_part))
+  end
   local maybe_existing = lookup_table[lookup_key]
   if maybe_existing and maybe_existing.ssdp_info.expires_at > os.time() then
     reply_tx:send(maybe_existing)
diff --git a/drivers/SmartThings/sonos/src/api/sonos_websocket_router.lua b/drivers/SmartThings/sonos/src/api/sonos_websocket_router.lua
@@ -1,7 +1,7 @@
-local log = require "log"
+local channel = require "cosock.channel"
 local cosock = require "cosock"
+local log = require "log"
 local socket = require "cosock.socket"
-local channel = require "cosock.channel"
 local ssl = require "cosock.ssl"
 local LustreConfig = require "lustre".Config
 local WebSocket = require "lustre".WebSocket
@@ -10,8 +10,8 @@ local CloseCode = require "lustre.frame.close".CloseCode
 local lb_utils = require "lunchbox.util"
 local st_utils = require "st.utils"
 
-local utils = require "utils"
 local SonosApi = require "api"
+local utils = require "utils"
 
 --- A "singleton" module that maintains all of the websockets for a
 --- home's Sonos players. A player as modeled in the driver will have
@@ -105,7 +105,13 @@ cosock.spawn(function()
               local wss = websockets[target]
               if wss == nil then
                 --TODO is this silencing a crash that is an indication of a state management bug in the run loop?
-                log.error(st_utils.stringify_table({ msg = msg }, "Coordinator doesn't exist for player", false))
+                log.error(
+                  st_utils.stringify_table(
+                    { msg = msg },
+                    "Coordinator doesn't exist for player",
+                    false
+                  )
+                )
                 goto continue
               end
 
@@ -123,7 +129,10 @@ cosock.spawn(function()
               if listener ~= nil then
                 if listener.device and listener.device.label then
                   log.debug(
-                    string.format("SonosConnection for device %s handling websocket message", listener.device.label)
+                    string.format(
+                      "SonosConnection for device %s handling websocket message",
+                      listener.device.label
+                    )
                   )
                 end
                 listener.on_message(uuid, msg)
@@ -153,7 +162,13 @@ local function _make_websocket(url_table, api_key)
     return nil, "Could not set TCP socket timeout: " .. sock_operation_err
   end
 
-  log.trace(string.format("Opening up websocket connection for host/port %s %s", url_table.host, url_table.port))
+  log.trace(
+    string.format(
+      "Opening up websocket connection for host/port %s %s",
+      url_table.host,
+      url_table.port
+    )
+  )
 
   _, sock_operation_err = sock:connect(url_table.host, url_table.port)
   if sock_operation_err ~= nil then
@@ -239,14 +254,16 @@ end
 --- @return boolean|nil success true on success, nil otherwise
 --- @return nil|string error the error message in the failure case
 function SonosWebSocketRouter.open_socket_for_player(household_id, player_id, wss_url, api_key)
-  local unique_key = utils.sonos_unique_key(household_id, player_id)
+  local unique_key, bad_key_part = utils.sonos_unique_key(household_id, player_id)
   if not websockets[unique_key] then
     log.debug("Opening websocket for player id " .. unique_key)
     local url_table = lb_utils.force_url_table(wss_url)
     local wss, err = _make_websocket(url_table, api_key)
 
     if err or not wss then
       return nil, string.format("Could not create websocket connection for %s: %s", unique_key, err)
+    elseif not unique_key then
+      return nil, string.format("Invalid Sonos Unique Key Part: %s", bad_key_part)
     else
       websockets[unique_key] = wss
       return true
@@ -307,8 +324,17 @@ function SonosWebSocketRouter.cleanup_unused_sockets(driver)
 
   for _, device in ipairs(known_devices) do
     local household_id, coordinator_id = driver.sonos:get_coordinator_for_device(device)
-    local coordinator_unique_key = utils.sonos_unique_key(household_id, coordinator_id)
-    if should_keep[coordinator_unique_key] == false then -- looking for false specifically, not nil
+    local coordinator_unique_key, bad_key_part =
+      utils.sonos_unique_key(household_id, coordinator_id)
+    if bad_key_part then
+      log.warn(
+        string.format(
+          "Invalid Sonos Unique Key Part while cleaning up unused websockets: %s",
+          bad_key_part
+        )
+      )
+    end
+    if coordinator_unique_key and should_keep[coordinator_unique_key] == false then -- looking for false specifically, not nil
       log.trace("Preserving coordinator socket " .. coordinator_id)
       should_keep[coordinator_unique_key] = true
     end
diff --git a/drivers/SmartThings/sonos/src/lifecycle_handlers.lua b/drivers/SmartThings/sonos/src/lifecycle_handlers.lua
@@ -71,7 +71,7 @@ function SonosDriverLifecycleHandlers.initialize_device(driver, device)
             local auth_success, api_key_or_err = driver:check_auth(info)
             if not auth_success then
               device:offline()
-              if auth_success == false then
+              if auth_success == false and api_version >= 14 then
                 local token_event_receive = driver:oauth_token_event_subscribe()
                 if not token_event_receive then
                   log.error("token event bus closed, aborting initialization")
diff --git a/drivers/SmartThings/sonos/src/sonos_driver.lua b/drivers/SmartThings/sonos/src/sonos_driver.lua
diff --git a/drivers/SmartThings/sonos/src/utils.lua b/drivers/SmartThings/sonos/src/utils.lua
