[Jakub Wlodek (jwlodek)]

• Use CIRC_NO_OVERWRITE mode, and keep track of frames pending in circular buffer. Also track dropped frames
• Remove defaults

[Copilot]

🟡 Changes recommended

Error and synchronization paths can leave frame-buffer state invalid or discard valid EOF notifications.

3 open findings

• Handle setup failure before using uninitialized frameBufferSize · New
• Avoid discarding pending frame callbacks during dequeue recovery · New
• Abort acquisition when unlocking the oldest frame fails · New

What changed in this PR

Updates continuous acquisition to preserve buffered frames and expose queue diagnostics/configuration.

Changes:

• Uses non-overwriting circular buffers and retrieves the oldest frame.
• Tracks pending and dropped frames.
• Adds configurable buffer sizing and unique-ID sources.

File	Description
kinetixApp/​src/​ADKinetixDS.h	Adds pending-frame state.
kinetixApp/​src/​ADKinetix.h	Defines new parameters and unique-ID options.
kinetixApp/​src/​ADKinetix.cpp	Implements buffered frame retrieval and diagnostics.
kinetixApp/​Db/​ADKinetix.template	Adds corresponding EPICS records.
kinetixApp/​Db/​ADKinetix_settings.req	Persists new configurable settings.

🧠 Review effort: Balanced

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🔵 Needs a closer look

Failed dequeues can erase concurrently delivered EOF notifications and strand buffered frames.

3 open findings

• Handle setup failure before using uninitialized frameBufferSize
• Abort acquisition when unlocking the oldest frame fails
• Avoid discarding pending frame callbacks during dequeue recovery

🧠 Review effort: Balanced

[Copilot]

🟡 Changes recommended

Concurrent stop/start requests can create overlapping acquisition workers and compromise shared buffer ownership.

4 open findings

• Prevent new acquisitions until the previous worker has joined · New
• Handle setup failure before using uninitialized frameBufferSize
• Abort acquisition when unlocking the oldest frame fails
• Avoid discarding pending frame callbacks during dequeue recovery

🧠 Review effort: Balanced

[Copilot]

🔵 Needs a closer look

Frame-number arithmetic can overflow, and startup failures do not retain an error status.

4 open findings

• Prevent new acquisitions until the previous worker has joined
• Handle setup failure before using uninitialized frameBufferSize
• Abort acquisition when unlocking the oldest frame fails
• Avoid discarding pending frame callbacks during dequeue recovery

Previously missed (2)

In code that hasn't changed since last review

Report allocation failures before publishing acquisition status

kinetixApp/​src/​ADKinetix.cpp:974

When buffer allocation or frame-info creation fails, execution still publishes ADStatusWaiting or ADStatusAcquire, skips the acquisition loop, and ends with ADStatusIdle. This hides the startup failure from status consumers. Clean up and return with ADStatusError before publishing an acquiring status.

Prevent signed overflow at INT32_MAX frame number

kinetixApp/​src/​ADKinetix.cpp:1009

If a continuous acquisition reaches INT32_MAX, the next comparison evaluates lastFrameNr + 1 with signed 32-bit arithmetic, causing undefined behavior. Promote the addition to 64 bits to keep this comparison safe at the frame-number boundary.

🧠 Review effort: Balanced

[Copilot]

🟡 Changes recommended

Frame metadata can diverge from retrieved pixels, and unconditional per-frame logging can block readout.

6 open findings

• Prevent new acquisitions until the previous worker has joined
• Handle setup failure before using uninitialized frameBufferSize
• Remove unconditional per-frame printf from the acquisition path · New
• Use retrieved frame numbers for gap detection and unique IDs · New
• Abort acquisition when unlocking the oldest frame fails
• Avoid discarding pending frame callbacks during dequeue recovery

🧠 Review effort: Balanced

[Copilot]

🔵 Needs a closer look

Buffer-allocation errors are hidden from status clients, and unconditional per-frame console output can stall acquisition.

6 open findings

• Prevent new acquisitions until the previous worker has joined
• Handle setup failure before using uninitialized frameBufferSize
• Use retrieved frame numbers for gap detection and unique IDs
• Remove unconditional per-frame printf from the acquisition path
• Abort acquisition when unlocking the oldest frame fails
• Avoid discarding pending frame callbacks during dequeue recovery

Previously missed (2)

In code that hasn't changed since last review

Startup failure status is overwritten before publication

kinetixApp/​src/​ADKinetix.cpp:962

For an invalid circular-buffer size or allocation failure, this error status is overwritten by the Waiting/Acquire assignment below before any parameter callback publishes it. Cleanup then reports Idle, so PV clients see a normal completion with zero frames rather than a startup failure. Clear ADAcquire, publish ADStatusError, and release the port lock before returning from this failure path.

Per-frame stdout logging stalls readout under locks

kinetixApp/​src/​ADKinetix.cpp:1014

This writes to stdout for every continuous frame while both the port lock and the oldest buffer slot are held. A slow or blocked IOC console can stall readout and prevent timely slot release, and disabling asyn tracing does not disable this output. Use the existing trace-controlled DEBUG_ARGS logging instead.

🧠 Review effort: Balanced

[Copilot]

🔵 Needs a closer look

Buffer-size and allocation failures overwrite the error status before clients receive it.

5 open findings

• Prevent new acquisitions until the previous worker has joined
• Handle setup failure before using uninitialized frameBufferSize
• Use retrieved frame numbers for gap detection and unique IDs
• Abort acquisition when unlocking the oldest frame fails
• Avoid discarding pending frame callbacks during dequeue recovery

1 resolved since last review

• Remove unconditional per-frame printf from the acquisition path

Previously missed (1)

In code that hasn't changed since last review

Failed buffer allocation status is overwritten before publication

kinetixApp/​src/​ADKinetix.cpp:962

If the configured buffer exceeds UINT32_MAX bytes or calloc fails, ADStatusError is overwritten by Acquire/Waiting before callParamCallbacks(), then by Idle during cleanup. Clients therefore never receive the failed-start status. Clear ADAcquire, publish the error, release the port lock, and return from this branch; neither the frame buffer nor FRAME_INFO has been allocated here.

🧠 Review effort: Balanced