home-assistant / architecture

Repo to discuss Home Assistant architecture
320 stars 100 forks source link

RFC: Add set_state service to Camera #35

Closed awarecan closed 6 years ago

awarecan commented 6 years ago

Camera defined three states [STATE_RECORDING, STATE_STREAMING, STATE_IDLE], but don't have services to allow user change state.

Proposed Solution:

Add abstract ~set_state~ set_mode service to Camera, take entity_id and ~state~ mode as parameter. Using different mode const to clarify that shall not directly set state machine, [MODE_RECORDING, MODE_STREAMING, MODE_IDLE]. Each platform should have its own implementation

Alternative Solution:

Add generic turn_on turn_off service to Camera.

Approved Solution:

balloob commented 6 years ago

This is not how Home Assistant works.

States represent the state of a device.

To change states, you interact with the device via services. This will then update the representation of the device in the state machine.

awarecan commented 6 years ago

I am providing service, not direct set state. Maybe set_mode is better name?

awarecan commented 6 years ago

Change set_state to set_mode

balloob commented 6 years ago

We already have a service 'enable_motion_detection'. Do you mean you want to add a new service start_recording and stop_recording?

awarecan commented 6 years ago

In some platform, an detected motion may trigger the state transition from STATE_STREAMING to STATE_RECORDING. But 'enable_motion_detection' wouldn't.

If we use stop/start_recording, then we also need provide stop/start_streaming, and from STATE_RECORDING to STATE_STREAMING, we may need first stop_recording, state to STATE_STREAMING, then stop_streaming.

balloob commented 6 years ago

No. The way the camera entity works has to be changed. The abstract base class should never control the state. It's wrong.

awarecan commented 6 years ago

So, I checked light and climate, they both have similar implement as camera, are they all wrong? Which component should be used as good example?

pnbruckner commented 6 years ago

Hi. I originally made this suggestion to @awarecan . Although I'm an experienced Python developer, I'm not set up to make formal HA pull requests yet. He was nice enough to offer to do this. Some background. I have a Nest Cam and an Amcrest camera that I use with HA. I created custom components to enhance the existing components and associated camera platforms. Besides other enhancements, I added platform level services for both to set their operation mode. However, I feel it makes more sense to provide the service and corresponding infrastructure in the camera component itself for changing the camera's operation mode, similar to what is done in the climate component. Obviously the implementation should be left to the platform code. My suggestion is to add a set_operation_mode service where the options are idle, streaming and recording, which correspond to the camera component's possible states. The platform code can determine if all or just a subset of these options make sense for it. E.g., for Nest, only idle and recording make sense, whereas for Amcrest, all three would make sense. Thank you for considering allowing this service to be added to the camera component.

balloob commented 6 years ago

I don't see how one could tell a camera to start streaming or how this is related to recording. A camera is "streaming" if someone is watching, which is probably a wrong thing to be in the state to begin with.

awarecan commented 6 years ago

Streaming is live streaming, for save bandwidth or storage, it may not be in "recording" video clip to somewhere. The transition between streaming and recording may trigger by some montion detection or by home/away status, or manually.

pnbruckner commented 6 years ago

I'm a little confused how you don't see this. I must not be explaining correctly, for which I apologize. E.g, the Nest Camera can have it's streaming/recording mode changed via its Cloud API. (See https://developers.nest.com/documentation/cloud/api-camera#is_streaming. Note that when the Nest Camera streaming mode is enabled it also records.) The Amcrest camera can also have streaming enabled/disabled, as well as recording enabled/disabled via its HTTP API. (It's API is documented here https://support.amcrest.com/hc/en-us/articles/232310528-Amcrest-HTTP-API-SDK.) I've used both of these interfaces to change the operating mode of the corresponding camera. I use this in my automations, e.g., to enable streaming/recording while nobody is home, at night, etc.

Kane610 commented 6 years ago

Recording is usually triggered by some preset condition like motion, and streaming is something only active while a user is accessing the video feed. Disabling and enabling recording is understandable that could be set by a service, but streaming is a different kind of thing.

Perhaps explain what use cases you want to solve with this.

awarecan commented 6 years ago

@Kane610 There are tons of reason I want to turn off my camera such as Privacy, save bandwidth (for camera such as Nest Cam that doesn't have local streaming options). I can do it through Nest App, and Nest App even have automation feature allow me to set turn on/off schedule.

Kane610 commented 6 years ago

So what you want is a sensor for video stream access and a stop/disable streaming service?

pnbruckner commented 6 years ago

We, and others (e.g., as just one example see https://community.home-assistant.io/t/disable-enable-nest-camera-recording/21096), are just looking to add a service that can disable a camera's video stream (at the source, whether or not someone is actually viewing it), enable it, or enable both streaming and recording if the camera has that capability, which in my case, both the Nest and Amcrest cameras do. We're not looking to add or change anything else. I.e., add "set operation mode" to either idle, streaming or recording, which also happens to be the states that the camera component can currently return.

I guess I'm not sure where the confusion is, unless it's a question of what the streaming state means. Are you thinking that state means someone is actively viewing the video from the camera? If so, then this is kind of like, if a tree falls in a forest, and nobody is there to hear it, does it make a sound? We're talking about making the sound (i.e., the source) as opposed to hearing the sound (i.e., the sink.) That is, we want to be able to control the source of the stream (and possibly controlling the camera's own recording of it), regardless if anyone else is actually viewing it.

balloob commented 6 years ago

Okay, so I think this issue started off all wrong. It proposes a solution to a non-defined problem. And now with a lot more explanation, it's still unclear to both @Kane610 and me.

If I read it correctly, you're saying that with "streaming", you mean that a camera is turned on and that the stream is available for people to watch.

Recording is when the stream is being recorded (either offline or online).

Those are two complete separate things, why would they be cramped into a single service called set_mode?

awarecan commented 6 years ago

The proposal is base on current design, Camera has there state, IDLE, STREAMING, RECORDING.

Anyway, how about this:

balloob commented 6 years ago

Do you have a list of cameras that Home Assistant supports that support on/off and recording? We should not add anything that only 1 or 2 platforms will use.

pnbruckner commented 6 years ago

In my opinion, streaming (enabled) and recording are not two completely separate things, because it makes no sense to record if video isn't enabled in the first place. It makes perfectly good sense to me that a camera has three basic "video operation" modes - ilde/off, streaming & recording (which implies streaming.) It makes even more sense since those are the three states a camera can be in given the current implementation.

Again, I make analogy to the climate component. Its states are off, heat, cool, etc., and its set_operation_mode service allows setting the thermostat's mode to be equivalent to one of those states.

Regarding a list of cameras that HA supports that support these modes, I don't have one. I've only used Nest and Amcrest, and these modes make perfect sense for them, and I would think they would for most cameras. And just because other camera integrations don't provide this functionality, to me, is not reason enough not to provide a standard infrastructure for setting the camera's mode. There are certainly other features/services that components provide that not all existing platforms make use of.

But, this is your baby and you can do whatever you think makes sense. Currently I'm all set since I have custom components that enhance the existing components and camera platforms that I use to provide the functionality I need. I just thought it made sense to put common functionality into the component for everyone's benefit.

Kane610 commented 6 years ago

Start and stop recording I can understand you want to override other logics resident in the camera with. But that should be kept separate from stopping a stream, since you can't also start the stream again since you cut the connection with the client.

And the proposed states are also not clear; if you're recording, you might be streaming as well? I don't think it is comparable to the thermostats, as the camera can be both streaming and recording at the same time. Which I suppose these things fit better as attributes for the camera than being its state.

pnbruckner commented 6 years ago

"kept separate from stopping a stream, since you can't also start the stream again since you cut the connection with the client"

I would agree, if state == streaming means that there is an active connection/viewer. But is that really what it means? Or does it mean streaming is enabled, and someone can connect for a live view? I think that is the crux of the issue here. FWIW, I interpreted it as the latter - i.e., state == streaming means streaming is enabled, not that there is necessarily an active viewer. But maybe that was not the original intent.

I'll grant you cameras a quite complex things with many, many individual controls, and there's not necessarily a lot of commonality. Whatever you decide is best is fine.

balloob commented 6 years ago

And just because other camera integrations don't provide this functionality, to me, is not reason enough not to provide a standard infrastructure for setting the camera's mode.

We're not going to include support for things that are not going to be used. Anything we add to Home Assistant is something that we have to support forever.

awarecan commented 6 years ago

Research on different camera support

I assume everyone agreed that turn on/off recording is a must have feature/function for cameras, I just focus on turn on/off streaming (or entire device) feature.

[x] Abode

Beginning the week of October 16th, 2017 you have the ability to turn your IP Cameras ON and OFF via your abode Web and Mobile Apps. You can also control the state of your IP Cameras via abode Automations. 1

[?] Amcrest

From the testings, shutdown acts like "reboot now". 2

[x] Arlo 3

# Turn camera off.
print(arlo.ToggleCamera(basestations[0], cameras[0], False))

[ ] August It is doorbell camera, no means to turn it off.

[ ] Axis That is @Kane610 maintained package, I just assume it doesn't support that feature.

[ ] Blink Only provide arm/disarm, not turn on/off feature.

[ ] Bloomsky That is weather station, seems no means to turn it off.

[x] Canary

Privacy: When you need privacy temporarily, you can turn all cameras at your location off. 4

[?] DoorBird

Users can have several permissions, which can be managed in the Administration area of the DoorBird App. Watch always: User can see live view and control the door/s at any time, even if there is no ring event When the request is correct, but the requesting user has no permission to view the live stream at the moment, the request is answered with a return code 204. This usually happens for users without “watch-always” permission when there was no ring event in the past 5 minutes.5

Not clear if it can be changed through API

[ ] Family Hub Refrigerator camera, seems no streaming at all

[ ] Foscam Legacy brand

[ ] ManyThing

If you only use Manything for live viewing, leaving your camera on standby mode (at the record screen, but not recording) substantially reduces power consumption. Simple activate the camera (by using our remote control functionality) when you want to check in, then turn it off again when you're done. 6

Sounds like they are suggesting a "hack" method to turn off camera.

[x] Neato That is a vacuum, I am assuming there is a turn off feature.

[x] Nest

When you turn off your Nest Cam or Dropcam with the Nest app, all video and audio activity stops. If you have a Nest Aware subscription, video history recording is stopped too. However, your camera will stay connected to power and the Internet so it can be remotely turned on with the Nest app and so that there’s no delay when it switches back on. 7

[x] Netatmo

You can turn the camera off at any time. Just open the top right settings menu from your app to turn surveillance on or off.8

========= pending

[] ONVIF [] Ring [] Skybell [] Synology [] USPS !! Why this guy in Camera Category !! [] UniFi [] Verisure [] Xeoma [] Yi Home [] ZoneMinder

=========

Kane610 commented 6 years ago

It is not that the Axis component doesn't support it, but there is no such concept of turning device/stream off for Axis devices. The closest you can come to stopping someones stream is to set a privacy mask on top of the video.

awarecan commented 6 years ago

Sure is. I am not investigating if current upstream library support on/off but the device itself.

balloob commented 6 years ago

Alright, so I think that's a good enough list to add a new turn_on and turn_off service and supported feature to camera platforms.

Camera entities currently do not have supported features, so turn_on/turn_off will be the first one. A PR is welcome that:

pnbruckner commented 6 years ago

Just a thought. Would it then make sense to rebase the Camera component from Entity to ToggleEntity?

balloob commented 6 years ago

Good idea.

pnbruckner commented 6 years ago

Sorry, maybe stating the obvious, but that would change the possible states from the current set of 'idle', 'streaming' and 'recording' to 'on' and 'off', right? Would this be considered a "breaking change"? (Doesn't matter to me either way, just trying to understand.)

If the possible states would change, then would it also make sense to make 'streaming' and 'recording' (binary) attributes instead, possibly as additional supported_features? If so, I would think it would then make sense for 'streaming' (being true) to mean there was at least one active viewer (assuming, of course, the platform can tell and would support this feature.) That is, if it even makes sense to have such an attribute. Maybe the 'on' state simply replaces the 'streaming' state and that's good enough. (I'm still not sure what the original intent was for the Camera class' is_streaming attribute.)

For 'recording' as an attribute, I would think that would mean the camera itself was recording the stream, assuming again that having such an attribute makes sense.

awarecan commented 6 years ago

I will keep that simple, not change base to ToggleEntity, since not all Camera model support that on/off feature. Base from ToggleEntity means it suppose to have this feature.

@balloob Is there a way to filter "unsupported" entity out the entities dropdown list in Service page? I just notice that my nest camera listed under camera.enable_motion_detection service which doesn't support. If I click call service, nothing show up in the UI, no success or failed notification. Only a ERROR log appears says NotImplementedError

mnoorenberghe commented 6 years ago

[x] ZoneMinder

A ZoneMinder camera can be set to the function "none" (no stream or recording) and it can also be toggled to be "disabled" (I forget the difference) but either way it does support this concept of turning it off.

P.S. I hope the state will continue to have a recording value as I use that for some automations.

yottatsa commented 6 years ago

What about actual representation of the fact that the camera has detected audible noise or motion?

If it started recording, then I could easily set is_recording for short period.

But how to expose the fact that motion were detected and recording is not set? I'm thinking about is_recording anyway, because synology and zoneminder (at least) are doing it this way.

I also could set motion/noise detection details in the state_attributes.

Badbananna commented 1 year ago

I need to reconfigure my Arlo cams for wired data

Rearrange multi display and no home base iOS screen recording if any can help