Skip to content

Upgrade Gentl backend cti discovery#51

Merged
C-Achard merged 8 commits intocy/update-installfrom
cy/upgrade-gentl-discovery
Feb 25, 2026
Merged

Upgrade Gentl backend cti discovery#51
C-Achard merged 8 commits intocy/update-installfrom
cy/upgrade-gentl-discovery

Conversation

@C-Achard
Copy link

Upgrades the Gentl backend cti discovery mechanisms to be more use-friendly and avoid relying on hard-coded "best guess" paths.


This pull request significantly refactors and improves the GenTL camera backend, focusing on more robust and flexible GenTL producer (.cti) file discovery and loading. The changes enhance error handling, diagnostics, and compatibility with multiple producers, while also simplifying and clarifying the code structure for device selection and initialization.

The most important changes are:

GenTL Producer (.cti) Discovery and Loading:

  • Added _resolve_cti_files_for_settings and _build_harvester_for_discovery methods to robustly discover and load all available GenTL producer files, supporting explicit user configuration, environment variables, and fallback patterns. Multiple producers are now supported without error, and detailed diagnostics are provided on failure. [1] [2]
  • Updated the open method to use the new CTI discovery logic, track which producers were loaded or failed, and persist this information for UI/debugging. Improved error messages when no producers or devices are found.

Device Selection and Initialization:

  • Refactored device selection logic for clarity: stable device_id matching, legacy serial fallback, and index fallback are now clearly separated and commented. [1] [2] [3]
  • Improved device metadata persistence and UI feedback, storing richer device information and clarifying key names. [1] [2] [3] [4]

Device Discovery Improvements:

  • Updated discover_devices to use the new harvester/CTI discovery logic, supporting multiple producers and providing better progress feedback and error handling. [1] [2]

General Code Cleanup and Clarification:

  • Removed unused imports and legacy code, clarified comments, and streamlined helper functions for better readability and maintainability. [1] [2] [3] [4] [5] [6]

Introduce a dedicated gentl_discovery utility to locate .cti GenTL producer files (from explicit files, env vars, glob patterns and extra dirs) and add selection helpers. Refactor GenTLCameraBackend to use the new discovery logic: resolve multiple CTIs, load all viable producers, persist CTI diagnostics into settings (cti_files, cti_files_loaded, cti_files_failed), and provide robust class-level Harvester building for discovery/quick_ping/get_device_count/rebind_settings. Remove legacy glob/search helpers and change behavior to prefer explicit user config while failing only when no producers can be found/loaded. Update tests and fixtures to use tmp_path-created dummy .cti files, adjust patched Harvester setup, and add unit tests covering CTI discovery and selection policies (including newest/raise-if-multiple behavior).
@C-Achard C-Achard requested a review from Copilot February 17, 2026 10:04
@C-Achard C-Achard self-assigned this Feb 17, 2026
@C-Achard C-Achard added enhancement New feature or request camera Related to cameras and camera backends labels Feb 17, 2026
Copy link

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.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Copy link

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

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

Comments suppressed due to low confidence (1)

tests/cameras/backends/test_gentl_backend.py:258

  • This test is intended to validate behavior when cti_file is provided via legacy top-level properties, but it only asserts that ns["cti_file"] is non-empty. With the current settings factory defaulting properties.gentl.cti_file, the backend will likely use the namespace CTI instead of the top-level one, so the test may pass without covering the intended scenario. Consider asserting that the persisted CTI path matches the from-props.cti path (or is present in cti_files_loaded).
    cti = tmp_path / "from-props.cti"
    cti.write_text("dummy", encoding="utf-8")
    settings = gentl_settings_factory(properties={"cti_file": str(cti), "gentl": {}})
    be = gb.GenTLCameraBackend(settings)
    be.open()

    ns = settings.properties["gentl"]
    assert isinstance(ns.get("cti_file"), str) and ns["cti_file"]


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

Copy link

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

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

Comments suppressed due to low confidence (1)

dlclivegui/cameras/backends/gentl_backend.py:590

  • If a RuntimeError is raised during device selection (lines 463-465, 493, 496, 502), the open() method exits without cleaning up the self._harvester that was created at line 350. This leaves the harvester allocated with CTI producers loaded but never released. The same issue occurs if any exception is raised during acquirer creation (lines 512-521) or during configuration (lines 523-590). Consider wrapping the entire device selection and configuration section in a try-except block that calls self._harvester.reset() and sets self._harvester = None before re-raising any exception.
                            selected_serial = sub[0][1] or None
                        elif len(sub) > 1:
                            candidates = [sn for _, sn in sub]
                            raise RuntimeError(
                                f"Ambiguous GenTL serial match for '{serial_target}'. Candidates: {candidates}"
                            )

        # Legacy serial selection fallback
        if selected_index is None:
            serial = self._serial_number
            if serial:
                serial = str(serial).strip()
                exact = []
                for idx, info in enumerate(infos):
                    sn = _info_get(info, "serial_number", "")
                    sn = str(sn).strip() if sn is not None else ""
                    if sn == serial:
                        exact.append((idx, sn))
                if exact:
                    selected_index = exact[0][0]
                    selected_serial = exact[0][1]
                else:
                    sub = []
                    for idx, info in enumerate(infos):
                        sn = _info_get(info, "serial_number", "")
                        sn = str(sn).strip() if sn is not None else ""
                        if serial and serial in sn:
                            sub.append((idx, sn))
                    if len(sub) == 1:
                        selected_index = sub[0][0]
                        selected_serial = sub[0][1] or None
                    elif len(sub) > 1:
                        candidates = [sn for _, sn in sub]
                        raise RuntimeError(f"Ambiguous GenTL serial match for '{serial}'. Candidates: {candidates}")
                    else:
                        available = [str(_info_get(i, "serial_number", "")).strip() for i in infos]
                        raise RuntimeError(f"Camera with serial '{serial}' not found. Available cameras: {available}")

        # Index fallback
        if selected_index is None:
            device_count = len(infos)
            if requested_index < 0 or requested_index >= device_count:
                raise RuntimeError(f"Camera index {requested_index} out of range for {device_count} GenTL device(s)")
            selected_index = requested_index
            sn = _info_get(infos[selected_index], "serial_number", "")
            selected_serial = str(sn).strip() if sn else None

        # Update settings.index to actual selected index (UI stability)
        self.settings.index = int(selected_index)
        selected_info = infos[int(selected_index)]

        # Create ImageAcquirer via Harvester.create(...)
        try:
            if selected_serial:
                self._acquirer = self._harvester.create({"serial_number": str(selected_serial)})
            else:
                self._acquirer = self._harvester.create(int(selected_index))
        except TypeError:
            if selected_serial:
                self._acquirer = self._harvester.create({"serial_number": str(selected_serial)})
            else:
                self._acquirer = self._harvester.create(index=int(selected_index))

        remote = self._acquirer.remote_device
        node_map = remote.node_map

        self._device_label = self._resolve_device_label(node_map)

        # Apply configuration
        self._configure_pixel_format(node_map)
        self._configure_resolution(node_map)
        self._configure_exposure(node_map)
        self._configure_gain(node_map)
        self._configure_frame_rate(node_map)

        # Read back telemetry
        try:
            self._actual_width = int(node_map.Width.value)
            self._actual_height = int(node_map.Height.value)
        except Exception:
            pass

        try:
            self._actual_fps = float(node_map.ResultingFrameRate.value)
        except Exception:
            self._actual_fps = None

        try:
            self._actual_exposure = float(node_map.ExposureTime.value)
        except Exception:
            self._actual_exposure = None

        try:
            self._actual_gain = float(node_map.Gain.value)
        except Exception:
            self._actual_gain = None

        # Persist identity + metadata
        computed_id = None
        try:
            computed_id = self._device_id_from_info(selected_info)
        except Exception:
            computed_id = None

        if computed_id:
            ns["device_id"] = computed_id
        elif selected_serial:
            ns["device_id"] = f"serial:{selected_serial}"

        if selected_serial:
            ns["serial_number"] = str(selected_serial)
            ns["device_serial_number"] = str(selected_serial)

        if self._device_label:
            ns["device_name"] = str(self._device_label)

        ns["device_display_name"] = str(_info_get(selected_info, "display_name", "") or "")
        ns["device_info_id"] = str(_info_get(selected_info, "id_", "") or "")
        ns["device_vendor"] = str(_info_get(selected_info, "vendor", "") or "")
        ns["device_model"] = str(_info_get(selected_info, "model", "") or "")
        ns["device_tl_type"] = str(_info_get(selected_info, "tl_type", "") or "")
        ns["device_user_defined_name"] = str(_info_get(selected_info, "user_defined_name", "") or "")
        ns["device_version"] = str(_info_get(selected_info, "version", "") or "")
        ns["device_access_status"] = _info_get(selected_info, "access_status", None)

        # Start acquisition unless fast_start
        if getattr(self, "_fast_start", False):
            LOG.info("GenTL open() in fast_start probe mode: acquisition not started.")
            return

        self._acquirer.start()

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

@deruyter92 deruyter92 self-requested a review February 19, 2026 14:19
Add a _reset_harvester helper on GenTLCameraBackend and call it where the harvester must be torn down on failure to ensure proper cleanup. Enhance FakeHarvester to support deterministic add_file failures and to record reset/add/update/create calls; keep create_image_acquirer for compatibility. Update test fixtures (conftest) to expose gentl_fail_add_file_for control, inject a dummy CTI only when none are explicitly provided, and expose gb.fail_add_file_for from patch_gentl_sdk. Add test helpers and new tests to isolate GENICAM env vars and verify CTI load diagnostics for all-success, partial-failure, and complete-failure scenarios. Also adjust some discovery tests to explicitly isolate the environment and tighten assertions.
Copy link

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

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.


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

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@C-Achard C-Achard marked this pull request as ready for review February 19, 2026 16:23
Copy link

@deruyter92 deruyter92 left a comment

Choose a reason for hiding this comment

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

Took me some time sorry. See one remaining question on the persistence of auto-discovered CTIs. But looks good to me!

Introduce a cti_files_source marker (auto/user) and wiring to track whether CTI file lists were user-specified or auto-discovered. Treat legacy top-level properties.cti_file(s) as strict user overrides; treat properties.gentl.cti_file(s) as either an auto-cache (falls back to discovery if stale) or a user override depending on the marker. Persist the resolved source back into the namespace when resolving CTIs, and update harvester-selection/rebind logic to fall back to discovery when auto-cached CTIs are stale while still raising for strict user overrides. Also add a small import and internal field (_cti_files_source_used) to track the chosen source, plus logging when falling back from a stale auto-cache.
@C-Achard
Copy link
Author

Note : CI is expected to fail, will be fixed by #54

Copy link

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

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


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

Add preflight checks, pattern validation and safer globbing for GenTL (.cti) discovery and loading. Renamed default CTI pattern constant to _LEGACY_DEFAULT_CTI_PATTERNS (Windows-only comment) and imported Path. Introduced _cti_preflight to detect missing/locked/permissioned files before Harvester.add_file and applied it where CTIs are loaded (skipping and logging problematic entries). Harden gentl_discovery with: redact-able diagnostics.summarize, _validate_glob_pattern, bounded _glob_limited, static-prefix checks, allowed-roots option and new discover_cti_files params (allow_globs, root_globs_allowed, max_glob_hits_per_pattern) to limit expensive scans. Also adjusted Harvester.update failure handling to treat update errors as discovery failures (return empty loaded list) and improved logging messages for discovery/load failures.
Introduce a single _expand_user_and_env() helper to consistently expand environment variables and '~', and use it across path normalization and glob validation. Update _normalize_path() to rely on the helper and Path handling, switch file existence checks to use a Path object, and add _dedup_key() to normalize deduplication on case-insensitive filesystems (Windows). Replace scattered os.path.expandvars/os.path.expanduser calls with the new helper for more predictable cross-platform behavior.
Copy link

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

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


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

Adjust Harvester.update() error log string concatenation in gentl_backend.py to ensure proper spaces between message parts. Also remove an extraneous comment line from the gentl backend test to clean up test output.
@C-Achard C-Achard mentioned this pull request Feb 25, 2026
19 tasks
@C-Achard C-Achard merged commit 1a795b9 into cy/update-install Feb 25, 2026
2 of 11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

camera Related to cameras and camera backends enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants