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

Allow birdseye to be overridden at the camera level #3083

Merged

Conversation

NickM-27
Copy link
Sponsor Collaborator

Implementation of #2582

@NickM-27
Copy link
Sponsor Collaborator Author

NickM-27 commented Apr 11, 2022

I think this is pretty simple, just not sure on the docs since the width, height, quality are not overridable at the camera level. Should there just be a note on those ones specifically to say they aren't overridable?

I created the separate BirdseyeCameraConfig object to throw an error if those values are set at the camera level

docs/docs/configuration/index.md Outdated Show resolved Hide resolved
frigate/output.py Outdated Show resolved Hide resolved
frigate/output.py Show resolved Hide resolved
frigate/output.py Outdated Show resolved Hide resolved
frigate/output.py Outdated Show resolved Hide resolved
frigate/output.py Outdated Show resolved Hide resolved
@@ -775,6 +785,7 @@ def runtime_config(self) -> FrigateConfig:
# Global config to propegate down to camera level
global_config = config.dict(
include={
"birdseye": ...,
Copy link
Owner

Choose a reason for hiding this comment

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

If you disable globally and then enable a single camera, does that enable Birdseye for the one camera?

Copy link
Sponsor Collaborator Author

@NickM-27 NickM-27 Apr 12, 2022

Choose a reason for hiding this comment

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

With birdseye disabled globally, there is no option in the sidebar. If I bypass that then it still isn't enabled as the global check to add the converter["birdseye"] doesn't go through. Not sure what the best behavior would be; I could add a warning in the config if it is disabled globally but has camera enabled, or adapt those behaviors to consider the camera level as well, or perhaps it is good as is.

@blakeblackshear
Copy link
Owner

Might be a good idea to add some unit tests for the additional config parsing to make sure it handles things the way we expect.

@NickM-27
Copy link
Sponsor Collaborator Author

Might be a good idea to add some unit tests for the additional config parsing to make sure it handles things the way we expect.

Yeah good idea, I think the two I added should cover the cases but let me know if I missed any 👍

@blakeblackshear blakeblackshear merged commit d749cf2 into blakeblackshear:release-0.11.0 Apr 15, 2022
@NickM-27 NickM-27 deleted the camera-level-birdseye branch April 15, 2022 12:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants