diff --git a/server/src/services/ptzCameraService/index.js b/server/src/services/ptzCameraService/index.js index 75d18959..f6fabf46 100644 --- a/server/src/services/ptzCameraService/index.js +++ b/server/src/services/ptzCameraService/index.js @@ -34,8 +34,6 @@ const SNAPSHOT_STREAM_INTERVAL_MS = 2000; const SPOTLIGHT_VERIFY_DELAY_MS = 1200; const PUBLISHER_STDERR_SYNC_MS = 10000; const PUBLISHER_RTSP_TIMEOUT_US = 10000000; -const REOLINK_API_RETRY_MS = 1000; -const REOLINK_API_WRITE_INTERVAL_MS = 1000; const events = new EventEmitter(); const config = loadConfig(); @@ -74,8 +72,6 @@ const state = { reolinkApi: { connected: false, connecting: false, - retryCount: 0, - retryAt: null, lastError: null, lastConnectedAt: null, lastEvent: 'idle', @@ -83,7 +79,6 @@ const state = { }; let onvifCam = null; -let reolinkClient = null; let reolinkModulePromise = null; let turnTimer = null; let publisherProcess = null; @@ -92,9 +87,6 @@ let publisherStderrSyncTimer = null; let snapshotTimer = null; let spotlightVerifyTimer = null; let vendorStatePromise = Promise.resolve(); -let reolinkApiLogNextAt = 0; -let reolinkWriteInFlight = false; -let lastReolinkWriteAcceptedAt = 0; let lastSnapshotState = null; const snapshotSubscribers = new Map(); const socketSnapshotSubscriptions = new Map(); @@ -133,12 +125,6 @@ function updateReolinkApiState(patch = {}, reason = 'reolink-api') { emitChange(reason); } -function sleep(ms) { - return new Promise((resolve) => { - setTimeout(resolve, ms); - }); -} - function parsePublisherProgressLine(line) { /* ffmpeg's "-progress pipe:2" emits simple key=value telemetry on stderr. @@ -651,10 +637,10 @@ function getErrorMessage(err) { async function closeReolinkClient(client, reason = 'reset') { /* - The Reolink SDK keeps a long-mode login session and may also own background - resources internally. close() is documented as idempotent, so use it as the - preferred cleanup path and ignore cleanup failures because the whole point - of this branch is that the old client/session may already be broken. + Each Reolink operation owns a short-lived long-mode session. close() logs + out and frees any SDK resources; cleanup failures are logged at debug level + because the command result has already been determined by the time finally + cleanup runs. */ if (!client || typeof client.close !== 'function') return; try { @@ -664,18 +650,13 @@ async function closeReolinkClient(client, reason = 'reset') { } } -async function resetReolinkClient(reason = 'reset') { - const previous = reolinkClient; - reolinkClient = null; - await closeReolinkClient(previous, reason); -} - -async function createReolinkClient() { +async function createReolinkClientSession() { /* reolink-nvr-api is published as an ESM-only package. This server is still CommonJS, so a top-level require() fails before the service can even start. - Dynamic import keeps the server bootable and only loads the vendor SDK when - spotlight or IR state is actually queried. + Dynamic import keeps the server bootable while still creating a fresh camera + API client for every command. Avoiding a cached long-lived client keeps one + wedged Reolink session from poisoning later light/IR commands. */ if (!reolinkModulePromise) { reolinkModulePromise = import('reolink-nvr-api'); @@ -690,88 +671,63 @@ async function createReolinkClient() { timeout: 10000, }); await client.login(); - reolinkClient = client; updateReolinkApiState({ connected: true, connecting: false, - retryAt: null, lastError: null, lastConnectedAt: Date.now(), lastEvent: 'connected', }, 'reolink-api-connected'); - return reolinkClient; + return client; } -async function ensureReolinkClient() { - if (reolinkClient) return reolinkClient; +async function callReolinkApi(command, payload = {}) { + /* + Commands should not depend on the previous command's session or observed + state. Open a fresh Reolink session, send exactly the requested API command, + and close it. If the camera rejects or ignores the command, the failure is + allowed to surface to the caller instead of being hidden behind retries that + can make the UI look successful while the physical emitter never changed. + */ + if (!enabled) throw new Error('PTZ camera disabled'); updateReolinkApiState({ connected: false, connecting: true, lastEvent: 'connecting', }, 'reolink-api-connecting'); - return createReolinkClient(); -} - -async function callReolinkApi(command, payload = {}) { - /* - Reolink's HTTP API is stateful in long mode. When the camera restarts or the - SDK session wedges, retrying the same cached client can leave all later - light/IR operations dead until the Node process restarts. This loop treats - any API/login failure as a disposable session, creates a fresh client, and - retries at one fixed cadence until the command succeeds. The caller usually - sits inside serializeVendorState(), so rapid UI toggles stay ordered behind - the reconnecting operation instead of racing multiple login attempts. - */ - while (enabled) { - try { - const client = await ensureReolinkClient(); - const result = await client.api(command, payload); - updateReolinkApiState({ - connected: true, - connecting: false, - retryAt: null, - lastError: null, - lastConnectedAt: Date.now(), - lastEvent: 'api-ok', - }, 'reolink-api-ok'); - return result; - } catch (err) { - const message = getErrorMessage(err); - const retryAt = Date.now() + REOLINK_API_RETRY_MS; - /* - Do not let one bad long-mode session poison the whole service. Clearing - the cached client before the fixed sleep makes the next loop iteration - perform a full login rather than reusing the session that just failed. - */ - await resetReolinkClient(`api-failed:${command}`); - updateReolinkApiState({ - connected: false, - connecting: true, - retryCount: Number(state.reolinkApi?.retryCount || 0) + 1, - retryAt, - lastError: message, - lastEvent: 'retrying', - }, 'reolink-api-retry'); - if (Date.now() >= reolinkApiLogNextAt) { - reolinkApiLogNextAt = Date.now() + 30000; - logger.warn('Reolink API failed; retrying at fixed interval', { - command, - retryMs: REOLINK_API_RETRY_MS, - error: message, - }); - } - await sleep(REOLINK_API_RETRY_MS); - } + let client = null; + try { + client = await createReolinkClientSession(); + const result = await client.api(command, payload); + updateReolinkApiState({ + connected: false, + connecting: false, + lastError: null, + lastConnectedAt: Date.now(), + lastEvent: 'api-ok-closed', + }, 'reolink-api-ok'); + return result; + } catch (err) { + const message = getErrorMessage(err); + updateReolinkApiState({ + connected: false, + connecting: false, + lastError: message, + lastEvent: 'api-error', + }, 'reolink-api-error'); + logger.warn('Reolink API command failed', { command, error: message }); + throw err; + } finally { + await closeReolinkClient(client, `api:${command}`); } - throw new Error('PTZ camera disabled'); } async function refreshVendorState() { if (!enabled) return; /* - Read these sequentially so a failed request can reset and rebuild the SDK - client before the next request starts. Running both in parallel would let - two calls fight over the same broken cached client during reconnect. + Read these sequentially so the camera sees one fresh-session API request at + a time. Parallel reads are not useful here, and avoiding overlap keeps the + vendor API behavior easier to reason about when it is already acting flaky. */ const white = await callReolinkApi('GetWhiteLed', { channel: 0 }); const ir = await callReolinkApi('GetIrLights', { channel: 0 }); @@ -818,47 +774,6 @@ function serializeVendorState(operation) { return vendorStatePromise; } -function reserveReolinkWriteSlot(control) { - /* - Reolink light/IR writes are physical camera API writes, not high-frequency - control signals. Rejecting too-fast requests here, before they enter - serializeVendorState(), is what makes this a true rate limit instead of a - delayed queue. A request either gets the current write slot immediately or - fails immediately; the server never stores skipped toggle states to replay - later. - */ - if (reolinkWriteInFlight) { - const err = new Error('PTZ API rate limited'); - err.code = 'PTZ_REOLINK_RATE_LIMITED'; - err.control = control; - err.retryAfterMs = REOLINK_API_WRITE_INTERVAL_MS; - throw err; - } - - const now = Date.now(); - const elapsed = now - lastReolinkWriteAcceptedAt; - if (elapsed < REOLINK_API_WRITE_INTERVAL_MS) { - const err = new Error('PTZ API rate limited'); - err.code = 'PTZ_REOLINK_RATE_LIMITED'; - err.control = control; - err.retryAfterMs = REOLINK_API_WRITE_INTERVAL_MS - elapsed; - throw err; - } - - reolinkWriteInFlight = true; - lastReolinkWriteAcceptedAt = now; -} - -function releaseReolinkWriteSlot() { - /* - Keep the in-flight flag separate from the timestamp. The flag blocks - overlap while an accepted write is still talking to the camera or waiting - through API reconnect; the timestamp blocks a second accepted write from - starting immediately after the first one finishes. - */ - reolinkWriteInFlight = false; -} - async function initialize() { if (!enabled || state.initialized || state.initializing) return; state.initializing = true; @@ -868,10 +783,8 @@ async function initialize() { /* Reolink light/IR state is useful, but it must not block PTZ startup. The vendor API can be unavailable while ONVIF and RTSP are already healthy; - because callReolinkApi() retries until reconnect, awaiting this refresh - here would keep the publisher and queue disabled until the HTTP API comes - back. Queue it instead so later light/IR operations naturally wait behind - the reconnecting refresh while video startup continues. + awaiting this refresh here would keep the publisher and queue disabled + until the HTTP API responds. Queue it instead so video startup continues. */ serializeVendorState(() => refreshVendorState()).catch((err) => { logger.warn('initial Reolink state refresh failed', { error: getErrorMessage(err) }); @@ -1217,74 +1130,68 @@ async function removePreset(socket, payload = {}) { async function setSpotlight(socket, payload = {}) { requireOperator(socket); - reserveReolinkWriteSlot('spotlight'); return serializeVendorState(async () => { - try { - let current = state.light ? normalizeSpotlightState(state.light) : null; - if (payload.state === undefined && !current) { - /* - Toggle requests need a base state. Normal button paths send an explicit - state, so this read only happens for rare generic toggle callers or - startup races before the initial vendor state has arrived. - */ - current = await refreshSpotlightState(); - } - const logicalOn = payload.state === undefined - ? !isSpotlightOn(current || {}) - : normalizeSpotlightPayloadState(payload.state); - const cameraState = spotlightCameraStateForLogicalOn(logicalOn); - const cameraPayload = { - channel: 0, - state: cameraState, - }; - const next = { - ...(current || {}), - ...cameraPayload, - on: logicalOn, - }; - if (Number.isFinite(Number(payload.bright))) { - const bright = Math.max(0, Math.min(100, Number(payload.bright))); - cameraPayload.bright = bright; - next.bright = bright; - } + let current = state.light ? normalizeSpotlightState(state.light) : null; + if (payload.state === undefined && !current) { /* - The camera accepts a minimal WhiteLed payload and reports success before - GetWhiteLed reflects the new state. Send only the fields we intend to - change, then keep the optimistic state until the delayed verification read - has a real chance to observe the update. + Toggle requests need a base state. Normal button paths send an explicit + state, so this read only happens for rare generic toggle callers or + startup races before the initial vendor state has arrived. */ - state.light = next; - emitChange('light-pending'); - await callReolinkApi('SetWhiteLed', { WhiteLed: cameraPayload }); - scheduleSpotlightVerification(); - return state.light; - } finally { - releaseReolinkWriteSlot(); + current = await refreshSpotlightState(); } + const logicalOn = payload.state === undefined + ? !isSpotlightOn(current || {}) + : normalizeSpotlightPayloadState(payload.state); + const cameraState = spotlightCameraStateForLogicalOn(logicalOn); + const cameraPayload = { + channel: 0, + state: cameraState, + }; + const next = { + ...(current || {}), + ...cameraPayload, + on: logicalOn, + }; + if (Number.isFinite(Number(payload.bright))) { + const bright = Math.max(0, Math.min(100, Number(payload.bright))); + cameraPayload.bright = bright; + next.bright = bright; + } + /* + Send the explicit requested state through a fresh API session before + changing public state. If the camera/API rejects the command, the UI should + not be left showing an optimistic state that never reached the device. + */ + await callReolinkApi('SetWhiteLed', { WhiteLed: cameraPayload }); + state.light = next; + emitChange('light'); + scheduleSpotlightVerification(); + return state.light; }); } async function setIr(socket, payload = {}) { requireOperator(socket); - reserveReolinkWriteSlot('ir'); return serializeVendorState(async () => { - try { - const nextState = normalizeIrState(payload.state); - /* - The camera requires channel inside IrLights. Without it, SetIrLights - returns param error (-4), while the optimistic local state makes the UI - look like the command worked. Keep the optimistic state, but send the - minimal payload the camera actually accepts. - */ - state.ir = { ...(state.ir || {}), channel: 0, state: nextState }; - emitChange('ir-pending'); - await callReolinkApi('SetIrLights', { IrLights: { channel: 0, state: nextState } }); - await refreshVendorState(); - emitChange('ir'); - return state.ir; - } finally { - releaseReolinkWriteSlot(); - } + const nextState = normalizeIrState(payload.state); + /* + The camera requires channel inside IrLights. Without it, SetIrLights + returns param error (-4), while the optimistic local state makes the UI + look like the command worked. Keep the optimistic state, but send the + minimal payload the camera actually accepts. + */ + const next = { ...(state.ir || {}), channel: 0, state: nextState }; + /* + As with spotlight, update session state after the fresh-session command + succeeds so a rejected Reolink request does not make the UI claim the IR + mode changed when the camera never accepted it. + */ + await callReolinkApi('SetIrLights', { IrLights: { channel: 0, state: nextState } }); + state.ir = next; + emitChange('ir'); + await refreshVendorState(); + return state.ir; }); }