Add Google Photos integration #124835

allenporter · 2024-08-29T00:53:40Z

Proposed change

Add Google Photos integration with an initial media source platform. The main parts of this integration have been copied from Google Tasks (config flow, client library, etc) with minor modernization updates.

Milestones for future PRs include:

Display recent photos in a media source
Config flow reauth
Uploading content
Display favorites as a folder in the media source
Display albums as folders in the media source
Display shared albums in the media source

Currently this requests scopes for read access to the entire library. As more features are added above, users can select which scopes they want for giving access to home assistant and the integration will respect those choices.

In the future we can consider expanding media sources to support paging and search to make them more useful for this integration.

---------- coverage: platform linux, python 3.12.2-final-0 -----------
Name                                                                Stmts   Miss  Cover   Missing
-------------------------------------------------------------------------------------------------
homeassistant/components/google_photos/__init__.py                     27      0   100%
homeassistant/components/google_photos/api.py                          64      0   100%
homeassistant/components/google_photos/application_credentials.py       7      1    86%   19
homeassistant/components/google_photos/config_flow.py                  30      0   100%
homeassistant/components/google_photos/const.py                         4      0   100%
homeassistant/components/google_photos/exceptions.py                    2      0   100%
homeassistant/components/google_photos/media_source.py                107      0   100%
-------------------------------------------------------------------------------------------------
TOTAL                                                                 241      1    99%


Results (2.68s):
      18 passed

Type of change

Dependency upgrade
Bugfix (non-breaking change which fixes an issue)
New integration (thank you!)
New feature (which adds functionality to an existing integration)
Deprecation (breaking change to happen in the future)
Breaking change (fix/feature causing existing functionality to break)
Code quality improvements to existing code or addition of tests

Additional information

This PR fixes or closes issue: fixes #
This PR is related to issue:
Link to documentation pull request: Add Google Photos integration documentation home-assistant.io#34486

Checklist

The code change is tested and works locally.
Local tests pass. Your PR cannot be merged unless tests pass
There is no commented out code in this PR.
I have followed the development checklist
I have followed the perfect PR recommendations
The code has been formatted using Ruff (ruff format homeassistant tests)
Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

Documentation added/updated for www.home-assistant.io

If the code communicates with devices, web services, or third-party tools:

The manifest file has all fields filled out correctly.
Updated and included derived files by running: python3 -m script.hassfest.
New or updated dependencies have been added to requirements_all.txt.
Updated by running python3 -m script.gen_requirements_all.
For the updated dependencies - a link to the changelog, or at minimum a diff between library versions is added to the PR description.

To help with the load of incoming pull requests:

I have reviewed two other open pull requests in this repository.

homeassistant/components/google_photos/__init__.py

homeassistant/components/google_photos/manifest.json

homeassistant/components/google_photos/media_source.py

joostlek · 2024-08-30T09:17:34Z

tests/components/google_photos/test_config_flow.py

+    with patch(
+        "homeassistant.components.google_photos.async_setup_entry", return_value=True
+    ) as mock_setup:


Create a generic fixture for this because you have more places where you will use this

This is only currently used once so I avoided adding a fixture. How about adding when it's needed in future PRs? Unless you're saying it's needed in more cases in this PR.

tests/components/google_photos/test_config_flow.py

tests/components/google_photos/test_init.py

home-assistant · 2024-08-30T09:19:29Z

Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍

Learn more about our pull request process.

Co-authored-by: Joost Lekkerkerker <[email protected]>

homeassistant/components/google_photos/media_source.py

Co-authored-by: Joost Lekkerkerker <[email protected]>

allenporter · 2024-08-30T14:27:27Z

Thank you for the speedy high quality review

homeassistant/components/google_photos/config_flow.py

homeassistant/components/google_photos/manifest.json

tronikos · 2024-08-29T22:34:15Z

homeassistant/components/google_photos/media_source.py

What use cases do you have in mind for media source?

The use cases I am thinking would be useful are:

display "favorites" or a shared album on a display

use as a generic way to read media content for automations e.g. for an automation like sending to google generative ai vision service

Are there any actions that give access to media sources? AFAIK, they are only used in the frontend for manually browsing so the only use case right now that I can think of is to browse your library for casting a single item to a media player.

There is discussion here home-assistant/architecture#1079 so -- fair -- a future use case.

The upload service can return a media id in the service response, though, which could be used to form a media source identifier. The first half of this is a real use case, but the second half is not really:

"Ok nabu, take a photo" (based on context of the device i'm speaking from)

Automation calls camera snapshot

Automation calls google_photos.upload with the snapshot filename, which returns a media id

Automation calls a display automation with the media id to show it somewhere

I may play around in a custom component as well, possibly exposing something like the service mentioned above, or code doing generally processing on media sources. I was thinking if there were useful integrations to call google_generative_ai_conversation.generate_content with a media source, but maybe it doesn't make sense in core until there are more actions/hooks.

tronikos · 2024-08-29T22:41:09Z

homeassistant/components/google_photos/media_source.py

+
+# Currently supports a single album of recent photos
+RECENT_PHOTOS_ALBUM = "recent"
+RECENT_PHOTOS_TITLE = "Recent Photos"


Move this to strings.json for localization?

I can see how to do a one-off for google photos translations, however that doesn't appear to be a common pattern. I think if we want to support this it would make sense to do for all media sources. I spent about 45 minutes looking at what it might take to do this and am pausing as it looks quite involved given the current media player APIs. If there is a simple common pattern here i'm missing happy follow it.

I just looked at it too. It doesn't seem it's possible without a hack such as adding the string as an exception. Maybe introduce a generic strings section in strings.json and add a function similar to async_get_exception_message in helpers.translation? The functions could share most of the code. Do you think you could open a design discussion?

tronikos · 2024-08-29T22:49:49Z

homeassistant/components/google_photos/media_source.py

+# Media Sources do not support paging, so we only show a subset of recent
+# photos when displaying the users library. We fetch a minimum of 50 photos
+# unless we run out, but in pages of 100 at a time given sometimes responses
+# may only contain a handful of items Fetches at least 50 photos.


Comment and constant name are confusing. I had to look at the code below to understand. Can you rename the constant and rephrase the comment? Or change the code below to truly have max 50 photos?

What i'm trying to hack around is if we set PAGE_SIZE = 50 you can get back like 10 photos (presumably due to server side filtering) and as a result spend a bunch of time on server side round trips. Given that context, have a suggestion for naming?

Yes I understand the potential server side filtering that might return less photos than requested in the page.
But there is a punctuation missing in the comment, I think a period before "Fetches at least 50 photos", and it's confusing having MAX_ in the constant (and now max_photos in SpecialAlbumDetails) when it's actually MIN_, but only if you have that many photos in your library, otherwise it's a MAX_. Unfortunately I don't have a good suggestion.

tronikos · 2024-08-29T23:14:41Z

homeassistant/components/google_photos/media_source.py

+    """
+    width = media_item["mediaMetadata"]["width"]
+    height = media_item["mediaMetadata"]["height"]
+    key = "h" if height > width else "w"


Does it make more sense to always have "h" here? If the image is shown fullscreen in a 4k screen we want max h2160. Similarly I assume the HA frontend when showing the grid of thumbnails caps the height? Depending on what resolution of thumbnails the HA frontend uses, maybe use both w and h? And maybe c too?

I did have a version setting h and w and couldn't convince myself what the right behavior is. I've updated to "h" since I agree the height is often what matters. For the full image, just setting it to be largeish in case someone wants a large image outside of the media player.

I can't convince myself what the right behavior here is or that c should be used.

I think just h is better than longest edge, at least for the full screen case. I haven't tried this integration yet but HA frontend in the grid for other images seems to already do some cropping by centering the image. That's why I think ideally for thumbnails we would specify w, h, and c that match what HA frontend does.

I trust your judgement here from your past experience optimizing photos on displays :) I'll experiment with it. I didn't notice the cropping until i saw some recent photos and they are cropped poorly (faces at top of the photo cut off -- didn't see if this is smart crop or not but it if is that would be useful)

homeassistant/components/google_photos/media_source.py

tronikos · 2024-08-30T15:54:42Z

homeassistant/components/google_photos/media_source.py

+        media_class=MediaClass.IMAGE if not is_video else MediaClass.VIDEO,
+        media_content_type=MediaType.IMAGE if not is_video else MediaType.VIDEO,
+        title=media_item["filename"],
+        can_play=is_video,


Should this be True for images too? I just tried the Media tab in Home Assistant and I was able to "play" an image entity (FullyKiosk screenshot image entity) on the browser by displaying it and also casting it by connecting it to a Nest Hub. Have you tried that playing images and videos works on media players?

I'll give it a try. I may not understand what 'can_play' means.

homeassistant/components/google_photos/strings.json

homeassistant/components/google_photos/media_source.py

Add Google Photos integration

9e0ed3b

home-assistant bot added cla-signed core has-tests new-integration integration: google_photos labels Aug 29, 2024

Mark credentials typing

344d2de

This was referenced Aug 29, 2024

Add Google Photos integration documentation home-assistant/home-assistant.io#34486

Merged

Add Google Photos logo home-assistant/brands#5811

Merged

joostlek requested changes Aug 30, 2024

View reviewed changes

home-assistant bot marked this pull request as draft August 30, 2024 09:19

allenporter and others added 2 commits August 30, 2024 13:39

Add code review suggestions to simpilfy google_photos

bcf7605

Update tests/components/google_photos/conftest.py

bd18b04

Co-authored-by: Joost Lekkerkerker <[email protected]>

joostlek reviewed Aug 30, 2024

View reviewed changes

homeassistant/components/google_photos/media_source.py Outdated Show resolved Hide resolved

allenporter and others added 5 commits August 30, 2024 06:42

Apply suggestions from code review

80d81f8

Co-authored-by: Joost Lekkerkerker <[email protected]>

Fix comment typo

73174f6

Update test fixtures from review feedback

0b96914

Remove unnecessary test for services

c3bc6ac

Remove keyword argument

8e871b4

allenporter marked this pull request as ready for review August 30, 2024 14:23

home-assistant bot requested a review from joostlek August 30, 2024 14:23

joostlek approved these changes Aug 30, 2024

View reviewed changes

allenporter merged commit c01bb44 into home-assistant:dev Aug 30, 2024
36 checks passed

allenporter mentioned this pull request Aug 30, 2024

Add Google Photos reauth support #124933

Merged

25 tasks

tronikos reviewed Aug 30, 2024

View reviewed changes

MartinHjelmare reviewed Aug 30, 2024

View reviewed changes

homeassistant/components/google_photos/media_source.py Show resolved Hide resolved

This was referenced Aug 31, 2024

Add Google Photos service for uploading content #124956

Merged

Address additional Google Photos integration feedback #124957

Merged

This was referenced Aug 31, 2024

Fix Google Photos get media calls #124958

Merged

Add Google Photos media source support for albums and favorites #124985

Merged

github-actions bot locked and limited conversation to collaborators Sep 2, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Google Photos integration #124835

Add Google Photos integration #124835

allenporter commented Aug 29, 2024 •

edited

Loading

joostlek Aug 30, 2024

allenporter Aug 30, 2024

home-assistant bot commented Aug 30, 2024

allenporter commented Aug 30, 2024

tronikos Aug 29, 2024

allenporter Aug 31, 2024

tronikos Sep 1, 2024

allenporter Sep 1, 2024

tronikos Aug 29, 2024

allenporter Aug 31, 2024

tronikos Sep 1, 2024

tronikos Aug 29, 2024

allenporter Aug 31, 2024

tronikos Sep 1, 2024

tronikos Aug 29, 2024

allenporter Aug 31, 2024

tronikos Sep 1, 2024

allenporter Sep 1, 2024

tronikos Aug 30, 2024

allenporter Aug 31, 2024

Add Google Photos integration #124835

Add Google Photos integration #124835

Conversation

allenporter commented Aug 29, 2024 • edited Loading

Proposed change

Type of change

Additional information

Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

home-assistant bot commented Aug 30, 2024

allenporter commented Aug 30, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

allenporter commented Aug 29, 2024 •

edited

Loading