Add option enabled for each camera in config #4162

banthungprong · 2022-10-25T05:48:49Z

Based on requirement and discussion in #4109:

for each camera an option "enabled: True/False" can be used
if option isn't used in config file the camera is enable as default (so no changes in the old config are necessary

Corrected 2 typos in config.py (propagate, generate)

Adding the if block resulted in 91 deletions and 96 additions after changing the indentation. The only real changes were:

in config.py in class CameraConfig
enabled: Optional[bool] = Field(default=True, title="Enable camera.")
in config.py in class FrigateConfig in runtime_config:
replace
for name, camera in config.cameras.items():
with
for name, camera in config.cameras.copy().items():
... generate camera_config ...
if camera_config.enabled:
... configurations ...
else: config.cameras.pop(name)
befor line
return config

frigate/config.py

NickM-27 · 2022-10-25T11:47:04Z

Also need to update configuration docs to reflect this new option

banthungprong · 2022-10-25T12:15:03Z

Docs Update will follow if the reviews here are ok. I don't want to update the doc before the solution seems to be ok.
I can add the documenation update as a commit to this branch later or not?
If it is better to change something in code AND in doc at the same time I will do from now on.

NickM-27 · 2022-10-25T12:16:42Z

Yes, docs and code updates should always be part of the same PR

NickM-27 · 2022-10-25T12:37:56Z

frigate/config.py

@@ -530,6 +530,7 @@ class CameraUiConfig(FrigateBaseModel):

 class CameraConfig(FrigateBaseModel):
    name: Optional[str] = Field(title="Camera name.", regex="^[a-zA-Z0-9_-]+$")
+    enabled: Optional[bool] = Field(default=True, title="Enable camera.")


Suggested change

enabled: Optional[bool] = Field(default=True, title="Enable camera.")

enabled: bool = Field(default=True, title="Enable camera.")

You are using the Optional qualifier which means the field is nullable, but it shouldn't be. This will always have a default value of True or a user set value.

Ok. Simply misunderstood the Optional.

banthungprong · 2022-10-25T14:54:10Z

Code changes as requested and updated documentation for the new option

NickM-27

Looks good, just one small thing

docs/docs/configuration/index.md

blakeblackshear · 2022-10-26T00:38:33Z

frigate/config.py

            merged_config = deep_merge(camera.dict(exclude_unset=True), global_config)
            camera_config: CameraConfig = CameraConfig.parse_obj(
                {"name": name, **merged_config}
            )

+            if not camera_config.enabled:
+                config.cameras.pop(name)


I haven't tested this, but won't this essentially just prevent the camera's config from being loaded at all? I thought the goal was to make it possible to have retain settings for a disabled camera that are different than the default values.

Yeah you're right, I wasn't sure if that was supposed to be in this initial pass.

@banthungprong In that case we'd want to leave the config itself alone and not run the ffmpeg commands if the camera is disabled.

I think we talk about 3 different requirements:

static: Option in config, only used one time when starting the container (the original requirement for this PR). Camera settings aren't loaded in config.py.

dynamic: Camera can be disabled or enabled while running frigate, so the config is changed in the UI. State of the camera will be saved "somewhere else" but not in the config. Settings are always available.

"A bit of both": Option in config, when starting frigate the settings are loaded but the views (recording/events/live/rtmp etc) do not show a disabled camera. But additionaly features in the UI to enable/disable every camera. Setting are always available.

Is it enough to "not run the ffmpeg commands" or is more logic necessary in the different views when a camera isn't shown (example: Events View, dropdown Filter)? I will start digging and trying here today.

Close this pull request and make a new one based on requirement 2 or 3?
Or update this one and switch to the dynamic solution?

If starting to change parameters dynamically (enabled/disabled is only a first possibility) shouldn't there be a view for configurations showing each camera with a list of parameters that might be changed in the UI (resolution, framerate, etc.)?

The dynamic solution should come later, that's not what's being suggested.

This solution should still be static. The thing that's being discussed is we don't want the cameras config completely removed as we still want the retention settings to be applied. If a camera is disabled it just shouldn't be running, it should still show in the dropdowns and other places because you may still want to view those events

Close this pull request and make a new one based on requirement 2 or 3?
Or update this one and switch to the dynamic solution?

I don't think there's a reason to close, but again we don't need the dynamic solution implemented. Just static solution that maintains camera settings in the config

Thanks! I started looking already in app.py. And "some other places". I will try it and I hope it won't get too annoying if my commits aren't perfect...

Not starting the process when the camera is disabled is working already.
Birdseye, Events, Debug all ok.
In Cameras: I hate the green image when no camera snapshot/stream is available...

In Cameras: I hate the green image when no camera snapshot/stream is available...

That is on the list to improve in the 0.12.0 release, so I'd say leave it with the green screens for now as that is a separate PR which will address the green screen. Maybe having one message for cameras with errors and a different message for disabled cameras.

I do not start camera processors and capture processes when a camera is disabled. Working. Will commit in the morning. Too tired now.

Naming looks strange:
start_camera_processors vs start_capture_processes
whereas inside camera_process and capture_process looks ok.

That is on the list to improve in the 0.12.0 release, so I'd say leave it with the green screens for now as that is a separate PR which will address the green screen. Maybe having one message for cameras with errors and a different message for disabled cameras.

I couldn't resist:

NickM-27

Just some small stuff

frigate/app.py

NickM-27 · 2022-10-27T11:42:17Z

docs/docs/configuration/index.md

@@ -380,6 +380,10 @@ timestamp_style:
 cameras:
  # Required: name of the camera
  back:
+    # Optional: Enable/Disable the camera (default: shown below).
+    #           If disabled: config is used but no live stream and no capture etc.


Indentation is incorrect for these (# is correct but doesn't need space after)

I do not understand this. Formatting is the same like other comment blocks with the Optional.
# in the same columns like the next line "enabled : True
and after # the same blanks like a few lines above

NickM-27 · 2022-10-27T11:43:58Z

docs/docs/guides/getting_started.md

@@ -62,6 +62,7 @@ detectors:

 cameras:
  camera_1: # <------ Name the camera
+    enabled: True # <----- Enable/disable camera


I'm not sure if it makes sense to put it here, this section is the minimal config and this isn't necessary

frigate/app.py

NickM-27 · 2022-10-27T11:44:46Z

frigate/config.py

@@ -816,7 +817,7 @@ def runtime_config(self) -> FrigateConfig:
            exclude_unset=True,
        )

-        for name, camera in config.cameras.items():
+        for name, camera in config.cameras.copy().items():


Don't need the copy here anymore

netlify · 2022-10-30T08:36:39Z

✅ Deploy Preview for frigate-docs ready!

Name	Link
🔨 Latest commit	`2fa0316`
🔍 Latest deploy log	https://app.netlify.com/sites/frigate-docs/deploys/635eade705859300083748b8
😎 Deploy Preview	https://deploy-preview-4162--frigate-docs.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

banthungprong · 2022-10-30T08:46:56Z

Commit comment length was limited (I just changed that in VS Code). So here a summary of the changes:

changed the if-logic as reqested (2x) in app.py
removed obsolete copy() in config.py
removed enabled option in minimal camera configuration in getting_started.md
added info in Cameras View when a camera is disabled and avoid green image in CameraImage.jsx

I didn't change the "wrong indentation" because I simply do not understand what might be wrong.

NickM-27 · 2022-10-30T15:28:17Z

docs/docs/configuration/index.md

+    # Optional: Enable/Disable the camera (default: shown below).
+    #           If disabled: config is used but no live stream and no capture etc.
+    #           Events/Recordings are still viewable.


Suggested change

# Optional: Enable/Disable the camera (default: shown below).

# If disabled: config is used but no live stream and no capture etc.

# Events/Recordings are still viewable.

# Optional: Enable/Disable the camera (default: shown below).

# If disabled: config is used but no live stream and no capture etc.

# Events/Recordings are still viewable.

this is what I mean, the start of the comment is indented too much

this still needs to be addressed

web/src/components/CameraImage.jsx

NickM-27 · 2022-10-30T16:42:39Z

docs/docs/configuration/index.md

+    # Optional: Enable/Disable the camera (default: shown below).
+    #           If disabled: config is used but no live stream and no capture etc.
+    #           Events/Recordings are still viewable.


this still needs to be addressed

banthungprong · 2022-10-30T17:02:53Z

A few lines above the indentation was used in the same way. It's not really consistent in the complete file.
But thanks for the replies and reviews. It's really helpful and nice to work like this.

NickM-27 · 2022-10-30T17:07:10Z

A few lines above the indentation was used in the same way. It's not really consistent in the complete file.
But thanks for the replies and reviews. It's really helpful and nice to work like this.

The ones that are indented start with WARNING: or NOTE: and are one statement. General descriptions around how the settings work should not be indented.

* add option enabled for each camera in config * Simplified If-block and removed wrong Optional * Update Docs enabling/disabling camera in config * correct format for option * Disabling Camera for processes, no config changes * Describe effects of disabled cam in documentation * change if-logic, obsolete copy, info disabled cam * changed color to white, added top padding in disabled camera info * changed indentation

Zaschii · 2023-04-03T09:14:26Z

Is there also the possibility to disable a cam per mqtt?
Because i have a webcam which is not always online - so how can i automate this (changing the config file per script is not really a smooth solution in my mind). Thx & BR

TheSleepySlee · 2023-04-04T19:22:17Z

This works perfectly, thanks for this new option!

add option enabled for each camera in config

1fe6236

banthungprong changed the title ~~add option enabled for each camera in config~~ Add option enabled for each camera in config Oct 25, 2022

NickM-27 requested changes Oct 25, 2022

View reviewed changes

frigate/config.py Outdated Show resolved Hide resolved

frigate/config.py Outdated Show resolved Hide resolved

NickM-27 reviewed Oct 25, 2022

View reviewed changes

banthungprong added 2 commits October 25, 2022 14:58

Simplified If-block and removed wrong Optional

52ac65a

Update Docs enabling/disabling camera in config

9d86f94

banthungprong requested a review from NickM-27 October 25, 2022 14:54

NickM-27 requested changes Oct 25, 2022

View reviewed changes

docs/docs/configuration/index.md Outdated Show resolved Hide resolved

correct format for option

4c78b49

banthungprong requested a review from NickM-27 October 25, 2022 15:18

NickM-27 approved these changes Oct 25, 2022

View reviewed changes

blakeblackshear reviewed Oct 26, 2022

View reviewed changes

banthungprong added 2 commits October 27, 2022 06:34

Disabling Camera for processes, no config changes

79548ff

Describe effects of disabled cam in documentation

ffbfc9f

NickM-27 requested changes Oct 27, 2022

View reviewed changes

change if-logic, obsolete copy, info disabled cam

cb88ff1

banthungprong requested a review from NickM-27 October 30, 2022 15:16

NickM-27 requested changes Oct 30, 2022

View reviewed changes

changed color to white, added top padding in disabled camera info

d05266f

banthungprong requested a review from NickM-27 October 30, 2022 16:28

NickM-27 requested changes Oct 30, 2022

View reviewed changes

changed indentation

2fa0316

banthungprong requested a review from NickM-27 October 30, 2022 17:03

NickM-27 approved these changes Oct 30, 2022

View reviewed changes

blakeblackshear merged commit 8163afc into blakeblackshear:dev Nov 2, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add option enabled for each camera in config #4162

Add option enabled for each camera in config #4162

banthungprong commented Oct 25, 2022

NickM-27 commented Oct 25, 2022

banthungprong commented Oct 25, 2022

NickM-27 commented Oct 25, 2022

NickM-27 Oct 25, 2022 •

edited

Loading

banthungprong Oct 25, 2022

banthungprong commented Oct 25, 2022

NickM-27 left a comment

blakeblackshear Oct 26, 2022

NickM-27 Oct 26, 2022 •

edited

Loading

banthungprong Oct 26, 2022

NickM-27 Oct 26, 2022

NickM-27 Oct 26, 2022

banthungprong Oct 26, 2022

banthungprong Oct 26, 2022

NickM-27 Oct 26, 2022

banthungprong Oct 26, 2022

banthungprong Oct 27, 2022

NickM-27 left a comment

NickM-27 Oct 27, 2022

banthungprong Oct 27, 2022 •

edited

Loading

NickM-27 Oct 27, 2022

NickM-27 Oct 27, 2022

netlify bot commented Oct 30, 2022 •

edited

Loading

banthungprong commented Oct 30, 2022 •

edited

Loading

NickM-27 Oct 30, 2022

NickM-27 Oct 30, 2022

NickM-27 Oct 30, 2022

banthungprong commented Oct 30, 2022 •

edited

Loading

NickM-27 commented Oct 30, 2022

Zaschii commented Apr 3, 2023

TheSleepySlee commented Apr 4, 2023

	enabled: Optional[bool] = Field(default=True, title="Enable camera.")
	enabled: bool = Field(default=True, title="Enable camera.")

Add option enabled for each camera in config #4162

Add option enabled for each camera in config #4162

Conversation

banthungprong commented Oct 25, 2022

NickM-27 commented Oct 25, 2022

banthungprong commented Oct 25, 2022

NickM-27 commented Oct 25, 2022

NickM-27 Oct 25, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

banthungprong commented Oct 25, 2022

NickM-27 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NickM-27 Oct 26, 2022 • edited Loading

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

NickM-27 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

banthungprong Oct 27, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

netlify bot commented Oct 30, 2022 • edited Loading

✅ Deploy Preview for frigate-docs ready!

banthungprong commented Oct 30, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

banthungprong commented Oct 30, 2022 • edited Loading

NickM-27 commented Oct 30, 2022

Zaschii commented Apr 3, 2023

TheSleepySlee commented Apr 4, 2023

NickM-27 Oct 25, 2022 •

edited

Loading

NickM-27 Oct 26, 2022 •

edited

Loading

banthungprong Oct 27, 2022 •

edited

Loading

netlify bot commented Oct 30, 2022 •

edited

Loading

banthungprong commented Oct 30, 2022 •

edited

Loading

banthungprong commented Oct 30, 2022 •

edited

Loading