Skip to content

Initial version of dynamic ONVIF camera discovery with DLS pipeline startup#759

Open
jmotow wants to merge 4 commits intomainfrom
onvif_async_discovery
Open

Initial version of dynamic ONVIF camera discovery with DLS pipeline startup#759
jmotow wants to merge 4 commits intomainfrom
onvif_async_discovery

Conversation

@jmotow
Copy link
Copy Markdown
Contributor

@jmotow jmotow commented Apr 10, 2026

Initial version of dynamic ONVIF camera discovery with DLS pipeline startup

Copilot AI review requested due to automatic review settings April 10, 2026 08:24
Comment thread samples/gstreamer/python/onvif_cameras_discovery/build_whl/setup.py Outdated
Comment thread samples/gstreamer/python/onvif_cameras_discovery/dls_onvif_discovery_engine.py Outdated
Comment thread samples/gstreamer/python/onvif_cameras_discovery/dls_onvif_discovery_thread.py Outdated
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Introduces an asyncio-based ONVIF discovery sample that continuously detects cameras, queries ONVIF media profiles, and starts/stops GStreamer (DL Streamer) pipelines dynamically based on a JSON configuration.

Changes:

  • Replaced the old one-shot discovery/launcher code with an async discovery engine + camera registry + pipeline lifecycle manager.
  • Introduced a new config.json schema (named camera entries with hostname, port, definition) and verbose controls.
  • Added wheel packaging assets and updated documentation for building/running the sample.

Reviewed changes

Copilot reviewed 16 out of 16 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
samples/gstreamer/python/onvif_cameras_discovery/requirements.txt Replace invalid urllib dependency with urllib3
samples/gstreamer/python/onvif_cameras_discovery/misc.py Add helper for printing discovered/removed cameras and profiles in a table
samples/gstreamer/python/onvif_cameras_discovery/dls_onvif_sample.py New asyncio entrypoint wiring the discovery engine + graceful shutdown
samples/gstreamer/python/onvif_cameras_discovery/dls_onvif_discovery_utils.py Remove legacy synchronous utilities (replaced by new engine)
samples/gstreamer/python/onvif_cameras_discovery/dls_onvif_discovery_thread.py Add GStreamer pipeline launcher/stopper using a GLib loop thread
samples/gstreamer/python/onvif_cameras_discovery/dls_onvif_discovery_engine.py Add unified WS-Discovery + ONVIF profile retrieval + async orchestration
samples/gstreamer/python/onvif_cameras_discovery/dls_onvif_data.py Extend ONVIFProfile with camera connection metadata fields/properties
samples/gstreamer/python/onvif_cameras_discovery/dls_onvif_config_manager.py Add config loader/refresh and pipeline lookup by ip:port
samples/gstreamer/python/onvif_cameras_discovery/dls_onvif_camera_entry.py Add registry + entry model to bind cameras, profiles, pipelines, lifecycle state
samples/gstreamer/python/onvif_cameras_discovery/config.json Update config to new structured format with named cameras and verbose flag
samples/gstreamer/python/onvif_cameras_discovery/build_whl/setup.py Add setuptools packaging definition for building a wheel
samples/gstreamer/python/onvif_cameras_discovery/build_whl/pyproject.toml Add build-system metadata for wheel builds
samples/gstreamer/python/onvif_cameras_discovery/build_whl/build_dls_onvif_sample_whl.sh Add helper script to build/uninstall the wheel
samples/gstreamer/python/onvif_cameras_discovery/build_whl/README.md Add instructions for building and validating the wheel
samples/gstreamer/python/onvif_cameras_discovery/build_whl/MANIFEST.in Include README/requirements/config/assets in packaging inputs
samples/gstreamer/python/onvif_cameras_discovery/README.md Rewrite docs to reflect async engine, new config format, and wheel build

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread samples/gstreamer/python/onvif_cameras_discovery/dls_onvif_discovery_thread.py Outdated
Comment thread samples/gstreamer/python/onvif_cameras_discovery/dls_onvif_config_manager.py Outdated
Comment thread samples/gstreamer/python/onvif_cameras_discovery/dls_onvif_config_manager.py Dismissed
Comment thread samples/gstreamer/python/onvif_cameras_discovery/dls_onvif_config_manager.py Dismissed
Comment thread samples/gstreamer/python/onvif_cameras_discovery/dls_onvif_sample.py Outdated
Comment thread samples/gstreamer/python/onvif_cameras_discovery/dls_onvif_discovery_engine.py Outdated
Comment thread samples/gstreamer/python/onvif_cameras_discovery/dls_onvif_discovery_thread.py Outdated
Comment thread samples/gstreamer/python/onvif_cameras_discovery/dls_onvif_discovery_engine.py Outdated
@oonyshch
Copy link
Copy Markdown
Contributor

oonyshch commented Apr 20, 2026

no unit tests are included for the new async discovery engine. given the lifecycle complexity (cameras appearing/disappearing, pipeline start/stop, rollback on failure), consider adding tests for at least: (1) camera discovered triggers pipeline start, (2) camera lost triggers pipeline stop, (3) startup failure triggers _cleanup_on_failed_start correctly. mock-based tests would significantly improve confidence here.

Copy link
Copy Markdown
Contributor

@oonyshch oonyshch left a comment

Choose a reason for hiding this comment

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

overall, good. please take attention before merge:

  • two required ci checks: license header scan and pylint. run pylint locally on all new python files and fix issues.
  • verify license header format matches existing files, also for CI check
  • rebase branch with main.

additional recommendations:

  • add unit tests for the async discovery lifecycle (camera discovered/lost, pipeline start/stop, failure rollback).
  • see inline comments for potential deadlock issue and path traversal security concern.

tbujewsk
tbujewsk previously approved these changes Apr 22, 2026
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.

6 participants