Merge pull request #2156 from SmartThingsCommunity/fix/additional-sonos-feedback-fixes

dljsjr · web-flow · commit 634161bfa039 · 2025-06-02T13:33:06.000-05:00
diff --git a/drivers/SmartThings/sonos/src/api/rest.lua b/drivers/SmartThings/sonos/src/api/rest.lua
@@ -1,7 +1,6 @@
+local json = require "st.json"
 local log = require "log"
-local net_url = require "net.url"
 local st_utils = require "st.utils"
-local json = require "st.json"
 
 local RestClient = require "lunchbox.rest"
 
@@ -13,11 +12,13 @@ local RestClient = require "lunchbox.rest"
 --- @return string|nil partial contents of partial read if successful
 local function process_rest_response(response, err, partial, err_callback)
   if err == nil and response == nil then
-    log.error(st_utils.stringify_table({
-      resp = response,
-      maybe_err = err,
-      maybe_partial = partial,
-    }, "[SonosRestApi] Unexpected nil for both response and error processing REST reply", false))
+    log.error(
+      st_utils.stringify_table({
+        resp = response,
+        maybe_err = err,
+        maybe_partial = partial,
+      }, "[SonosRestApi] Unexpected nil for both response and error processing REST reply", false)
+    )
   end
   if err ~= nil then
     if type(err_callback) == "function" then
@@ -71,70 +72,32 @@ end
 local SonosRestApi = {}
 
 --- Query a Sonos Group IP address for individual player info
----@param ip_or_url string|table
----@param ... unknown
+---@param url table a URL table created by `net_url`
+---@param headers table<string,string>?
 ---@return SonosDiscoveryInfo|SonosErrorResponse|nil
 ---@return string|nil error
----@overload fun(ip_or_url: table, headers: table<string,string>): SonosDiscoveryInfo?,string?
----@overload fun(ip_or_url: string, port: number, headers: table<string,string>): SonosDiscoveryInfo?,string?
-function SonosRestApi.get_player_info(ip_or_url, ...)
-  local url
-  local headers
-  if type(ip_or_url) == "table" then
-    headers = select(1, ...)
-    ip_or_url.path = "/api/v1/players/local/info"
-    url = ip_or_url
-  else
-    local port = select(1, ...)
-    headers = select(2, ...)
-    url = net_url.parse(string.format("https://%s:%s/api/v1/players/local/info", ip_or_url, port))
-  end
+function SonosRestApi.get_player_info(url, headers)
+  url.path = "/api/v1/players/local/info"
   return process_rest_response(RestClient.one_shot_get(url, headers))
 end
 
----@param ip_or_url string|table
----@param ... unknown
+---@param url table a URL table created by `net_url`
+---@param household HouseholdId
+---@param headers table<string,string>?
 ---@return SonosGroupsResponseBody|SonosErrorResponse|nil response
 ---@return nil|string error
----@overload fun(ip_or_url: table, household: HouseholdId, headers: table<string,string>?): SonosGroupsResponseBody?,string?
----@overload fun(ip_or_url: string, port: number, household: HouseholdId, headers: table<string,string>?): SonosGroupsResponseBody?,string?
-function SonosRestApi.get_groups_info(ip_or_url, ...)
-  local url
-  local headers
-  if type(ip_or_url) == "table" then
-    local household = select(1, ...)
-    headers = select(2, ...)
-    ip_or_url.path = string.format("/api/v1/households/%s/groups", household)
-    url = ip_or_url
-  else
-    local port = select(1, ...)
-    local household = select(2, ...)
-    headers = select(3, ...)
-    url = net_url.parse(string.format("https://%s:%s/api/v1/households/%s/groups", ip_or_url, port, household))
-  end
+function SonosRestApi.get_groups_info(url, household, headers)
+  url.path = string.format("/api/v1/households/%s/groups", household)
   return process_rest_response(RestClient.one_shot_get(url, headers))
 end
 
----@param ip_or_url string|table
----@param ... unknown
+---@param url table a URL table created by `net_url`
+---@param household HouseholdId
+---@param headers table<string,string>?
 ---@return SonosFavoritesResponseBody|SonosErrorResponse|nil response
 ---@return nil|string error
----@overload fun(ip_or_url: table, household: HouseholdId, headers: table<string,string>?): SonosFavoritesResponseBody?,string?
----@overload fun(ip_or_url: string, port: number, household: HouseholdId, headers: table<string,string>?): SonosFavoritesResponseBody?,string?
-function SonosRestApi.get_favorites(ip_or_url, ...)
-  local url
-  local headers
-  if type(ip_or_url) == "table" then
-    local household = select(1, ...)
-    headers = select(2, ...)
-    ip_or_url.path = string.format("/api/v1/households/%s/favorites", household)
-    url = ip_or_url
-  else
-    local port = select(1, ...)
-    local household = select(2, ...)
-    headers = select(3, ...)
-    url = net_url.parse(string.format("https://%s:%s/api/v1/households/%s/favorites", ip_or_url, port, household))
-  end
+function SonosRestApi.get_favorites(url, household, headers)
+  url.path = string.format("/api/v1/households/%s/favorites", household)
   return process_rest_response(RestClient.one_shot_get(url, headers))
 end
 
diff --git a/drivers/SmartThings/sonos/src/api/sonos_connection.lua b/drivers/SmartThings/sonos/src/api/sonos_connection.lua
@@ -410,9 +410,11 @@ function SonosConnection.new(driver, device)
             end
 
             local url_ip = lb_utils.force_url_table(coordinator_player.websocketUrl).host
-
+            local base_url = lb_utils.force_url_table(
+              string.format("https://%s:%s", url_ip, SonosApi.DEFAULT_SONOS_PORT)
+            )
             local favorites_response, err, _ =
-              SonosRestApi.get_favorites(url_ip, SonosApi.DEFAULT_SONOS_PORT, header.householdId)
+              SonosRestApi.get_favorites(base_url, header.householdId)
 
             if err or not favorites_response then
               log.error("Error querying for favorites: " .. err)
diff --git a/drivers/SmartThings/sonos/src/api/sonos_ssdp_discovery.lua b/drivers/SmartThings/sonos/src/api/sonos_ssdp_discovery.lua
@@ -1,5 +1,6 @@
 local cosock = require "cosock"
 local log = require "log"
+local net_url = require "net.url"
 local ssdp = require "ssdp"
 local st_utils = require "st.utils"
 
@@ -268,8 +269,12 @@ function sonos_ssdp.spawn_persistent_ssdp_task()
 
     if is_new_information then
       local headers = SonosApi.make_headers()
-      local discovery_info, err =
-        SonosApi.RestApi.get_player_info(sonos_ssdp_info.ip, SonosApi.DEFAULT_SONOS_PORT, headers)
+      local discovery_info, err = SonosApi.RestApi.get_player_info(
+        net_url.parse(
+          string.format("https://%s:%s", sonos_ssdp_info.ip, SonosApi.DEFAULT_SONOS_PORT)
+        ),
+        headers
+      )
       if not discovery_info then
         log.error(string.format("Error getting discovery info from SSDP response: %s", err))
       else
diff --git a/drivers/SmartThings/sonos/src/init.lua b/drivers/SmartThings/sonos/src/init.lua
@@ -1,3 +1,11 @@
+local api_version = require("version").api
+local cosock = require "cosock"
+
+if type(cosock.bus) == "nil" then
+  local cosock_bus = require "cosock.bus"
+  cosock.bus = cosock_bus
+end
+
 ---@module 'result'
 require "result" {
   register_globals = true,
@@ -22,6 +30,15 @@ if driver.datastore["dni_to_device_id"] ~= nil then
   driver.datastore["dni_to_device_id"] = nil
 end
 
+-- API Version 14 was the version that came out with 0.57.x
+--
+-- In API >= 14, the SSDP task will start when the driver receives the startup state.
+-- To support older versions of hub core, we start the SSDP task before the run loop,
+-- where we won't be using the startup state handling.
+if api_version < 14 then
+  driver:start_ssdp_event_task()
+end
+
 log.info "Starting Sonos run loop"
 driver:run()
 log.info "Exiting Sonos run loop"
diff --git a/drivers/SmartThings/sonos/src/lifecycle_handlers.lua b/drivers/SmartThings/sonos/src/lifecycle_handlers.lua
@@ -1,3 +1,4 @@
+local api_version = require("version").api
 local capabilities = require "st.capabilities"
 
 local cosock = require "cosock"
@@ -45,7 +46,7 @@ function SonosDriverLifecycleHandlers.initialize_device(driver, device)
     capabilities.mediaTrackControl.commands.previousTrack.NAME,
   }))
 
-  if not driver:has_received_startup_state() then
+  if api_version >= 14 and not driver:has_received_startup_state() then
     device.log.debug("Driver startup state not yet received, delaying initialization of device.")
     driver:queue_device_init_for_startup_state(device)
     return
@@ -76,8 +77,76 @@ function SonosDriverLifecycleHandlers.initialize_device(driver, device)
                   log.error("token event bus closed, aborting initialization")
                   return
                 end
-                driver:request_oauth_token()
-                token_event_receive:receive()
+                token_event_receive:settimeout(30)
+                local token, token_recv_err
+                -- max 30 mins
+                local backoff_builder = utils.backoff_builder(60 * 30, 30, 2)
+                if not driver:is_waiting_for_oauth_token() then
+                  local _, request_token_err = driver:request_oauth_token()
+                  if request_token_err then
+                    log.warn(string.format("Error sending token request: %s", request_token_err))
+                  end
+                end
+
+                local backoff_timer = nil
+                while not token do
+                  local send_request = false
+                  -- we use the backoff to create a timer and utilize a select loop here, instead of
+                  -- utilizing a sleep, so that we can create a long delay on our polling of the cloud
+                  -- without putting ourselves in a situation where we're sleeping for an extended period
+                  -- of time so that we don't sleep through the users's log-in attempt and fail to resume
+                  -- our connection attempts in a timely manner.
+                  --
+                  -- The backoff caps at 30 mins, as commented above
+                  if not backoff_timer then
+                    backoff_timer = cosock.timer.create_oneshot(backoff_builder())
+                  end
+                  local token_recv_ready, _, token_select_err =
+                    cosock.socket.select({ token_event_receive, backoff_timer }, nil, nil)
+
+                  if token_select_err then
+                    log.warn(string.format("select error: %s", token_select_err))
+                  end
+
+                  token, token_recv_err = nil, nil
+                  for _, receiver in pairs(token_recv_ready or {}) do
+                    if receiver == backoff_timer then
+                      -- we just make a note that the backoff has elapsed, rather than
+                      -- put a request in flight immediately.
+                      --
+                      -- This is just in case both receivers are ready, so that we can prioritize
+                      -- handling the token instead of putting another request in flight.
+                      send_request = true
+                      backoff_timer = nil
+                    end
+
+                    if receiver == token_event_receive then
+                      token, token_recv_err = token_event_receive:receive()
+                    end
+                  end
+
+                  if token_recv_err == "timeout" then
+                    log.debug("timeout waiting for OAuth token in reconnect task")
+                  elseif token_recv_err and not token then
+                    log.warn(
+                      string.format(
+                        "Unexpected error on token event receive bus: %s",
+                        token_recv_err
+                      )
+                    )
+                  end
+
+                  if send_request then
+                    if not driver:is_waiting_for_oauth_token() then
+                      local _, request_token_err = driver:request_oauth_token()
+                      if request_token_err then
+                        log.warn(
+                          string.format("Error sending token request: %s", request_token_err)
+                        )
+                      end
+                    end
+                  end
+                end
               else
                 device.log.error(
                   string.format(
diff --git a/drivers/SmartThings/sonos/src/sonos_driver.lua b/drivers/SmartThings/sonos/src/sonos_driver.lua
@@ -296,6 +296,7 @@ end
 ---@return { accessToken: string, expiresAt: number }? the token if a currently valid token is available, nil if not
 ---@return "token expired"|"no token"|nil reason the reason a token was not provided, nil if there is a valid token available
 function SonosDriver:get_oauth_token()
+  self.hub_augmented_driver_data = self.hub_augmented_driver_data or {}
   local decode_success, maybe_token =
     pcall(json.decode, self.hub_augmented_driver_data.sonosOAuthToken)
   if
@@ -316,7 +317,7 @@ function SonosDriver:get_oauth_token()
     if now < expiration then
       -- token is expiring soon, so we pre-emptively refresh
       if math.abs(expiration - now) < ONE_HOUR_IN_SECONDS then
-        local result, err = self:request_oauth_token()
+        local result, err = security.get_sonos_oauth()
         if not result then
           log.warn(string.format("Error requesting OAuth token via Security API: %s", err))
         end
diff --git a/drivers/SmartThings/sonos/src/ssdp.lua b/drivers/SmartThings/sonos/src/ssdp.lua
@@ -355,7 +355,7 @@ function _ssdp_mt:setwaker(kind, waker)
 
   assert(
     self.waker_ref == nil or waker == nil,
-    "Waker already set, cannot await SSDP serach instance from multiple locations."
+    "Waker already set, cannot await SSDP search instance from multiple locations."
   )
 
   self.waker_ref = waker

Original file line number	Diff line number	Diff line change
`@@ -355,7 +355,7 @@ function _ssdp_mt:setwaker(kind, waker)`
`355`	`355`
`356`	`356`	`assert(`
`357`	`357`	`self.waker_ref == nil or waker == nil,`
`358`		`- "Waker already set, cannot await SSDP serach instance from multiple locations."`
	`358`	`+ "Waker already set, cannot await SSDP search instance from multiple locations."`
`359`	`359`	`)`
`360`	`360`
`361`	`361`	`self.waker_ref = waker`