[jamie75]

Summary

This PR adds an optional --camera-api setting that allows users to explicitly select the recording API instead of performing firmware detection during every synchronization.

Supported modes:

• auto (default): Preserves existing firmware detection.
• legacy: Skips detection and uses /blackvue_vod.cgi.
• modern: Skips detection and uses /vodList.

The setting is also available through the Docker environment variable CAMERA_API.

Motivation

For cameras with known API compatibility, firmware detection introduces an unnecessary HTTP request during every synchronization.

Allowing users to select the API explicitly eliminates that request while preserving automatic detection as the default.

Testing

• 185 automated tests passed.
• 27 modern and 28 legacy behavioral scenarios passed.
• Docker AMD64 image built and validated.
• Tested against a BlackVue DR970X LTE.
• Successfully retrieved an inventory containing 952 recordings.
• Confirmed forced legacy mode does not request /accessible.
• Confirmed successful recording downloads in a production Docker deployment.

This change does not attempt to address underlying network connectivity issues.

Compatibility

Existing installations continue using automatic detection unless the new setting is explicitly configured.

[acolomba]

hi @jamie75, an automated review found these items, which i have in turn reviewed to be legitimate. if you are able to apply this feedback, that's great. if not, i'll get to it with my own agent. either way, this is a good feature and i will incorporate it. thank you!

Findings

Most severe first. Line numbers refer to the PR branch.

1. Mode log is noisy in cron mode (correctness)

blackvuesync.py:2038: main() logs Camera API mode : %s with cron_logger.info. In the default Docker setup (CRON=1), cron_logger runs at INFO, so this line prints on every cron run (every 15 minutes), even when nothing downloads. cron_logger is reserved for Normal/Manual recordings and errors.

Fix: use logger.debug (like the existing firmware_detected message) or logger.info. Update the README paragraph that says the mode is logged in cron mode.

2. Empty CAMERA_API aborts container startup (correctness)

entrypoint.sh:3 and blackvuesync.sh:4 use ${CAMERA_API-auto}, not ${CAMERA_API:-auto}. A set-but-empty CAMERA_API= (common in Portainer and compose) exits with code 2, and the container restart-loops. Every other env var treats empty as unset.

Fix: use ${CAMERA_API:-auto} in both scripts. Update the README ("an empty or invalid value is rejected") and the empty cases in test_docker_startup_rejects_invalid_camera_api.

3. End-to-end test depends on host disk usage (test-coverage)

test/blackvuesync_test.py, test_camera_api_preserves_inventory_and_downloads: the test runs the real CLI with --max-used-disk 98. It fails on any machine whose temp filesystem is more than 98% full.

Fix: pass --max-used-disk 100, or stub the disk-usage check.

4. Mode list validated in three places (simplification)

The allowed values are hard-coded in the argparse choices, in a runtime ValueError in get_dashcam_filenames (blackvuesync.py:846), and in the entrypoint.sh case statement. The ValueError guards a case argparse already blocks.

Fix: add a module constant for the modes, use it for argparse choices, and type the parameter with Literal. Drop the ValueError and test_inventory_rejects_invalid_camera_api.

5. Misleading error when the forced mode is wrong (correctness)

blackvuesync.py:854: if the camera doesn't support the forced API (for example, a firmware update changed it), the 404 surfaces as the generic Cannot obtain list of recordings ... HTTP Error 404. Nothing points to CAMERA_API.

Fix: when the mode is forced and the index request returns 404, raise an error that names the mode and suggests auto.

6. Tests assert on message text (conventions)

test_cli_rejects_invalid_camera_api, test_inventory_rejects_invalid_camera_api, and the "Camera API mode : " in first_run.stderr check use assert_that(x in y, equal_to(True)) or compare message text. skills/python-unit-testing/SKILL.md says to assert errors by class and structured fields, not by message text.

Fix: use contains_string() for stderr, and assert the exception class.

7. Shell-script tests in the module's test file (conventions)

test_docker_wrapper_passes_camera_api and test_docker_startup_rejects_invalid_camera_api test blackvuesync.sh and entrypoint.sh from inside test/blackvuesync_test.py. AGENTS.md says test/ has one test module per source module, and Docker behavior is tested in docker-build.yml and features/.

Fix: move these tests to the Docker or behave suites.

8. No behave scenarios for forced modes (test-coverage)

No features/ scenarios cover --camera-api legacy, --camera-api modern, or the Docker CAMERA_API path. AGENTS.md says to test both protocols.

Fix: add scenarios that force each mode against the mock dashcam, and confirm that forced modes never request /accessible.

9. PR title is not in Conventional Commits format (conventions)

The title "Features/camera api" breaks the AGENTS.md rule. Merges are squash-only, so the title becomes the commit subject on main.

Fix: retitle, for example feat: add --camera-api to skip firmware detection.

[jamie75]

Thanks for the detailed review. I've addressed all nine findings.

The update moves mode logging to DEBUG, defaults empty or unset Docker values to auto, centralizes the CLI choices with Literal annotations, and adds actionable errors for incompatible forced API modes.

I also moved shell coverage into Behave and added scenarios covering both protocols, probe bypass, environment validation, and metadata downloads. The unit tests now use deterministic disk-usage values, and assertions follow the project's conventions.

Validation passed on Python 3.9 and 3.13, including the direct and Docker Behave suites. Coverage is 82.01%, and all pre-commit checks passed.

Existing automatic detection, download, and retention behavior remain unchanged.