Refactor player load logic

pyheos/connection.py

@@ -284,9 +284,8 @@ async def _handle_event(self, response: HeosResponse):
         elif response.command in const.HEOS_EVENTS:
             # pylint: disable=protected-access
             result = await self._heos._handle_event(response)
-            if result:
-                self._heos.dispatcher.send(
-                    const.SIGNAL_CONTROLLER_EVENT, response.command)
+            self._heos.dispatcher.send(
+                const.SIGNAL_CONTROLLER_EVENT, response.command, result)
             _LOGGER.debug("Event received: %s", response)
         else:
             _LOGGER.debug("Unrecognized event: %s", response)

pyheos/const.py

@@ -13,6 +13,9 @@
 STATE_DISCONNECTED = "disconnected"
 STATE_RECONNECTING = "reconnecting"
 
+DATA_NEW = "new"
+DATA_MAPPED_IDS = "mapped_ids"
+
 PLAY_STATE_PLAY = 'play'
 PLAY_STATE_PAUSE = 'pause'
 PLAY_STATE_STOP = 'stop'

pyheos/heos.py

@@ -7,7 +7,8 @@
 from .connection import HeosConnection
 from .dispatch import Dispatcher
 from .group import HeosGroup, create_group
-from .player import HeosPlayer
+from .player import (
+    HeosPlayer, parse_player_id, parse_player_name, parse_player_version)
 from .response import HeosResponse
 from .source import HeosSource, InputSource
 
@@ -47,12 +48,12 @@ async def disconnect(self):
         """Disconnect from the CLI."""
         await self._connection.disconnect()
 
-    async def _handle_event(self, event: HeosResponse) -> bool:
+    async def _handle_event(self, event: HeosResponse):
         """Handle a heos event."""
         if event.command == const.EVENT_PLAYERS_CHANGED \
                 and self._players_loaded:
-            await self.get_players(refresh=True)
-        elif event.command == const.EVENT_SOURCES_CHANGED \
+            return await self.load_players()
+        if event.command == const.EVENT_SOURCES_CHANGED \
                 and self._music_sources_loaded:
             await self.get_music_sources(refresh=True)
         elif event.command == const.EVENT_USER_CHANGED:
@@ -71,31 +72,55 @@ async def sign_out(self):
         """Sign-out of the HEOS account on the device directly connected."""
         await self._connection.commands.sign_out()
 
+    async def load_players(self):
+        """Refresh the players."""
+        changes = {
+            const.DATA_NEW: [],
+            const.DATA_MAPPED_IDS: {}
+        }
+        players = {}
+        payload = await self._connection.commands.get_players()
+        existing = list(self._players.values())
+        for player_data in payload:
+            player_id = parse_player_id(player_data)
+            name = parse_player_name(player_data)
+            version = parse_player_version(player_data)
+            # Try finding existing player by id or match name when firmware
+            # verion is different because IDs change after a firmware upgrade
+            player = next((
+                player for player in existing
+                if player.player_id == player_id
+                or (player.name == name and player.version != version)), None)
+            if player:
+                # Existing player matched - update
+                if player.player_id != player_id:
+                    changes[const.DATA_MAPPED_IDS][player_id] = \
+                        player.player_id
+                player.from_data(player_data)
+                players[player_id] = player
+                existing.remove(player)
+            else:
+                # New player
+                player = HeosPlayer(self, player_data)
+                changes[const.DATA_NEW].append(player_id)
+                players[player_id] = player
+        # For any item remaining in existing, mark unavailalbe, add to updated
+        for player in existing:
+            player.set_available(False)
+            players[player.player_id] = player
+
+        # Update all statuses
+        await asyncio.gather(*[player.refresh() for player in
+                               players.values() if player.available])
+        self._players = players
+        self._players_loaded = True
+        return changes
+
     async def get_players(self, *, refresh=False) -> Dict[int, HeosPlayer]:
         """Get available players."""
         # get players and pull initial state
-        if not self._players or refresh:
-            payload = await self._connection.commands.get_players()
-            players = {}
-            player_data = {}
-            for data in payload:
-                player = HeosPlayer(self, data)
-                players[player.player_id] = player
-                player_data[player.player_id] = data
-            # Match to existing
-            for player_id, player in self._players.items():
-                if player_id in players:
-                    players.pop(player_id)
-                    player.set_available(True)
-                    player.from_data(player_data[player_id])
-                else:
-                    player.set_available(False)
-
-            self._players.update(players)
-            # Update all statuses
-            await asyncio.gather(*[player.refresh() for player in
-                                   self._players.values() if player.available])
-            self._players_loaded = True
+        if not self._players_loaded or refresh:
+            await self.load_players()
         return self._players
 
     async def get_groups(self, *, refresh=False) -> Dict[int, HeosGroup]:

pyheos/player.py

@@ -8,6 +8,21 @@
 from .source import HeosSource, InputSource
 
 
+def parse_player_id(data: dict) -> int:
+    """Parse the player ID from the data payload."""
+    return int(data['pid'])
+
+
+def parse_player_name(data: dict) -> str:
+    """Parse the player name from the data payload."""
+    return data['name']
+
+
+def parse_player_version(data: dict) -> str:
+    """Parse the player version from the data payload."""
+    return data.get('version')
+
+
 class HeosNowPlayingMedia:
     """Define now playing media information."""
 
@@ -173,13 +188,14 @@ def __repr__(self):
 
     def from_data(self, data: dict):
         """Update the attributes from the supplied data."""
-        self._name = data['name']
-        self._player_id = int(data['pid'])
+        self._name = parse_player_name(data)
+        self._player_id = parse_player_id(data)
         self._model = data['model']
-        self._version = data.get('version')
+        self._version = parse_player_version(data)
         self._ip_address = data['ip']
         self._network = data['network']
         self._line_out = int(data['lineout'])
+        self._available = True
 
     def set_available(self, available):
         """Mark player removed after a change event."""

tests/fixtures/player.get_players_firmware_update.json

@@ -0,0 +1,25 @@
+{
+	"heos": {
+		"command": "player/get_players",
+		"result": "success",
+		"message": ""
+	},
+	"payload": [{
+			"name": "Back Patio",
+			"pid": 101,
+			"model": "HEOS Drive",
+			"version": "1.500.000",
+			"ip": "192.168.0.1",
+			"network": "wired",
+			"lineout": 1
+		}, {
+			"name": "Front Porch",
+			"pid": 102,
+			"model": "HEOS Drive",
+			"version": "1.500.000",
+			"ip": "192.168.0.2",
+			"network": "wireless",
+			"lineout": 1
+		}
+	]
+}

tests/test_heos.py

@@ -500,8 +500,12 @@ async def test_players_changed_event(mock_device, heos):
     # Attach dispatch handler
     signal = asyncio.Event()
 
-    async def handler(event: str):
+    async def handler(event: str, data):
         assert event == const.EVENT_PLAYERS_CHANGED
+        assert data == {
+            const.DATA_NEW: [3],
+            const.DATA_MAPPED_IDS: {}
+        }
         signal.set()
     heos.dispatcher.connect(const.SIGNAL_CONTROLLER_EVENT, handler)
 
@@ -523,6 +527,41 @@ async def handler(event: str):
     assert heos.players.get(1).name == 'Backyard'
 
 
+@pytest.mark.asyncio
+async def test_players_changed_event_new_ids(mock_device, heos):
+    """Test players are resynced when event received after firmware update."""
+    # assert not playing
+    old_players = (await heos.get_players()).copy()
+    # Attach dispatch handler
+    signal = asyncio.Event()
+
+    async def handler(event: str, data):
+        assert event == const.EVENT_PLAYERS_CHANGED
+        assert data == {
+            const.DATA_NEW: [],
+            const.DATA_MAPPED_IDS: {
+                101: 1,
+                102: 2
+            }
+        }
+        signal.set()
+    heos.dispatcher.connect(const.SIGNAL_CONTROLLER_EVENT, handler)
+
+    # Write event through mock device
+    mock_device.register(const.COMMAND_GET_PLAYERS, None,
+                         'player.get_players_firmware_update', replace=True)
+    await mock_device.write_event(await get_fixture("event.players_changed"))
+    await signal.wait()
+    # Assert players are the same as before just updated.
+    assert len(heos.players) == 2
+    assert heos.players[101] == old_players[1]
+    assert heos.players[101].available
+    assert heos.players[101].name == "Back Patio"
+    assert heos.players[102] == old_players[2]
+    assert heos.players[102].available
+    assert heos.players[102].name == "Front Porch"
+
+
 @pytest.mark.asyncio
 async def test_sources_changed_event(mock_device, heos):
     """Test sources changed fires dispatcher."""
@@ -531,7 +570,7 @@ async def test_sources_changed_event(mock_device, heos):
     await heos.get_music_sources()
     signal = asyncio.Event()
 
-    async def handler(event: str):
+    async def handler(event: str, data):
         assert event == const.EVENT_SOURCES_CHANGED
         signal.set()
     heos.dispatcher.connect(const.SIGNAL_CONTROLLER_EVENT, handler)
@@ -554,7 +593,7 @@ async def test_groups_changed_event(mock_device, heos):
     assert len(groups) == 1
     signal = asyncio.Event()
 
-    async def handler(event: str):
+    async def handler(event: str, data):
         assert event == const.EVENT_GROUPS_CHANGED
         signal.set()
     heos.dispatcher.connect(const.SIGNAL_CONTROLLER_EVENT, handler)
@@ -642,7 +681,7 @@ async def test_user_changed_event(mock_device, heos):
     """Test user changed fires dispatcher and updates logged in user."""
     signal = asyncio.Event()
 
-    async def handler(event: str):
+    async def handler(event: str, data):
         assert event == const.EVENT_USER_CHANGED
         signal.set()
     heos.dispatcher.connect(const.SIGNAL_CONTROLLER_EVENT, handler)