fix(sonos): Improved handling of bonded set membership changes.

We have historically ignored non-primary devices in bonded sets during
discovery/onboarding, going back to the pre-Edge days. 

Bonded sets are things like stereo pairs or home theater setups.

These devices cannot be controlled via the WSS LAN API at all, and they
aren't treated as being part of a Group in the Sonos group model; the
intent is for API consumers to treat the entire bonded set as a single
"player". Only one of the devices in the set exposes an API endpoint,
which differs from a Group where all the players expose an endpoint.
While groups service most commands via calling the API endpoint on the
Group Coordinator, certain commands like volume can be sent to
non-coordinator players. This is not the case with bonded sets; the
entire set always acts atomically, and only the primary device in the
set can service commands. Hence, we treat non-primary devices as not
controllable in a bonded set, and we don't onboard them.

However, we didn't really have very robust handling of devices that
become part of a bonded set at runtime after onboarding. This led to
unnecessary CPU and RAM usage by creating a few tight loops due to
unexpected failure/edge cases when making certain calls.

Because we would fail to discover this device under normal
circumstances, the preferred way to handle this transition is to mark
the device as being offline. We discussed emitting a delete for these
devices, but it seems that it wouldn't be too uncommon for some users to
create and destroy bonded sets ephemerally the way one might do with a
Group. For this reason we decided we would avoid deleting the records.

If a non-primary in a bonded set is removed from the bonded set, it will
be marked as online again.

Author: dljsjr

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

@@ -3,6 +3,8 @@ local log = require "log"
 
 local st_utils = require "st.utils"
 
+local PlayerFields = require "fields".SonosPlayerFields
+
 local CapEventHandlers = {}
 
 CapEventHandlers.PlaybackStatus = {
@@ -12,41 +14,48 @@ CapEventHandlers.PlaybackStatus = {
   Playing = "PLAYBACK_STATE_PLAYING",
 }
 
+local function _do_emit(device, attribute_event)
+  local bonded = device:get_field(PlayerFields.BONDED)
+  if not bonded then
+    device:emit_event(attribute_event)
+  end
+end
+
 function CapEventHandlers.handle_player_volume(device, new_volume, is_muted)
-  device:emit_event(capabilities.audioVolume.volume(new_volume))
+  _do_emit(device, capabilities.audioVolume.volume(new_volume))
   if is_muted then
-    device:emit_event(capabilities.audioMute.mute.muted())
+    _do_emit(device, capabilities.audioMute.mute.muted())
   else
-    device:emit_event(capabilities.audioMute.mute.unmuted())
+    _do_emit(device, capabilities.audioMute.mute.unmuted())
   end
 end
 
 function CapEventHandlers.handle_group_volume(device, new_volume, is_muted)
-  device:emit_event(capabilities.mediaGroup.groupVolume(new_volume))
+  _do_emit(device, capabilities.mediaGroup.groupVolume(new_volume))
   if is_muted then
-    device:emit_event(capabilities.mediaGroup.groupMute.muted())
+    _do_emit(device, capabilities.mediaGroup.groupMute.muted())
   else
-    device:emit_event(capabilities.mediaGroup.groupMute.unmuted())
+    _do_emit(device, capabilities.mediaGroup.groupMute.unmuted())
   end
 end
 
 function CapEventHandlers.handle_group_role_update(device, group_role)
-  device:emit_event(capabilities.mediaGroup.groupRole(group_role))
+  _do_emit(device, capabilities.mediaGroup.groupRole(group_role))
 end
 
 function CapEventHandlers.handle_group_coordinator_update(device, coordinator_id)
-  device:emit_event(capabilities.mediaGroup.groupPrimaryDeviceId(coordinator_id))
+  _do_emit(device, capabilities.mediaGroup.groupPrimaryDeviceId(coordinator_id))
 end
 
 function CapEventHandlers.handle_group_id_update(device, group_id)
-  device:emit_event(capabilities.mediaGroup.groupId(group_id))
+  _do_emit(device, capabilities.mediaGroup.groupId(group_id))
 end
 
 function CapEventHandlers.handle_group_update(device, group_info)
   local groupRole, groupPrimaryDeviceId, groupId = table.unpack(group_info)
-  device:emit_event(capabilities.mediaGroup.groupRole(groupRole))
-  device:emit_event(capabilities.mediaGroup.groupPrimaryDeviceId(groupPrimaryDeviceId))
-  device:emit_event(capabilities.mediaGroup.groupId(groupId))
+  _do_emit(device, capabilities.mediaGroup.groupRole(groupRole))
+  _do_emit(device, capabilities.mediaGroup.groupPrimaryDeviceId(groupPrimaryDeviceId))
+  _do_emit(device, capabilities.mediaGroup.groupId(groupId))
 end
 
 function CapEventHandlers.handle_audio_clip_status(device, clips)
@@ -61,11 +70,11 @@ end
 
 function CapEventHandlers.handle_playback_status(device, playback_state)
   if playback_state == CapEventHandlers.PlaybackStatus.Playing then
-    device:emit_event(capabilities.mediaPlayback.playbackStatus.playing())
+    _do_emit(device, capabilities.mediaPlayback.playbackStatus.playing())
   elseif playback_state == CapEventHandlers.PlaybackStatus.Idle then
-    device:emit_event(capabilities.mediaPlayback.playbackStatus.stopped())
+    _do_emit(device, capabilities.mediaPlayback.playbackStatus.stopped())
   elseif playback_state == CapEventHandlers.PlaybackStatus.Paused then
-    device:emit_event(capabilities.mediaPlayback.playbackStatus.paused())
+    _do_emit(device, capabilities.mediaPlayback.playbackStatus.paused())
   elseif playback_state == CapEventHandlers.PlaybackStatus.Buffering then
     -- TODO the DTH doesn't currently do anything w/ buffering;
     -- might be worth figuring out what to do with this in the future.
@@ -74,7 +83,7 @@ function CapEventHandlers.handle_playback_status(device, playback_state)
 end
 
 function CapEventHandlers.update_favorites(device, new_favorites)
-  device:emit_event(capabilities.mediaPresets.presets(new_favorites))
+  _do_emit(device, capabilities.mediaPresets.presets(new_favorites))
 end
 
 function CapEventHandlers.handle_playback_metadata_update(device, metadata_status_body)
@@ -128,7 +137,7 @@ function CapEventHandlers.handle_playback_metadata_update(device, metadata_statu
   end
 
   if type(audio_track_data.title) == "string" then
-    device:emit_event(capabilities.audioTrackData.audioTrackData(audio_track_data))
+    _do_emit(device, capabilities.audioTrackData.audioTrackData(audio_track_data))
   end
 end
 

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

@@ -166,8 +166,12 @@ local function _open_coordinator_socket(sonos_conn, household_id, self_player_id
       return
     end
 
-    _, err =
-      Router.open_socket_for_player(household_id, coordinator_id, coordinator.websocketUrl, api_key)
+    _, err = Router.open_socket_for_player(
+      household_id,
+      coordinator_id,
+      coordinator.player.websocketUrl,
+      api_key
+    )
     if err ~= nil then
       log.error(
         string.format(
@@ -302,10 +306,13 @@ end
 --- @return SonosConnection
 function SonosConnection.new(driver, device)
   log.debug(string.format("Creating new SonosConnection for %s", device.label))
-  local self = setmetatable(
-    { driver = driver, device = device, _listener_uuids = {}, _initialized = false, _reconnecting = false },
-    SonosConnection
-  )
+  local self = setmetatable({
+    driver = driver,
+    device = device,
+    _listener_uuids = {},
+    _initialized = false,
+    _reconnecting = false,
+  }, SonosConnection)
 
   -- capture the label here in case something goes wonky like a callback being fired after a
   -- device is removed
@@ -358,19 +365,28 @@ function SonosConnection.new(driver, device)
         local household_id, current_coordinator =
           self.driver.sonos:get_coordinator_for_device(self.device)
         local _, player_id = self.driver.sonos:get_player_for_device(self.device)
-        self.driver.sonos:update_household_info(header.householdId, body, self.device)
+        self.driver.sonos:update_household_info(header.householdId, body, self.driver)
         self.driver.sonos:update_device_record_from_state(header.householdId, self.device)
         local _, updated_coordinator = self.driver.sonos:get_coordinator_for_device(self.device)
 
+        local bonded = self.device:get_field(PlayerFields.BONDED)
+        if bonded then
+          self:stop()
+        end
+
         Router.cleanup_unused_sockets(self.driver)
 
-        if not self:coordinator_running() then
-          --TODO this is not infallible
-          _open_coordinator_socket(self, household_id, player_id)
-        end
+        if not bonded then
+          if not self:coordinator_running() then
+            --TODO this is not infallible
+            _open_coordinator_socket(self, household_id, player_id)
+          end
 
-        if current_coordinator ~= updated_coordinator then
-          self:refresh_subscriptions()
+          if current_coordinator ~= updated_coordinator then
+            self:refresh_subscriptions()
+          end
+        else
+          self.device:offline()
         end
       elseif header.type == "playerVolume" then
         log.trace(string.format("PlayerVolume type message for %s", device_name))
@@ -477,7 +493,7 @@ function SonosConnection.new(driver, device)
               return
             end
 
-            local url_ip = lb_utils.force_url_table(coordinator_player.websocketUrl).host
+            local url_ip = lb_utils.force_url_table(coordinator_player.player.websocketUrl).host
             local base_url = lb_utils.force_url_table(
               string.format("https://%s:%s", url_ip, SonosApi.DEFAULT_SONOS_PORT)
             )
@@ -590,7 +606,7 @@ function SonosConnection:coordinator_running()
       )
     )
   end
-  return type(unique_key) == "string" and Router.is_connected(unique_key) and self._initialized
+  return type(unique_key) == "string" and Router.is_connected(unique_key)
 end
 
 function SonosConnection:refresh_subscriptions(maybe_reply_tx)

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

@@ -300,7 +300,7 @@ function sonos_ssdp.spawn_persistent_ssdp_task()
     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 = true },
+          { hub_logs = false },
           st_utils.stringify_table(info_to_send, "Sonos Discovery Info has unexpected structure")
         )
         return

drivers/SmartThings/sonos/src/fields.lua

@@ -5,6 +5,7 @@ local Fields = {}
 Fields.SonosPlayerFields = {
   _IS_INIT = "init",
   _IS_SCANNING = "scanning",
+  BONDED = "bonded",
   CONNECTION = "conn",
   UNIQUE_KEY = "unique_key",
   HOUSEHOLD_ID = "householdId",

drivers/SmartThings/sonos/src/init.lua

@@ -39,6 +39,6 @@ if api_version < 14 then
   driver:start_ssdp_event_task()
 end
 
-log.info "Starting Sonos run loop"
+log.info("Starting Sonos run loop")
 driver:run()
-log.info "Exiting Sonos run loop"
+log.info("Exiting Sonos run loop")

drivers/SmartThings/sonos/src/lifecycle_handlers.lua

@@ -164,10 +164,12 @@ function SonosDriverLifecycleHandlers.initialize_device(driver, device)
                   return
                 end
                 log.error_with(
-                  { hub_logs = true },
-                  "Error handling Sonos player initialization: %s, error code: %s",
-                  error,
-                  (error_code or "N/A")
+                  { hub_logs = false },
+                  string.format(
+                    "Error handling Sonos player initialization: %s, error code: %s",
+                    error,
+                    (error_code or "N/A")
+                  )
                 )
               end
             end

drivers/SmartThings/sonos/src/sonos_driver.lua

@@ -42,11 +42,27 @@ local ONE_HOUR_IN_SECONDS = 3600
 ---@field private waiting_for_oauth_token boolean
 ---@field private startup_state_received boolean
 ---@field private devices_waiting_for_startup_state SonosDevice[]
+---@field package bonded_devices table<string, boolean> map of Device device_network_id to a boolean indicating if the device is currently known as a bonded device.
 ---
 ---@field public ssdp_task SonosPersistentSsdpTask?
 ---@field private ssdp_event_thread_handle table?
 local SonosDriver = {}
 
+---@param device SonosDevice
+function SonosDriver:update_bonded_device_tracking(device)
+  local already_bonded = self.bonded_devices[device.device_network_id]
+  local currently_bonded = device:get_field(PlayerFields.BONDED)
+  self.bonded_devices[device.device_network_id] = currently_bonded
+
+  if currently_bonded and not already_bonded then
+    device:offline()
+  end
+
+  if already_bonded and not currently_bonded then
+    SonosDriverLifecycleHandlers.initialize_device(self, device)
+  end
+end
+
 function SonosDriver:has_received_startup_state()
   return self.startup_state_received
 end
@@ -127,13 +143,13 @@ end
 function SonosDriver:handle_augmented_store_delete(update_key)
   if update_key == "endpointAppInfo" then
     if update_key == "endpointAppInfo" then
-      log.trace "deleting endpoint app info"
+      log.trace("deleting endpoint app info")
       self.oauth.endpoint_app_info = nil
     elseif update_key == "sonosOAuthToken" then
-      log.trace "deleting OAuth Token"
+      log.trace("deleting OAuth Token")
       self.oauth.token = nil
     elseif update_key == "force_oauth" then
-      log.trace "deleting Force OAuth"
+      log.trace("deleting Force OAuth")
       self.oauth.force_oauth = nil
     else
       log.debug(string.format("received delete of unexpected key: %s", update_key))
@@ -423,9 +439,15 @@ local function make_ssdp_event_handler(
             local event, recv_err = discovery_event_subscription:receive()
 
             if event then
+              local mac_addr = utils.extract_mac_addr(event.discovery_info.device)
               local unique_key = utils.sonos_unique_key_from_ssdp(event.ssdp_info)
               if
-                event.force_refresh or not (unauthorized[unique_key] or discovered[unique_key])
+                event.force_refresh
+                or not (
+                  unauthorized[unique_key]
+                  or discovered[unique_key]
+                  or driver.bonded_devices[mac_addr]
+                )
               then
                 local _, api_key = driver:check_auth(event)
                 local success, handle_err, err_code =
@@ -435,7 +457,7 @@ local function make_ssdp_event_handler(
                     unauthorized[unique_key] = event
                   end
                   log.warn_with(
-                    { hub_logs = true },
+                    { hub_logs = false },
                     string.format("Failed to handle discovered speaker: %s", handle_err)
                   )
                 else
@@ -483,12 +505,22 @@ function SonosDriver:handle_player_discovery_info(api_key, info, device)
   -- speaker in a bonded set (e.g. a home theater system, a stereo pair, etc).
   -- These aren't the same as speaker groups, and bonded speakers can't be controlled
   -- via websocket at all. So we ignore all bonded non-primary speakers
-  if #info.ssdp_info.group_id == 0 then
-    return nil,
-      string.format(
-        "Player %s is a non-primary bonded Sonos device, ignoring",
-        info.discovery_info.device.name
-      )
+  -- if  then
+  --   return nil,
+  --     string.format(
+  --       "Player %s is a non-primary bonded Sonos device, ignoring",
+  --       info.discovery_info.device.name
+  --     )
+  -- end
+
+  local discovery_info_mac_addr = utils.extract_mac_addr(info.discovery_info.device)
+  local bonded = (#info.ssdp_info.group_id == 0)
+  self.bonded_devices[discovery_info_mac_addr] = bonded
+
+  local maybe_device = self:get_device_by_dni(discovery_info_mac_addr)
+  if maybe_device then
+    maybe_device:set_field(PlayerFields.BONDED, bonded, { persist = false })
+    self:update_bonded_device_tracking(maybe_device)
   end
 
   api_key = api_key or self:get_fallback_api_key()
@@ -543,7 +575,7 @@ function SonosDriver:handle_player_discovery_info(api_key, info, device)
   end
 
   --- @cast response SonosGroupsResponseBody
-  self.sonos:update_household_info(info.ssdp_info.household_id, response)
+  self.sonos:update_household_info(info.ssdp_info.household_id, response, self)
 
   local device_to_update, device_mac_addr
 
@@ -565,7 +597,7 @@ function SonosDriver:handle_player_discovery_info(api_key, info, device)
     if not (info and info.discovery_info and info.discovery_info.device) then
       return nil, st_utils.stringify_table(info, "Sonos Discovery Info has unexpected structure")
     end
-    device_mac_addr = utils.extract_mac_addr(info.discovery_info.device)
+    device_mac_addr = discovery_info_mac_addr
   end
 
   if not device_to_update then
@@ -578,7 +610,7 @@ function SonosDriver:handle_player_discovery_info(api_key, info, device)
   if device_to_update then
     self.dni_to_device_id[device_mac_addr] = device_to_update.id
     self.sonos:associate_device_record(device_to_update, info)
-  else
+  elseif not bonded then
     local name = info.discovery_info.device.name
       or info.discovery_info.device.modelDisplayName
       or "Unknown Sonos Player"
@@ -631,6 +663,7 @@ function SonosDriver.new_driver_template()
     waiting_for_oauth_token = false,
     startup_state_received = false,
     devices_waiting_for_startup_state = {},
+    bonded_devices = utils.new_mac_address_keyed_table(),
     dni_to_device_id = utils.new_mac_address_keyed_table(),
     lifecycle_handlers = SonosDriverLifecycleHandlers,
     capability_handlers = {