-
-
Notifications
You must be signed in to change notification settings - Fork 36.2k
Add PlayStation Network Integration #133901
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Do you get a separate unique id for every playstation? |
Yes and no. I can pull a list of registered devices and they do have a unique deviceId. However, the call to return the user state (What game are they playing, are they online), only returns the device type (PS4, PS5) which is not associated to any particular deviceId. I also question the validity of the registered device data. I've had two PS4s over the years but I only have one entry in the response. I do have entries for my three PS3 systems though. I meant to mention this in the original pull request, and I'll add it there now too, but I'm working with an undocumented and "unofficial" api. The psnawp project reverse engineered it from the android PlayStation Mobile app. The end result is that I'm left to infer some things rather than read it in the docs. |
7898fc4 to
679b123
Compare
|
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
9fcee12 to
f84ee93
Compare
homeassistant/components/playstation_network/quality_scale.yaml
Outdated
Show resolved
Hide resolved
homeassistant/components/playstation_network/quality_scale.yaml
Outdated
Show resolved
Hide resolved
homeassistant/components/playstation_network/quality_scale.yaml
Outdated
Show resolved
Hide resolved
homeassistant/components/playstation_network/quality_scale.yaml
Outdated
Show resolved
Hide resolved
homeassistant/components/playstation_network/quality_scale.yaml
Outdated
Show resolved
Hide resolved
ef11f41 to
6e93647
Compare
6e93647 to
b12c19e
Compare
649b5ca to
a4296ad
Compare
bieniu
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you please rabase and run python3 -m script.hassfest?
a4296ad to
b46872a
Compare
This comment was marked as resolved.
This comment was marked as resolved.
e7c165f to
0ba59ae
Compare
| if not SUPPORTED_PLATFORMS - devices_added: | ||
| remove_listener() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's this for?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The integration only adds entities for devices that the user is actively using. This is to prevent adding a PS3/PS4 that is in the user's lifetime device list, but that they no longer own or use. And once all of the integration's supported devices have been added, there is no longer a need to check for new ones.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But we remove old ones right
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I may not be following. Are you asking if we removed older platforms from the supported list? In which case, yes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would suggest we should allow users to remove devices manually as there is no way to recognize if they don't own that platform anymore
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But if they don't own the platform anymore, they won't be actively logged in to the playstation network on it, and the integration won't add it as a media player. In the current state, the no-longer-owned PS3 for instance won't be added to HA.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
anyway, that's something that can be added later
0ba59ae to
a0a478c
Compare
| if LEGACY_PLATFORMS & data.registered_platforms: | ||
| assert self.client is not None | ||
| self.legacy_profile = await self.hass.async_add_executor_job( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so if I understand correctly, in theory we can already do this request in the retrieve_psn_data. As in, we can do this check and this call there
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right again. I didn't want to do this call unnecessarily but as long as we shift some logic we can group it too and only make the call when necessary. Will fix
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All grouped together 👍🏻
| mock_psnawp_npsso.parse_npsso_token.side_effect = PSNAWPInvalidTokenError | ||
| result = await hass.config_entries.flow.async_init( | ||
| DOMAIN, | ||
| context={"source": config_entries.SOURCE_USER}, | ||
| data={CONF_NPSSO: NPSSO_TOKEN_INVALID_JSON}, | ||
| ) | ||
| assert result["errors"] == {"base": "invalid_account"} | ||
|
|
||
| result = await hass.config_entries.flow.async_init( | ||
| DOMAIN, context={"source": config_entries.SOURCE_USER} | ||
| ) | ||
| assert result["type"] is FlowResultType.FORM | ||
| assert result["errors"] == {} | ||
|
|
||
| mock_psnawp_npsso.parse_npsso_token.side_effect = None | ||
| result = await hass.config_entries.flow.async_configure( | ||
| result["flow_id"], | ||
| {CONF_NPSSO: NPSSO_TOKEN}, | ||
| ) | ||
|
|
||
| assert result["type"] is FlowResultType.CREATE_ENTRY | ||
| assert result["data"] == { | ||
| CONF_NPSSO: NPSSO_TOKEN, | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Currently this test is also creating 2 config flows while only one is needed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe this is fixed but wanted you to check first.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| "npsso": "NPSSO token" | ||
| }, | ||
| "data_description": { | ||
| "npsso": "The NPSSO token is generated during successful login of your PlayStation Network account and is used to authenticate your requests from with Home Assistant." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We missed the "… requests from with Home Assistant." here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
already fixed in another PR 👍🏼
* clean pull request * Create one device per console * Requested changes * Pr/tr4nt0r/1 (#2) * clean pull request * Create one device per console * device setup * Merge PR1 - Dynamic Device Support * Merge PR1 - Dynamic Device Support --------- Co-authored-by: tr4nt0r <[email protected]> * nitpicks * Update config_flow test * Update quality_scale.yaml * repair integrations.json * minor updates * Add translation string for invalid account * misc changes post review * Minor strings updates * strengthen config_flow test * Requested changes * Applied patch to commit a358725 * migrate PlayStationNetwork helper classes to HA * Revert to standard psn library * Updates to media_player logic * add default_factory, change registered_platforms to set * Improve test coverage * Add snapshot test for media_player platform * fix token parse error * Parametrize media player test * Add PS3 support * Add PS3 support * Add concurrent console support * Adjust psnawp rate limit * Convert to package PlatformType * Update dependency to PSNAWP==3.0.0 * small improvements * Add PlayStation PC Support * Refactor active sessions list * shift async logic to helper * Implemented suggested changes * Suggested changes * Updated tests * Suggested changes * Fix test * Suggested changes * Suggested changes * Update config_flow tests * Group remaining api call in single executor --------- Co-authored-by: tr4nt0r <[email protected]> Co-authored-by: Joost Lekkerkerker <[email protected]>
Proposed change
This change adds support for integrating with the PlayStation Network. In this first release (my first HA PR), I have attempted to remove all unneeded code (no entity base class since I only have a single platform) but anticipate adding it, and refactoring elsewhere to follow home assistant conventions in subsequent pull requests. The integration contains a single platform (Media Player), is setup fully via the UI, supplies unique IDs, and attempts to follow all current Home Assistant best practices.
Type of change
Additional information
When reviewing this integration, please keep in mind that it represents a user on the playstation network and not the PlayStation console itself. It is structured around allowing the user to associate multiple PSN accounts and the device names are setup to match this. This integration started its life as a custom component and has about ~400 users. For a look at what this may evolve into, here is a link to the custom repository.
This initial release of the PlayStation Network Integration exposes a media player based on the user's registered systems with the playstation network. One will be created for any PS4 systems and another for any PS5 systems. However, due to api limitations, it is not possible to pull data for more than a single console at a time nor do I have dates for when a console was last active. Given these limitations I'd like to discuss what the Home Assistant team believes is the best course of action to take in this regard:
A quality_scale.yaml file is included and I did my best to conform to all bronze level items. Some are marked as done, like common-modules, that still have pending tasks (the creation of entity.py for instance) but was skipped for this initial release. If this is the wrong approach, please let me know.
The integration communicates with the PlayStation Network via a supplied NPSSO token that the config flow walks the user through obtaining (along with the documentation referenced in PR#36520.
This integration relies on the PSNAWP project which was reversed engineered from the PlayStation Mobile app and is not endorsed or supported by Sony.
This PR wouldn't have been possible without the amazing help and guidance from @tr4nt0r. He put in so much time and effort to ensure this first PR go smoothly.
The associated brand images were included for the custom integration and are still up to date. Brands PR
Updates
After speaking with Joost, we decided to make a few changes to the submission:
1.1) I also submitted two PRs to his repo to integrate a couple of changes/additions I added to my fork of his repo. One of which has already been merged.
3.1) helpers.py has been added which includes the bulk of what I was maintaining in my fork.
3.2) config_flow - parse_npsso_token got some requested updates in my PR and required a change + tests in the integration
4.1) If this needs to be held for another PR, just let me know.
Checklist
ruff format homeassistant tests)If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
Updated and included derived files by running:
python3 -m script.hassfest.requirements_all.txt.Updated by running
python3 -m script.gen_requirements_all.To help with the load of incoming pull requests: