Upgrade Gentl backend cti discovery

[C-Achard]

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]

[Copilot]

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

[Copilot]

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.

[Copilot]

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.

[Copilot]

The harvester is not properly cleaned up before setting it to None. If harvester.add_file() loaded some CTI files before others failed, or if the Harvester constructor allocated resources, those resources should be released by calling harvester.reset() before setting self._harvester to None. This can cause resource leaks.

[Copilot]

The harvester is not properly cleaned up before setting it to None. Even though no devices were found, the harvester instance may have allocated resources (opened handles, loaded CTIs) that need to be released via harvester.reset() before discarding the reference. This can cause resource leaks.

[Copilot]

The assertion uses 'or' which makes it always pass. If the first condition (candidates == [str(cti.resolve())]) is False, the test will still pass if the second condition (Path(candidates[0]).name == "Direct.cti") is True. However, if candidates is empty, accessing candidates[0] will raise an IndexError. The intent appears to be checking that the single candidate matches the expected file, but the logic should verify that there's exactly one candidate first, then check its value. Consider using: assert len(candidates) == 1 and Path(candidates[0]).name == "Direct.cti"

Suggested change

	assert candidates == [str(cti.resolve())] or Path(candidates[0]).name == "Direct.cti"
	assert len(candidates) == 1
	assert Path(candidates[0]).name == "Direct.cti"

[Copilot]

If no CTI files are successfully loaded (loaded is empty), the harvester instance is abandoned without cleanup. The harvester was created at line 790 and may have acquired resources even if no CTI files loaded successfully. Add harvester.reset() before the early return to prevent resource leaks.

Suggested change

	if not loaded:
	if not loaded:
	harvester.reset()

[Copilot]

The new persistence keys (cti_files, cti_files_loaded, cti_files_failed) added at lines 364-366 are not covered by any tests. These diagnostics are important for debugging CTI loading issues in production, so test coverage should verify that these keys are correctly populated when CTI files are loaded successfully, partially fail, or completely fail to load.