From f4d2b3c045bd6bfd31925d0d70be2aeeb249c6f7 Mon Sep 17 00:00:00 2001 From: Nicolas Mowen Date: Tue, 29 Sep 2026 15:56:42 -0600 Subject: [PATCH] Cleanup --- .../settings/camera-detect-scene.spec.ts | 68 ++++++++++++++++++- .../config-form/section-configs/detect.ts | 11 +-- .../sections/section-special-cases.ts | 18 +---- web/src/utils/modelUtil.ts | 29 ++++++-- 4 files changed, 98 insertions(+), 28 deletions(-) diff --git a/web/e2e/specs/settings/camera-detect-scene.spec.ts b/web/e2e/specs/settings/camera-detect-scene.spec.ts index 03ba2bf6ae..df549102ea 100644 --- a/web/e2e/specs/settings/camera-detect-scene.spec.ts +++ b/web/e2e/specs/settings/camera-detect-scene.spec.ts @@ -2,8 +2,8 @@ * Camera detect scene tests -- MEDIUM tier. * * Scenes are free-form names declared by the configured models, so a camera - * only needs to pick one when there is more than one model, and the choices - * are the default scene plus the scenes of those models. + * only needs to pick one when there is a choice to make. The choices are the + * scenes of the configured models, plus a saved scene that no model uses. */ import { readFileSync } from "node:fs"; @@ -26,8 +26,14 @@ const SETTINGS_URL = "/settings?page=cameraDetect&camera=front_door"; async function installRoutes( page: Page, models: { scene: string; devices: string[] }[], + cameraScene?: string, ) { - const config = configFactory({ models } as never); + const config = configFactory({ + models, + ...(cameraScene + ? { cameras: { front_door: { detect: { scene: cameraScene } } } } + : {}), + } as never); await page.route("**/api/config/schema.json", (route) => route.fulfill({ json: CONFIG_SCHEMA }), @@ -70,4 +76,60 @@ test.describe("camera detect scene @medium", () => { const options = frigateApp.page.getByRole("option"); await expect(options).toHaveText(["Default", "thermal"]); }); + + test("a saved scene no model uses stays editable", async ({ frigateApp }) => { + // the camera falls back to the default model, but its saved scene would + // silently take effect if a model for it were added later + await installRoutes( + frigateApp.page, + [{ scene: "default", devices: ["cpu"] }], + "garage", + ); + await frigateApp.goto(SETTINGS_URL); + + await expect(frigateApp.page.locator("#pageRoot")).toContainText( + "Detect scene", + ); + + await frigateApp.page.locator("#root_scene").click(); + await expect(frigateApp.page.getByRole("option")).toHaveText([ + "Default", + "garage", + ]); + }); + + test("default is only offered when a default model exists", async ({ + frigateApp, + }) => { + await installRoutes( + frigateApp.page, + [ + { scene: "thermal", devices: ["cpu"] }, + { scene: "visible", devices: ["openvino:GPU.0"] }, + ], + "thermal", + ); + await frigateApp.goto(SETTINGS_URL); + + await frigateApp.page.locator("#root_scene").click(); + await expect(frigateApp.page.getByRole("option")).toHaveText([ + "thermal", + "visible", + ]); + }); + + test("one model every camera selects needs no choice", async ({ + frigateApp, + }) => { + await installRoutes( + frigateApp.page, + [{ scene: "thermal", devices: ["cpu"] }], + "thermal", + ); + await frigateApp.goto(SETTINGS_URL); + + const root = frigateApp.page.locator("#pageRoot"); + await expect(root).toContainText("Detect FPS"); + await expect(root).not.toContainText("Detect scene"); + }); }); diff --git a/web/src/components/config-form/section-configs/detect.ts b/web/src/components/config-form/section-configs/detect.ts index 33ad525393..6de92a6bd1 100644 --- a/web/src/components/config-form/section-configs/detect.ts +++ b/web/src/components/config-form/section-configs/detect.ts @@ -1,10 +1,11 @@ import type { HiddenFieldContext } from "@/types/configForm"; -import { DEFAULT_SCENE, getModelScenes } from "@/utils/modelUtil"; +import { DEFAULT_SCENE, getSceneChoices } from "@/utils/modelUtil"; import type { SectionConfigOverrides } from "./types"; -// picking a scene only means something once there is more than one model -const hideSceneWithOneModel = ({ fullConfig }: HiddenFieldContext): string[] => - getModelScenes(fullConfig).length > 1 ? [] : ["scene"]; +// picking a scene only means something when there is more than one choice, +// which includes a saved scene that no model uses +const hideSceneWithoutChoice = (ctx: HiddenFieldContext): string[] => + getSceneChoices(ctx).length > 1 ? [] : ["scene"]; const detect: SectionConfigOverrides = { base: { @@ -209,7 +210,7 @@ const detect: SectionConfigOverrides = { }, }, }, - hiddenFields: ["enabled_in_config", hideSceneWithOneModel], + hiddenFields: ["enabled_in_config", hideSceneWithoutChoice], advancedFields: [ "min_initialized", "max_disappeared", diff --git a/web/src/components/config-form/sections/section-special-cases.ts b/web/src/components/config-form/sections/section-special-cases.ts index 23155aed0f..77d24f045b 100644 --- a/web/src/components/config-form/sections/section-special-cases.ts +++ b/web/src/components/config-form/sections/section-special-cases.ts @@ -12,7 +12,7 @@ import { applySchemaDefaults } from "@/lib/config-schema"; import { isJsonObject } from "@/lib/utils"; import { HiddenFieldContext, JsonObject, JsonValue } from "@/types/configForm"; import { getEffectiveAttributeLabels } from "@/utils/configUtil"; -import { getModelScenes } from "@/utils/modelUtil"; +import { getSceneChoices } from "@/utils/modelUtil"; /** * Sections that require special handling at the global level. @@ -40,7 +40,7 @@ export function isSpecialCaseSection( * * - genai: Inject a default provider value on the additionalProperties shape. * - detect: Scenes are free-form names, so offer the configured model scenes - * as the choices for `scene`. + * (and a saved scene no model uses) as the choices for `scene`. * - objects: Promote tracked attribute labels (face, license_plate, courier * logos) from `filters.additionalProperties` to explicit * `filters.properties.` entries with a restricted FilterConfig @@ -104,23 +104,11 @@ function modifyDetectSchema( if (!ctx || !properties?.scene) return schema; - // keep a saved scene that no model uses selectable, so the form doesn't - // silently swap it for another value - const saved = - (ctx.level !== "global" - ? ctx.fullCameraConfig?.detect?.scene - : undefined) ?? ctx.fullConfig.detect?.scene; - const scenes = getModelScenes(ctx.fullConfig); - - if (saved && !scenes.includes(saved)) { - scenes.push(saved); - } - return { ...schema, properties: { ...properties, - scene: { ...properties.scene, enum: scenes }, + scene: { ...properties.scene, enum: getSceneChoices(ctx) }, }, }; } diff --git a/web/src/utils/modelUtil.ts b/web/src/utils/modelUtil.ts index 9a59e4354c..b608d2ae23 100644 --- a/web/src/utils/modelUtil.ts +++ b/web/src/utils/modelUtil.ts @@ -1,4 +1,5 @@ import type { TFunction } from "i18next"; +import type { HiddenFieldContext } from "@/types/configForm"; import { DetectionModelConfig, FrigateConfig } from "@/types/frigateConfig"; /** The scene of the model used by cameras that don't name one. */ @@ -13,11 +14,29 @@ export function getSceneLabel(t: TFunction, scene: string | undefined): string { return scene; } -/** The distinct scenes of the configured models, default first. */ -export function getModelScenes(config?: FrigateConfig): string[] { - const scenes = new Set([DEFAULT_SCENE]); - config?.models?.forEach((model) => scenes.add(model.scene || DEFAULT_SCENE)); - return [...scenes]; +/** + * The scenes a detect section can choose from: those of the configured models, + * default first when a default model exists, plus the saved scene when no model + * uses it, so it can still be seen and changed. + */ +export function getSceneChoices( + ctx: Pick, +): string[] { + const scenes = [ + ...new Set( + ctx.fullConfig.models?.map((model) => model.scene || DEFAULT_SCENE), + ), + ].sort((a, b) => Number(b === DEFAULT_SCENE) - Number(a === DEFAULT_SCENE)); + const saved = + (ctx.level !== "global" + ? ctx.fullCameraConfig?.detect?.scene + : undefined) ?? ctx.fullConfig.detect?.scene; + + if (saved && !scenes.includes(saved)) { + scenes.push(saved); + } + + return scenes; } /**