From 68d3e624e651b08e22bb55ce5347191030959aae Mon Sep 17 00:00:00 2001 From: ktnrg45 <38207570+ktnrg45@users.noreply.github.com> Date: Tue, 23 Apr 2019 16:32:36 -0700 Subject: [PATCH] Fix ps4 not able to use different PSN accounts (#22799) * Remove skipping of creds step. * Check for device added per account * typo * lint * Pylint * Fix test * Fix test * Typo * Add auto location * blank space * Add new identifier handling + fix select source * Add cred_timeout error * add credential timeout error * Fix Tests * patch decorator * Update test_config_flow.py * add test * Revert * Rename vars * fix tests * Add attr location * Bump 0.6.0 * Bump 0.6.0 * Bump 0.6.0 * Update handling exception * Update remove method * Update tests * Refactoring * Pylint * revert * chmod * 0.6.1 * 0.6.1 * 0.6.1 * Remove func * Add migration * Version 3 * Remove redefinition * Add format unique id * Add format unique id * pylint * pylint * 0.7.1 * 0.7.1 * 0.7.1 * Changes with media_art call * Add library exception * 0.7.2 * 0.7.2 * 0.7.2 * Version and entry_version update * Revert list comprehension * Corrected exception handling * Update media_player.py * Update media_player.py * white space --- CODEOWNERS | 1 + homeassistant/components/ps4/__init__.py | 51 ++++++++++++-- homeassistant/components/ps4/config_flow.py | 70 ++++++++++++-------- homeassistant/components/ps4/manifest.json | 6 +- homeassistant/components/ps4/media_player.py | 39 ++++++++--- homeassistant/components/ps4/strings.json | 1 + requirements_all.txt | 2 +- requirements_test_all.txt | 2 +- tests/components/ps4/test_config_flow.py | 32 +++++++++ 9 files changed, 157 insertions(+), 47 deletions(-) diff --git a/CODEOWNERS b/CODEOWNERS index c2cd1f4553a..b96aae298a5 100644 --- a/CODEOWNERS +++ b/CODEOWNERS @@ -170,6 +170,7 @@ homeassistant/components/pi_hole/* @fabaff homeassistant/components/plant/* @ChristianKuehnel homeassistant/components/point/* @fredrike homeassistant/components/pollen/* @bachya +homeassistant/components/ps4/* @ktnrg45 homeassistant/components/push/* @dgomes homeassistant/components/pvoutput/* @fabaff homeassistant/components/qnap/* @colinodell diff --git a/homeassistant/components/ps4/__init__.py b/homeassistant/components/ps4/__init__.py index 22c21fcffbe..16c09d7ce2d 100644 --- a/homeassistant/components/ps4/__init__.py +++ b/homeassistant/components/ps4/__init__.py @@ -1,7 +1,9 @@ """Support for PlayStation 4 consoles.""" import logging -from homeassistant.const import CONF_REGION +from homeassistant.core import split_entity_id +from homeassistant.const import CONF_REGION, CONF_TOKEN +from homeassistant.helpers import entity_registry from homeassistant.util import location from .config_flow import PlayStation4FlowHandler # noqa: pylint: disable=unused-import @@ -37,9 +39,14 @@ async def async_migrate_entry(hass, entry): data = entry.data version = entry.version - reason = {1: "Region codes have changed"} # From 0.89 + _LOGGER.debug("Migrating PS4 entry from Version %s", version) - # Migrate Version 1 -> Version 2 + reason = { + 1: "Region codes have changed", + 2: "Format for Unique ID for entity registry has changed" + } + + # Migrate Version 1 -> Version 2: New region codes. if version == 1: loc = await hass.async_add_executor_job(location.detect_location_info) if loc: @@ -47,11 +54,41 @@ async def async_migrate_entry(hass, entry): if country in COUNTRIES: for device in data['devices']: device[CONF_REGION] = country - entry.version = 2 + version = entry.version = 2 config_entries.async_update_entry(entry, data=data) _LOGGER.info( "PlayStation 4 Config Updated: \ Region changed to: %s", country) + + # Migrate Version 2 -> Version 3: Update identifier format. + if version == 2: + # Prevent changing entity_id. Updates entity registry. + registry = await entity_registry.async_get_registry(hass) + + for entity_id, e_entry in registry.entities.items(): + if e_entry.config_entry_id == entry.entry_id: + unique_id = e_entry.unique_id + + # Remove old entity entry. + registry.async_remove(entity_id) + await hass.async_block_till_done() + + # Format old unique_id. + unique_id = format_unique_id(entry.data[CONF_TOKEN], unique_id) + + # Create new entry with old entity_id. + new_id = split_entity_id(entity_id)[1] + registry.async_get_or_create( + 'media_player', DOMAIN, unique_id, + suggested_object_id=new_id, + config_entry_id=e_entry.config_entry_id, + device_id=e_entry.device_id + ) + entry.version = 3 + _LOGGER.info( + "PlayStation 4 identifier for entity: %s \ + has changed", entity_id) + config_entries.async_update_entry(entry) return True msg = """{} for the PlayStation 4 Integration. @@ -64,3 +101,9 @@ async def async_migrate_entry(hass, entry): notification_id='config_entry_migration' ) return False + + +def format_unique_id(creds, mac_address): + """Use last 4 Chars of credential as suffix. Unique ID per PSN user.""" + suffix = creds[-4:] + return "{}_{}".format(mac_address, suffix) diff --git a/homeassistant/components/ps4/config_flow.py b/homeassistant/components/ps4/config_flow.py index 1b184a3774f..ff028682739 100644 --- a/homeassistant/components/ps4/config_flow.py +++ b/homeassistant/components/ps4/config_flow.py @@ -7,6 +7,7 @@ import voluptuous as vol from homeassistant import config_entries from homeassistant.const import ( CONF_CODE, CONF_HOST, CONF_IP_ADDRESS, CONF_NAME, CONF_REGION, CONF_TOKEN) +from homeassistant.util import location from .const import DEFAULT_NAME, DOMAIN @@ -25,7 +26,7 @@ PORT_MSG = {UDP_PORT: 'port_987_bind_error', TCP_PORT: 'port_997_bind_error'} class PlayStation4FlowHandler(config_entries.ConfigFlow): """Handle a PlayStation 4 config flow.""" - VERSION = 2 + VERSION = 3 CONNECTION_CLASS = config_entries.CONN_CLASS_LOCAL_POLL def __init__(self): @@ -39,6 +40,7 @@ class PlayStation4FlowHandler(config_entries.ConfigFlow): self.region = None self.pin = None self.m_device = None + self.location = None self.device_list = [] async def async_step_user(self, user_input=None): @@ -50,23 +52,25 @@ class PlayStation4FlowHandler(config_entries.ConfigFlow): if failed in ports: reason = PORT_MSG[failed] return self.async_abort(reason=reason) - # Skip Creds Step if a device is configured. - if self.hass.config_entries.async_entries(DOMAIN): - return await self.async_step_mode() return await self.async_step_creds() async def async_step_creds(self, user_input=None): """Return PS4 credentials from 2nd Screen App.""" + from pyps4_homeassistant.errors import CredentialTimeout + errors = {} if user_input is not None: - self.creds = await self.hass.async_add_executor_job( - self.helper.get_creds) - - if self.creds is not None: - return await self.async_step_mode() - return self.async_abort(reason='credential_error') + try: + self.creds = await self.hass.async_add_executor_job( + self.helper.get_creds) + if self.creds is not None: + return await self.async_step_mode() + return self.async_abort(reason='credential_error') + except CredentialTimeout: + errors['base'] = 'credential_timeout' return self.async_show_form( - step_id='creds') + step_id='creds', + errors=errors) async def async_step_mode(self, user_input=None): """Prompt for mode.""" @@ -99,6 +103,7 @@ class PlayStation4FlowHandler(config_entries.ConfigFlow): """Prompt user input. Create or edit entry.""" from pyps4_homeassistant.media_art import COUNTRIES regions = sorted(COUNTRIES.keys()) + default_region = None errors = {} if user_input is None: @@ -112,26 +117,23 @@ class PlayStation4FlowHandler(config_entries.ConfigFlow): self.device_list = [device['host-ip'] for device in devices] - # If entry exists check that devices found aren't configured. - if self.hass.config_entries.async_entries(DOMAIN): - creds = {} - for entry in self.hass.config_entries.async_entries(DOMAIN): - # Retrieve creds from entry - creds['data'] = entry.data[CONF_TOKEN] - # Retrieve device data from entry - conf_devices = entry.data['devices'] - for c_device in conf_devices: - if c_device['host'] in self.device_list: - # Remove configured device from search list. - self.device_list.remove(c_device['host']) + # Check that devices found aren't configured per account. + entries = self.hass.config_entries.async_entries(DOMAIN) + if entries: + # Retrieve device data from all entries if creds match. + conf_devices = [device for entry in entries + if self.creds == entry.data[CONF_TOKEN] + for device in entry.data['devices']] + + # Remove configured device from search list. + for c_device in conf_devices: + if c_device['host'] in self.device_list: + # Remove configured device from search list. + self.device_list.remove(c_device['host']) + # If list is empty then all devices are configured. if not self.device_list: return self.async_abort(reason='devices_configured') - # Add existing creds for linking. Should be only 1. - if not creds: - # Abort if creds is missing. - return self.async_abort(reason='credential_error') - self.creds = creds['data'] # Login to PS4 with user data. if user_input is not None: @@ -163,11 +165,21 @@ class PlayStation4FlowHandler(config_entries.ConfigFlow): }, ) + # Try to find region automatically. + if not self.location: + self.location = await self.hass.async_add_executor_job( + location.detect_location_info) + if self.location: + country = self.location.country_name + if country in COUNTRIES: + default_region = country + # Show User Input form. link_schema = OrderedDict() link_schema[vol.Required(CONF_IP_ADDRESS)] = vol.In( list(self.device_list)) - link_schema[vol.Required(CONF_REGION)] = vol.In(list(regions)) + link_schema[vol.Required( + CONF_REGION, default=default_region)] = vol.In(list(regions)) link_schema[vol.Required(CONF_CODE)] = vol.All( vol.Strip, vol.Length(min=8, max=8), vol.Coerce(int)) link_schema[vol.Required(CONF_NAME, default=DEFAULT_NAME)] = str diff --git a/homeassistant/components/ps4/manifest.json b/homeassistant/components/ps4/manifest.json index 605dd3f530c..fcfcad95c12 100644 --- a/homeassistant/components/ps4/manifest.json +++ b/homeassistant/components/ps4/manifest.json @@ -3,8 +3,10 @@ "name": "Ps4", "documentation": "https://www.home-assistant.io/components/ps4", "requirements": [ - "pyps4-homeassistant==0.5.2" + "pyps4-homeassistant==0.7.2" ], "dependencies": [], - "codeowners": [] + "codeowners": [ + "@ktnrg45" + ] } diff --git a/homeassistant/components/ps4/media_player.py b/homeassistant/components/ps4/media_player.py index 3382cd6fe43..a53110b6f0e 100644 --- a/homeassistant/components/ps4/media_player.py +++ b/homeassistant/components/ps4/media_player.py @@ -9,6 +9,7 @@ from homeassistant.components.media_player import ( from homeassistant.components.media_player.const import ( MEDIA_TYPE_GAME, SUPPORT_SELECT_SOURCE, SUPPORT_STOP, SUPPORT_TURN_OFF, SUPPORT_TURN_ON) +from homeassistant.components.ps4 import format_unique_id from homeassistant.const import ( ATTR_COMMAND, ATTR_ENTITY_ID, CONF_HOST, CONF_NAME, CONF_REGION, CONF_TOKEN, STATE_IDLE, STATE_OFF, STATE_PLAYING) @@ -87,7 +88,7 @@ def setup_platform(hass, config, add_entities, discovery_info=None): name = device[CONF_NAME] ps4 = pyps4.Ps4(host, creds) device_list.append(PS4Device( - name, host, region, ps4, games_file)) + name, host, region, ps4, creds, games_file)) add_entities(device_list, True) @@ -102,21 +103,24 @@ class PS4Data(): class PS4Device(MediaPlayerDevice): """Representation of a PS4.""" - def __init__(self, name, host, region, ps4, games_file): + def __init__(self, name, host, region, ps4, creds, games_file): """Initialize the ps4 device.""" self._ps4 = ps4 self._host = host self._name = name self._region = region + self._creds = creds self._state = None self._games_filename = games_file self._media_content_id = None self._media_title = None self._media_image = None + self._media_type = None self._source = None self._games = {} self._source_list = [] self._retry = 0 + self._disconnected = False self._info = None self._unique_id = None self._power_on = False @@ -145,6 +149,7 @@ class PS4Device(MediaPlayerDevice): status = None if status is not None: self._retry = 0 + self._disconnected = False if status.get('status') == 'Ok': # Check if only 1 device in Hass. if len(self.hass.data[PS4_DATA].devices) == 1: @@ -187,7 +192,9 @@ class PS4Device(MediaPlayerDevice): """Set states for state unknown.""" self.reset_title() self._state = None - _LOGGER.warning("PS4 could not be reached") + if self._disconnected is False: + _LOGGER.warning("PS4 could not be reached") + self._disconnected = True self._retry = 0 def reset_title(self): @@ -198,19 +205,24 @@ class PS4Device(MediaPlayerDevice): def get_title_data(self, title_id, name): """Get PS Store Data.""" + from pyps4_homeassistant.errors import PSDataIncomplete app_name = None art = None try: - app_name, art = self._ps4.get_ps_store_data( + title = self._ps4.get_ps_store_data( name, title_id, self._region) - except TypeError: + except PSDataIncomplete: _LOGGER.error( "Could not find data in region: %s for PS ID: %s", self._region, title_id) + else: + app_name = title.name + art = title.cover_art finally: self._media_title = app_name or name self._source = self._media_title self._media_image = art + self._media_type = MEDIA_TYPE_GAME self.update_list() def update_list(self): @@ -257,7 +269,7 @@ class PS4Device(MediaPlayerDevice): self.save_games(games) def get_device_info(self, status): - """Return device info for registry.""" + """Set device info for registry.""" _sw_version = status['system-version'] _sw_version = _sw_version[1:4] sw_version = "{}.{}".format(_sw_version[0], _sw_version[1:]) @@ -270,12 +282,14 @@ class PS4Device(MediaPlayerDevice): 'manufacturer': 'Sony Interactive Entertainment Inc.', 'sw_version': sw_version } - self._unique_id = status['host-id'] + + self._unique_id = format_unique_id(self._creds, status['host-id']) async def async_will_remove_from_hass(self): """Remove Entity from Hass.""" # Close TCP Socket - await self.hass.async_add_executor_job(self._ps4.close) + if self._ps4.connected: + await self.hass.async_add_executor_job(self._ps4.close) self.hass.data[PS4_DATA].devices.remove(self) @property @@ -321,7 +335,7 @@ class PS4Device(MediaPlayerDevice): @property def media_content_type(self): """Content type of current playing media.""" - return MEDIA_TYPE_GAME + return self._media_type @property def media_image_url(self): @@ -370,13 +384,18 @@ class PS4Device(MediaPlayerDevice): def select_source(self, source): """Select input source.""" for title_id, game in self._games.items(): - if source == game: + if source.lower().encode(encoding='utf-8') == \ + game.lower().encode(encoding='utf-8') \ + or source == title_id: _LOGGER.debug( "Starting PS4 game %s (%s) using source %s", game, title_id, source) self._ps4.start_title( title_id, running_id=self._media_content_id) return + _LOGGER.warning( + "Could not start title. '%s' is not in source list", source) + return def send_command(self, command): """Send Button Command.""" diff --git a/homeassistant/components/ps4/strings.json b/homeassistant/components/ps4/strings.json index ea69d8c7a8c..77443b1ee9a 100644 --- a/homeassistant/components/ps4/strings.json +++ b/homeassistant/components/ps4/strings.json @@ -26,6 +26,7 @@ } }, "error": { + "credential_timeout": "Credential service timed out. Press submit to restart.", "not_ready": "PlayStation 4 is not on or connected to network.", "login_failed": "Failed to pair to PlayStation 4. Verify PIN is correct.", "no_ipaddress": "Enter the IP Address of the PlayStation 4 you would like to configure." diff --git a/requirements_all.txt b/requirements_all.txt index 24371f3d0f6..e53402d49a3 100644 --- a/requirements_all.txt +++ b/requirements_all.txt @@ -1237,7 +1237,7 @@ pypoint==1.1.1 pypollencom==2.2.3 # homeassistant.components.ps4 -pyps4-homeassistant==0.5.2 +pyps4-homeassistant==0.7.2 # homeassistant.components.qwikswitch pyqwikswitch==0.93 diff --git a/requirements_test_all.txt b/requirements_test_all.txt index e0051c8edaf..4f805039041 100644 --- a/requirements_test_all.txt +++ b/requirements_test_all.txt @@ -243,7 +243,7 @@ pyopenuv==1.0.9 pyotp==2.2.7 # homeassistant.components.ps4 -pyps4-homeassistant==0.5.2 +pyps4-homeassistant==0.7.2 # homeassistant.components.qwikswitch pyqwikswitch==0.93 diff --git a/tests/components/ps4/test_config_flow.py b/tests/components/ps4/test_config_flow.py index 06fe1ef65da..5db3fc2dd81 100644 --- a/tests/components/ps4/test_config_flow.py +++ b/tests/components/ps4/test_config_flow.py @@ -7,6 +7,7 @@ from homeassistant.components.ps4.const import ( DEFAULT_NAME, DEFAULT_REGION) from homeassistant.const import ( CONF_CODE, CONF_HOST, CONF_IP_ADDRESS, CONF_NAME, CONF_REGION, CONF_TOKEN) +from homeassistant.util import location from tests.common import MockConfigEntry @@ -47,11 +48,17 @@ MOCK_TCP_PORT = int(997) MOCK_AUTO = {"Config Mode": 'Auto Discover'} MOCK_MANUAL = {"Config Mode": 'Manual Entry', CONF_IP_ADDRESS: MOCK_HOST} +MOCK_LOCATION = location.LocationInfo( + '0.0.0.0', 'US', 'United States', 'CA', 'California', + 'San Diego', '92122', 'America/Los_Angeles', 32.8594, + -117.2073, True) + async def test_full_flow_implementation(hass): """Test registering an implementation and flow works.""" flow = ps4.PlayStation4FlowHandler() flow.hass = hass + flow.location = MOCK_LOCATION manager = hass.config_entries # User Step Started, results in Step Creds @@ -105,6 +112,7 @@ async def test_multiple_flow_implementation(hass): """Test multiple device flows.""" flow = ps4.PlayStation4FlowHandler() flow.hass = hass + flow.location = MOCK_LOCATION manager = hass.config_entries # User Step Started, results in Step Creds @@ -165,6 +173,13 @@ async def test_multiple_flow_implementation(hass): {'host-ip': MOCK_HOST_ADDITIONAL}]): result = await flow.async_step_user() assert result['type'] == data_entry_flow.RESULT_TYPE_FORM + assert result['step_id'] == 'creds' + + # Step Creds results with form in Step Mode. + with patch('pyps4_homeassistant.Helper.get_creds', + return_value=MOCK_CREDS): + result = await flow.async_step_creds({}) + assert result['type'] == data_entry_flow.RESULT_TYPE_FORM assert result['step_id'] == 'mode' # Step Mode with User Input which is not manual, results in Step Link. @@ -229,6 +244,7 @@ async def test_duplicate_abort(hass): MockConfigEntry(domain=ps4.DOMAIN, data=MOCK_DATA).add_to_hass(hass) flow = ps4.PlayStation4FlowHandler() flow.hass = hass + flow.creds = MOCK_CREDS with patch('pyps4_homeassistant.Helper.has_devices', return_value=[{'host-ip': MOCK_HOST}]): @@ -284,6 +300,7 @@ async def test_manual_mode(hass): """Test host specified in manual mode is passed to Step Link.""" flow = ps4.PlayStation4FlowHandler() flow.hass = hass + flow.location = MOCK_LOCATION # Step Mode with User Input: manual, results in Step Link. with patch('pyps4_homeassistant.Helper.has_devices', @@ -305,10 +322,24 @@ async def test_credential_abort(hass): assert result['reason'] == 'credential_error' +async def test_credential_timeout(hass): + """Test that Credential Timeout shows error.""" + from pyps4_homeassistant.errors import CredentialTimeout + flow = ps4.PlayStation4FlowHandler() + flow.hass = hass + + with patch('pyps4_homeassistant.Helper.get_creds', + side_effect=CredentialTimeout): + result = await flow.async_step_creds({}) + assert result['type'] == data_entry_flow.RESULT_TYPE_FORM + assert result['errors'] == {'base': 'credential_timeout'} + + async def test_wrong_pin_error(hass): """Test that incorrect pin throws an error.""" flow = ps4.PlayStation4FlowHandler() flow.hass = hass + flow.location = MOCK_LOCATION with patch('pyps4_homeassistant.Helper.link', return_value=(True, False)), \ @@ -324,6 +355,7 @@ async def test_device_connection_error(hass): """Test that device not connected or on throws an error.""" flow = ps4.PlayStation4FlowHandler() flow.hass = hass + flow.location = MOCK_LOCATION with patch('pyps4_homeassistant.Helper.link', return_value=(False, True)), \