Skip to content
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

Created a media player control widget #1005

Merged
merged 13 commits into from Oct 13, 2020

Conversation

KBerstene
Copy link
Contributor

@KBerstene KBerstene commented Oct 4, 2020

I'm working on creating a 4x1 widget for media player entities. It has a spot for a picture of the currently playing media, a label, a play/pause button, and optional seek/skip buttons.

I've added the Picasso library (https://github.com/square/picasso) for fetching an image URL and loading it into the widget ImageView. Just like the state widget, this is only updated when the image is clicked on. If webhooks ever get implemented, it would be a good idea to add this to the list of things to get updated via webhooks (along with the play/pause button being replaced by play or pause buttons depending on the state of the media player).

Below is an image of the the variations on the buttons:
image

Image with media loaded (set to center crop, which seems to be the best option, or at least better than center fit):
image

Image of the configure screen:

image

@KBerstene KBerstene changed the title Added initial media player widget code Created a media player control widget Oct 4, 2020
@KBerstene KBerstene marked this pull request as ready for review October 4, 2020 19:21
android:minHeight="40dp"
android:updatePeriodMillis="86400000"
android:widgetCategory="home_screen" />
<!--android:previewImage="@drawable/example_appwidget_preview"-->
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think going forward we are going to want every widget to include a preview image. If you need help with it let me know and I can add it. I personally just took a screenshot of the widget, cropped and set leftover background transparent.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, totally forgot to include that. I saw your other preview images, I'll try one like those.

Copy link
Contributor Author

@KBerstene KBerstene Oct 10, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added a preview image in commit e3a66e5, but it's in light mode instead of dark mode. Do you want it to be dark mode to match the rest or light mode to have an example of what the light mode widgets look like?

}

private suspend fun retrieveMediaPlayerImageUrl(entityId: String): String? {
val entity = integrationUseCase.getEntities().find { e -> e.entityId.equals(entityId) }
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be an expensive call, I know we use it elsewhere but we should consider either in this PR or in the near future add a way to get a single entity rather than all of the entities.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I 100% agree. I saw that the Static widget was using it and was incredibly surprised it hadn't been more targeted. I'll see if I can add the code to call just the single entity as part of the next set of changes in this PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Implemented this change in d923e8c. Obviously, I had to add an additional GET call in the integration repository. There are a few things in there that work but I'm not sure if they are the best method possible (specifically SingleEntityResponse) that I would appreciate you looking at extra-closely.

app/src/main/res/layout/widget_media_controls.xml Outdated Show resolved Hide resolved
Comment on lines +19 to +35
<ImageView
android:id="@+id/widgetMediaImage"
android:layout_width="match_parent"
android:layout_height="match_parent"
android:visibility="invisible"
android:contentDescription="@string/widget_media_image_description"
android:scaleType="centerCrop" />
<ImageView
android:id="@+id/widgetMediaPlaceholder"
android:layout_width="match_parent"
android:layout_height="match_parent"
android:layout_marginStart="4dp"
android:layout_marginTop="4dp"
android:layout_marginBottom="4dp"
android:src="@drawable/app_icon"
android:contentDescription="@string/widget_media_image_description"
android:scaleType="fitCenter" />
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we just use the same ImageView and replace the image when we have/don't have an image?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason I have it set like this is because of the margins. If the image is supposed to take up the entire left-hand section, it can't have any margins on it, but the placeholder looks silly if it is touching the edge. I can give you some screenshots of what it looks like if it's just straight replacement if you'd like.

@KBerstene
Copy link
Contributor Author

I will get on these changes at my next opportunity. Work's been crazy lately so I might not have the energy to take care of it until next weekend, but I'll try to get something done this week.

Thanks as always for digging through my crap and finding these fixes!

suspend fun getState(
@Header("Authorization") auth: String,
@Path("entity") entityId: String
): SingleEntityResponse<Any>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't need to create another object, can just reuse the one we use when requesting all states. Then if we get a message back we can just throw an exception rather than return null.

Suggested change
): SingleEntityResponse<Any>
): EntityResponse<Any>

@@ -380,6 +380,31 @@ class IntegrationRepositoryImpl @Inject constructor(
}.toTypedArray()
}

override suspend fun getEntity(entityId: String): Entity<Any>? {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably want to throw an exception if we don't have the entity.

Suggested change
override suspend fun getEntity(entityId: String): Entity<Any>? {
override suspend fun getEntity(entityId: String): Entity<Any> {

@JBassett JBassett merged commit 51072b9 into home-assistant:master Oct 13, 2020
@KBerstene KBerstene deleted the media_widget branch October 24, 2020 04:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants