[romainantes]

Adds support for CTronics cameras using HTTP snapshots and ONVIF PTZ/Imaging controls.
It also extends RTSP stream path customization and adds dedicated documentation for camera adapters.

⚠️ Branch "docs/camera-adapter-guide" must be merged before this branch.

Changes:
New Adapter:

• Introduced CTronicsCamera supporting authenticated HTTP snapshots, ONVIF PTZ, CGI relative focus, and ONVIF Imaging.
• Registry & Config: Registered the ctronics adapter in registry.py and updated core/config.py to allow custom rtsp_path overrides.
• Dependencies: Added onvif-zeep dependency to pyproject.toml and requirements.txt.
• Documentation: Updated README.md and camera/adapters/README.md to document adapter responsibilities and configuration details.
• Tests: Added unit and real-hardware integration tests in test_ctronics.py.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 79.46%. Comparing base (f2dd3b4) to head (1f9d8b2).

Additional details and impacted files

@@           Coverage Diff            @@
##           develop     #404   +/-   ##
========================================
  Coverage    79.46%   79.46%           
========================================
  Files            6        6           
  Lines          852      852           
========================================
  Hits           677      677           
  Misses         175      175           

Flag	Coverage Δ
unittests	79.46% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[MateoLostanlen]

Review of the CTronics adapter. 8 correctness issues, 1 dead code block, 1 efficiency note. Details inline.

[MateoLostanlen]

Hi @romainantes,

Thanks a lot for the PR, and sorry for the late review! I left a few inline comments. I don't have access to the camera so it's not ideal for reviewing, but let me know what you think.

Quick question by the way: why go through ONVIF rather than the camera's native API? I've heard ONVIF is often less well maintained on the firmware side.

And following on from that, if it's pure ONVIF, I'm wondering if we should have a generic ONVIF adapter directly (reusable for other brands) rather than a CTronics-specific one. What do you think?

[romainantes]

Hi @romainantes,

Thanks a lot for the PR, and sorry for the late review! I left a few inline comments. I don't have access to the camera so it's not ideal for reviewing, but let me know what you think.

Quick question by the way: why go through ONVIF rather than the camera's native API? I've heard ONVIF is often less well maintained on the firmware side.

And following on from that, if it's pure ONVIF, I'm wondering if we should have a generic ONVIF adapter directly (reusable for other brands) rather than a CTronics-specific one. What do you think?

Thanks for the review and for raising this!

I chose ONVIF because the camera’s native API did not expose a sufficiently complete and reliable interface for all the operations required by the adapter. In particular, I had difficulty covering PTZ presets, absolute focus, focus status, and the different control paths consistently through the native API. ONVIF provided a single control interface for these operations, and I validated the relevant absolute-focus behavior on the available CTronics camera hardware.

That said, I am aware that ONVIF support can vary between firmware versions and vendors. The adapter therefore still handles several CTronics-specific details, such as snapshot capture, connection defaults, profile selection, preset-token behavior, and the camera’s advertised Imaging/PTZ capabilities.

I agree that a generic ONVIF adapter could be valuable in the future, especially for the PTZ and Imaging functionality. However, I would prefer to keep this PR CTronics-specific for now because the current implementation still contains vendor-specific behavior and has only been validated on CTronics hardware.

[MateoLostanlen]

Hi @romainantes

Thanks for all the updates, that was a lot of work and the adapter is in much better shape now.

I went through the ten points and checked the code, not just the replies. All of them are addressed: zoom no longer touches the focus motor, the preset index fallback is gone and now fails loudly, focus is clamped and seeded from the camera, ONVIF errors return 503, the CGI half is deleted, and the preset cache plus DEBUG logs are in. Camera API tests pass locally (73 passed, 7 skipped).

Three things before this can merge:

1. The branch needs a rebase on develop, there are conflicts on both READMEs since Docs/camera adapter guide #407 landed.
2. ruff format --check fails on test_config.py, test_ctronics.py and test_focus_cancel.py. A make style should do it.
3. No CI check has run on the PR at all, probably because of the conflicts. Worth confirming green after the rebase.

A few small things I noticed while reading:

• In _ensure_onvif, self._ptz_service is assigned before _refresh_preset_cache(). If GetPresets fails once, the init raises but _ptz_service stays set, so the next call returns early and _imaging_service is never created. Focus then stays broken for good after a single transient failure. Assigning _ptz_service last, or resetting the attributes in the except, would fix it.
• USER_ENC / PWD_ENC in core/config.py are unused now after the RTSP refactor.
• README.md still lists model as a CTronics option.
• focus_finder seeds correctly now, but the window is still ±50 in steps of 10 over a 0 to 1000 range, so a badly defocused camera still will not be recovered. Fine to leave as is, just noting it.

Also worth flagging: tests.yml only runs pytest tests/, so pyro_camera_api/tests/ never runs on CI. That is a repo-wide gap rather than something for this PR, but it means your new tests are not actually gating anything yet.

On the ONVIF question, your reasoning makes sense and I am fine keeping this CTronics specific. Just noting that after removing the CGI path, capture() is about the only vendor specific piece left, so splitting a generic ONVIF base out of this will be easy the day a second ONVIF camera shows up. No need to do it here.

Last one: you mentioned absolute zoom could not be validated on your unit. Can we get it tested on a camera that supports it before we rely on it in the stream stop path?

[romainantes]

Hi @romainantes

Thanks for all the updates, that was a lot of work and the adapter is in much better shape now.

I went through the ten points and checked the code, not just the replies. All of them are addressed: zoom no longer touches the focus motor, the preset index fallback is gone and now fails loudly, focus is clamped and seeded from the camera, ONVIF errors return 503, the CGI half is deleted, and the preset cache plus DEBUG logs are in. Camera API tests pass locally (73 passed, 7 skipped).

Three things before this can merge:

1. The branch needs a rebase on develop, there are conflicts on both READMEs since Docs/camera adapter guide #407 landed.
2. ruff format --check fails on test_config.py, test_ctronics.py and test_focus_cancel.py. A make style should do it.
3. No CI check has run on the PR at all, probably because of the conflicts. Worth confirming green after the rebase.

A few small things I noticed while reading:

• In _ensure_onvif, self._ptz_service is assigned before _refresh_preset_cache(). If GetPresets fails once, the init raises but _ptz_service stays set, so the next call returns early and _imaging_service is never created. Focus then stays broken for good after a single transient failure. Assigning _ptz_service last, or resetting the attributes in the except, would fix it.
• USER_ENC / PWD_ENC in core/config.py are unused now after the RTSP refactor.
• README.md still lists model as a CTronics option.
• focus_finder seeds correctly now, but the window is still ±50 in steps of 10 over a 0 to 1000 range, so a badly defocused camera still will not be recovered. Fine to leave as is, just noting it.

Also worth flagging: tests.yml only runs pytest tests/, so pyro_camera_api/tests/ never runs on CI. That is a repo-wide gap rather than something for this PR, but it means your new tests are not actually gating anything yet.

On the ONVIF question, your reasoning makes sense and I am fine keeping this CTronics specific. Just noting that after removing the CGI path, capture() is about the only vendor specific piece left, so splitting a generic ONVIF base out of this will be easy the day a second ONVIF camera shows up. No need to do it here.

Last one: you mentioned absolute zoom could not be validated on your unit. Can we get it tested on a camera that supports it before we rely on it in the stream stop path?

1. The branch needs a rebase on develop, there are conflicts on both READMEs since Docs/camera adapter guide Docs/camera adapter guide #407 landed.
   => I wasn't able to perform the rebase correctly, so I'd appreciate it if you could take care of it.
2. ruff format --check fails on test_config.py, test_ctronics.py and test_focus_cancel.py. A make style should do it.
   => I've fixed the formatting issues reported by Ruff on all three files.
3. No CI check has run on the PR at all, probably because of the conflicts. Worth confirming green after the rebase.
   => Agreed. Once the rebase is done, I'll check the CI and address any issues if needed.

"In _ensure_onvif, self._ptz_service is assigned before _refresh_preset_cache(). If GetPresets fails once, the init raises but _ptz_service stays set, so the next call returns early and _imaging_service is never created. Focus then stays broken for good after a single transient failure. Assigning _ptz_service last, or resetting the attributes in the except, would fix it."
=> This was a real retry issue: _ptz_service could remain set after _refresh_preset_cache() failed, causing subsequent calls to skip ONVIF initialization. I reset the ONVIF state in the exception path and added a regression test covering a transient GetPresets failure followed by a successful retry.
USER_ENC and PWD_ENC were indeed unused after the RTSP refactor, so I removed them. The CTronics README entry has been updated to remove the unsupported model option. I left the focus_finder search window unchanged for now, as the current behavior is correct and the wider recovery range can be addressed separately if needed.

"Also worth flagging: tests.yml only runs pytest tests/, so pyro_camera_api/tests/ never runs on CI. That is a repo-wide gap rather than something for this PR, but it means your new tests are not actually gating anything yet."
=> I hadn't noticed that because I was running the tests manually, but you're right. This is definitely something we should address separately by opening a story to update tests.yml.

"On the ONVIF question, your reasoning makes sense and I am fine keeping this CTronics specific. Just noting that after removing the CGI path, capture() is about the only vendor specific piece left, so splitting a generic ONVIF base out of this will be easy the day a second ONVIF camera shows up. No need to do it here."
=> Regarding ONVIF, that makes sense. We can open a separate story to introduce a generic ONVIF base and move the common logic there.

"Last one: you mentioned absolute zoom could not be validated on your unit. Can we get it tested on a camera that supports it before we rely on it in the stream stop path?"
=> My current camera does not support ONVIF absolute zoom, so I could not validate the physical movement on that unit. The existing hardware test already covers start_zoom_focus; it needs to be run against a camera that supports ONVIF AbsoluteMove zoom before relying on it operationally. The stream stop path remains best-effort and safely ignores failures when the camera does not support this capability.

[MateoLostanlen]

Hi @romainantes

Sorry for the late reply, September was busy.

Thanks for the last round, the retry fix looks good. I took care of the rest:

Rebased on develop and fixed the README and uv.lock conflicts.
ruff: ignore[...] is not valid ruff syntax, so I replaced those comments with per-file ignores in pyproject.toml.
Fixed 3 small mypy errors.
Synced pyro_camera_api/uv.lock with onvif-zeep.

CI is green, merging. Thanks for the work on this one!