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

WebDAV sensor #19758

Closed
wants to merge 5 commits into from
Closed

WebDAV sensor #19758

wants to merge 5 commits into from

Conversation

jkeljo
Copy link
Contributor

@jkeljo jkeljo commented Jan 4, 2019

Description:

This sensor monitors a WebDAV share. Its state is simply a boolean
of whether the share is available or not. It has a files attribute
that lists all files in the share.

Related issue (if applicable): n/a

Pull request in home-assistant.io with documentation (if applicable): home-assistant/home-assistant.io#8038

Example entry for configuration.yaml (if applicable):

binary_sensor:
  - platform: webdav
    name: file_server
    host: URL_OF_SHARE

Checklist:

  • The code change is tested and works locally.
  • Local tests pass with tox. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.

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

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

  • New dependencies have been added to the REQUIREMENTS variable (example).
  • New dependencies are only imported inside functions that use them (example).
  • New or updated dependencies have been added to requirements_all.txt by running script/gen_requirements_all.py.
  • New files were added to .coveragerc.

If the code does not interact with devices:

  • Tests have been added to verify that the new code works.

This sensor monitors a WebDAV share. Its state is simply a boolean
of whether the share is available or not. It has a `files` attribute
that lists all files in the share.
@ghost ghost added the in progress label Jan 4, 2019
jkeljo added a commit to jkeljo/home-assistant that referenced this pull request Jan 4, 2019
The Depict art frame (https://depict.com) is a wall-mounted 49" 4K display
that looks like a picture frame. It seems to be mostly intended to view
curated art collections (most of which are paid subscriptions). The app that comes
with it allows one-off uploading of photos, but has no slideshow support or
similar.

Home Assistant to the rescue! This component allows Home Assistant to turn
the frame on and off, adjust brightness and contrast, and set what image
is displayed on the frame. By combining this component with a
[`random`](https://www.home-assistant.io/components/sensor.random/) sensor,
a `webdav` sensor (home-assistant#19758), and a templated action,
Home Assistant can run a shuffled slideshow of images on the frame.
jkeljo added a commit to jkeljo/home-assistant that referenced this pull request Jan 4, 2019
The Depict art frame (https://depict.com) is a wall-mounted 49" 4K display
that looks like a picture frame. It seems to be mostly intended to view
curated art collections (most of which are paid subscriptions). The app that comes
with it allows one-off uploading of photos, but has no slideshow support or
similar.

Home Assistant to the rescue! This component allows Home Assistant to turn
the frame on and off, adjust brightness and contrast, and set what image
is displayed on the frame. By combining this component with a
[`random`](https://www.home-assistant.io/components/sensor.random/) sensor,
a `webdav` sensor (home-assistant#19758), and a templated action,
Home Assistant can run a shuffled slideshow of images on the frame.
jkeljo added a commit to jkeljo/home-assistant that referenced this pull request Jan 4, 2019
The Depict art frame (https://depict.com) is a wall-mounted 49" 4K display
that looks like a picture frame. It seems to be mostly intended to view
curated art collections (most of which are paid subscriptions). The app that comes
with it allows one-off uploading of photos, but has no slideshow support or
similar.

Home Assistant to the rescue! This component allows Home Assistant to turn
the frame on and off, adjust brightness and contrast, and set what image
is displayed on the frame. By combining this component with a
[`random`](https://www.home-assistant.io/components/sensor.random/) sensor,
a `webdav` sensor (home-assistant#19758), and a templated action,
Home Assistant can run a shuffled slideshow of images on the frame.
@jkeljo jkeljo mentioned this pull request Jan 4, 2019
9 tasks
@fabaff
Copy link
Member

fabaff commented Jan 4, 2019

Its state is simply a boolean of whether the share is available or not.

If there are only two states then it should be a binary sensor.

@property
def unique_id(self):
"""Return the URL for the share."""
return self._get_url("")
Copy link
Member

Choose a reason for hiding this comment

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

Unique IDs should be truly unique. For example if the user changes the path of this sensor, it will have another unique ID and thus an entity_id with _2 added at the end. See https://developers.home-assistant.io/docs/en/entity_registry_index.html#unique-id-requirements for requirements.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If the user changes the URL, the sensor is now monitoring a different share. Doesn't that make it a different sensor?

- `state_attributes` -> `device_state_attributes`
- remove unnecessary `str`
- try to connect in `setup_platform`
from webdav3.client import Client
from webdav3.exceptions import WebDavException

name = config.get(CONF_NAME)
Copy link
Contributor

Choose a reason for hiding this comment

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

config[CONF_NAME] as CONF_NAME is required

from webdav3.exceptions import WebDavException

name = config.get(CONF_NAME)
host = config.get(CONF_HOST)
Copy link
Contributor

Choose a reason for hiding this comment

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

as with CONF_NAME

return self._name

@property
def unique_id(self):
Copy link
Contributor

Choose a reason for hiding this comment

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

Really should be left out ... it's by no mean unique

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're the second person to push back on this identifier, and I really don't understand it. As I read the wiki, the unique ID must be unique within the platform (i.e. sensor.webdav). The URL of the share is the identity of the thing being observed, just like a device serial number or MAC address (examples from the unique ID docs) identifies a device. I could enforce that people cannot configure multiple sensors for the same URL if you guys want, but I haven't seen other platforms prevent the same device from being referenced twice in configuration, even though they use device serial numbers for unique IDs. What am I missing?

Copy link
Contributor

Choose a reason for hiding this comment

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

The thing with unique_id's is that they will live forever in HA (even after you remove the configuration). MAC, EUI, etc aren't user configurable, and that's what's set them apart.

Why don't you add a friendly_name configuration parameter instead?

def device_state_attributes(self):
"""Return the list of files on the share."""
return {
"files": self._files,
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm concerned this list can become very big and blow up the UI... can we have a limit on the number of files here at least ? say 10 ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It could become very large indeed. However, limiting the files, particularly to a number as small as 10, makes it useless for the scenario that's motivating this component for me (driving slideshows in a photo frame). Another alternative I considered was exposing a service that one would have to call to get the data somehow. The simplest thing would be to have that service populate this state field (instead of populating it all the time, so at least if it's blowing up the UI you have a way to make it not do so). Another approach would be to pass a template to the service, which it would evaluate against the file list and then pass to another service call. Would either of those be acceptable?

Copy link
Contributor

Choose a reason for hiding this comment

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

OK, you described a use case that is by no means related to this PR.

This binary_sensor is about connectivity (see your DEVICE_CLASS). It should be just about availability of the webdav server an nothing more.

What you wan't is an entity that feeds you files into your photo frame, right ? I think you could model your use case through a camera, but I need to understand better your use-case, can you describe in more detail the end result you expect ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Camera... that's an interesting idea.

So I've got two kinds of photo frames I'm working with. The API for the Depict lets you give it a URL and the frame will download the photo. The other (the Samsung Frame TV) lets you upload the photo to the frame. Neither has slideshow support on the frame itself, so I'm looking to build that with HA.

What I have going with the Depict now is this webdav sensor listing available photos, a random sensor driving which photo should appear, and a template action using the value of the random sensor to pick a URL from the webdav sensor to send to the Depict. I haven't started on the Samsung yet but I imagine I'll drive it similarly, only have the HA component download the photo from the URL and then send it to the frame. Modeling the WebDAV share as a camera could simplify that stuff, and open up new possibilities like sending surveillance camera images to the frames in response to certain events, which would be cool.

This kind of setup works, and has the advantage of decoupling the image source from the photo frames -- one could add cameras for various cloud storage providers, social media sites, whatever, and use those just as easily as this WebDAV thing, without each different kind of frame having to have explicit support for each service. However, it's not ideal.

What would be really nice is if the photo frame media player entities could support play/pause/forward/backward/shuffle/etc... the way I have it set up right now (and the way I'm imagining a camera component would work), none of that is controllable by the media player since the media player is just given one image at a time. I think the existing media players each have their own intrinsic notion of a playlist, and I could add playlist support to depict-control, but I do really like this idea of being able to use Home Assistant to plumb images from any source to any display device. Is there a type of device that would make sense for providing a list of images that a media player could use as a playlist?

Copy link
Contributor

Choose a reason for hiding this comment

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

Very long thread :)

IMHO I would split Depict from the Samsung Frame TV.

For Depict, you really just need a camera, you pass the camera URL to Depict and that's it. Don't call it camera.depict, call it camera.webdav. It would be nice to have such camera showing up family pictures in the middle of the interface :)

For samsung frame tv... definitely a media_player, ultimately that's what you're doing...

The issue now is the playlist.... for the camera.webdav, the playlist can be part of the component (that is something IMHO fits into the scope of the "camera"... but don't take my word for it). Another option would be to create a new component just for the purpose of the playlist's...

I think we should take baby steps and learn in the process, why don't you start with the camera.webdav ? I would use it :)

@balloob
Copy link
Member

balloob commented Feb 21, 2019

We should not merge this . This is clearly an integration built for a very specific purpose, which does not fit with the general theme of Home Assistant integrations.

A webdav integration would be a component that fires events whenever a file has changed. We should not abuse our state machine to transfer data for automations.

@balloob balloob closed this Feb 21, 2019
@ghost ghost removed the in progress label Feb 21, 2019
jkeljo added a commit to jkeljo/home-assistant that referenced this pull request Apr 1, 2019
As suggested by @dgomes in home-assistant#19758, here's a camera backed by image files on
a WebDAV share. The entity exposes a stream and individual images, iterating
through the images in sorted order by filename at a configurable rate.
jkeljo added a commit to jkeljo/home-assistant that referenced this pull request Apr 1, 2019
As suggested by @dgomes in home-assistant#19758, here's a camera backed by image files on
a WebDAV share. The entity exposes a stream and individual images, iterating
through the images in sorted order by filename at a configurable rate.
jkeljo added a commit to jkeljo/home-assistant that referenced this pull request Apr 1, 2019
As suggested by @dgomes in home-assistant#19758, here's a camera backed by image files on
a WebDAV share. The entity exposes a stream and individual images, iterating
through the images in sorted order by filename at a configurable rate.
@jkeljo jkeljo mentioned this pull request Apr 1, 2019
9 tasks
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

8 participants