From 218d080dc370a77a3da45c11f32c12c1018693a5 Mon Sep 17 00:00:00 2001 From: Alessandro Colomba Date: Tue, 7 Jul 2026 07:33:00 -0400 Subject: [PATCH 1/7] docs: clarify skip-metadata accelerometer behavior on V1.009+ Adds a README footnote noting that V1.009+ firmware embeds accelerometer data in the video recordings, so no separate .3gf file exists and the 3 flag has no effect there. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01E6HUzpKoyDaLmCKFxDoYe8 --- README.md | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 54607c3..55f6da7 100644 --- a/README.md +++ b/README.md @@ -130,7 +130,7 @@ Other options: * `--max-used-disk`: Downloads stop once the specified used disk percentage threshold is reached. Defaults to `90` (i.e. 90%.) * `--timeout`: Sets a timeout for establishing a connection to the dashcam, in seconds. Defaults to `10.0` seconds. * `--retry-failed-after`: Sets the minimum elapsed time before retrying a failed download. Accepted units are `s` for seconds, `h` for hours, `d` for days and `w` for weeks. If no unit is indicated, days are assumed. Defaults to `1d`. -* `--skip-metadata`: Skips downloading metadata file types. Takes a string of characters: `t` for thumbnail (`.thm`), `3` for accelerometer (`.3gf`), `g` for GPS (`.gps`). For example, `--skip-metadata t3g` skips all metadata files, downloading only the `.mp4` video recordings. +* `--skip-metadata`: Skips downloading metadata file types. Takes a string of characters: `t` for thumbnail (`.thm`), `3` for accelerometer (`.3gf`)[^1], `g` for GPS (`.gps`). For example, `--skip-metadata t3g` skips all metadata files, downloading only the `.mp4` video recordings. * `--include`: Downloads only recordings matching the given codes. Each code is a recording type letter optionally followed by a camera direction letter, comma-separated. For example, `--include P,NF` downloads all Parking recordings and Normal Front recordings. See the table below for valid codes. * `--exclude`: Excludes recordings matching the given codes, same format as `--include`. Takes priority over `--include`. For example, `--include N,E --exclude NR` downloads all Normal and Event recordings except Normal Rear. * `--quiet`: Quiets down output messages, except for unexpected errors. Takes precedence over `--verbose`. @@ -308,7 +308,7 @@ Other parameters: * `TIMEOUT`: If set to a float value, sets the timeout in seconds for connecting to the dashcam. (Default: `10.0` seconds.) * `RETRY_FAILED_AFTER`: If set, sets the minimum elapsed time before retrying a failed download. Accepted units are `s` for seconds, `h` for hours, `d` for days and `w` for weeks. If no unit is indicated, days are assumed. (Default: `1d`.) * `VERBOSE`: If set to a number greater than zero, increases logging verbosity. (Default: `0`.) -* `SKIP_METADATA`: If set, skips downloading the indicated metadata file types. Takes a string of characters: `t` for thumbnail (`.thm`), `3` for accelerometer (`.3gf`), `g` for GPS (`.gps`). For example, `t3g` skips all metadata files. (Default: empty.) +* `SKIP_METADATA`: If set, skips downloading the indicated metadata file types. Takes a string of characters: `t` for thumbnail (`.thm`), `3` for accelerometer (`.3gf`)[^1], `g` for GPS (`.gps`). For example, `t3g` skips all metadata files. (Default: empty.) * `INCLUDE`: If set, downloads only recordings matching the given codes. Each code is a recording type letter optionally followed by a camera direction letter, comma-separated. For example, `P,NF` downloads all Parking recordings and Normal Front recordings. (Default: empty, meaning all recordings are downloaded.) * `EXCLUDE`: If set, excludes recordings matching the given codes, same format as `INCLUDE`. Takes priority over `INCLUDE`. For example, setting `INCLUDE=N` and `EXCLUDE=NR` downloads all Normal recordings except Normal Rear. (Default: empty.) * `QUIET`: If set to any value, quiets down logs: only unexpected errors will be logged. (Default: empty.) @@ -327,3 +327,7 @@ Other parameters: This project is licensed under the MIT License - see the [COPYING](COPYING) file for details Copyright 2018-2026 [Alessandro Colomba](https://github.com/acolomba) + +## Footnotes + +[^1]: In firmware V1.009+, accelerometer data is embedded in the video recordings and no separate `.3gf` file exists, so the `3` flag has no effect. From 8686b913c5fe82c38724335694375af94d07a045 Mon Sep 17 00:00:00 2001 From: Alessandro Colomba Date: Wed, 8 Jul 2026 06:44:54 -0400 Subject: [PATCH 2/7] test: run behave suite against both camera protocols Runs the full integration suite twice, once against the V1.009+ mock and once against the legacy protocol, with the mode selected at launch via the legacy_api userdata flag. Scenarios tagged @legacy (accelerometer skip, which V1.009+ cannot exercise) auto-skip outside legacy runs. Coverage files from each run carry a protocol suffix so consecutive runs accumulate instead of overwriting each other; CI workflows invoke both runs before combining. Removes the API parity feature: the dual runs now cover both protocols wholesale. Its metadata content assertions move to the basic sync feature, joined by a failure-marker check that catches spurious accelerometer download attempts on V1.009+ (previously undetectable). --- .github/workflows/ci.yml | 6 ++++- .github/workflows/docker-build.yml | 6 ++++- .github/workflows/sonarcloud.yml | 6 ++++- behave.ini | 3 +++ features/environment.py | 26 ++++++++++++++----- features/lib/dashcam.py | 15 +++++++++++ features/steps/dashcam_recordings_steps.py | 8 ++---- features/steps/downloaded_recordings_steps.py | 8 ++++-- features/sync_api_parity.feature | 23 ---------------- features/sync_basic.feature | 3 +++ features/sync_skip_metadata.feature | 2 +- 11 files changed, 64 insertions(+), 42 deletions(-) create mode 100644 features/lib/dashcam.py delete mode 100644 features/sync_api_parity.feature diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 7808c16..cd25062 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -59,10 +59,14 @@ jobs: run: | pip install -e ".[dev]" - - name: Run Behave tests + - name: Run Behave tests (V1.009+ protocol) run: | behave --no-capture --format progress + - name: Run Behave tests (legacy protocol) + run: | + behave --no-capture --format progress -D legacy_api=true + build: # builds package on all events (PRs, main, and tags) to catch build errors early needs: [unit-tests, integration-tests] diff --git a/.github/workflows/docker-build.yml b/.github/workflows/docker-build.yml index 2c9370d..dab5fd1 100644 --- a/.github/workflows/docker-build.yml +++ b/.github/workflows/docker-build.yml @@ -58,10 +58,14 @@ jobs: cache-from: type=gha cache-to: type=gha,mode=max - - name: Run Docker integration tests + - name: Run Docker integration tests (V1.009+ protocol) run: | behave -D implementation=docker -D image_name=${{ env.REGISTRY_IMAGE }}:test --no-capture --format progress + - name: Run Docker integration tests (legacy protocol) + run: | + behave -D implementation=docker -D image_name=${{ env.REGISTRY_IMAGE }}:test --no-capture --format progress -D legacy_api=true + build: needs: [test] runs-on: ubuntu-latest diff --git a/.github/workflows/sonarcloud.yml b/.github/workflows/sonarcloud.yml index 63c8263..b744468 100644 --- a/.github/workflows/sonarcloud.yml +++ b/.github/workflows/sonarcloud.yml @@ -42,10 +42,14 @@ jobs: run: | COVERAGE_FILE=.coverage.pytest pytest test/blackvuesync_test.py --cov=. --cov-report= -v - - name: Run integration tests with coverage + - name: Run integration tests with coverage (V1.009+ protocol) run: | behave --no-capture --format progress -D collect_coverage=true + - name: Run integration tests with coverage (legacy protocol) + run: | + behave --no-capture --format progress -D collect_coverage=true -D legacy_api=true + - name: Combine coverage data run: | coverage combine --keep diff --git a/behave.ini b/behave.ini index be5a372..e1e14c0 100644 --- a/behave.ini +++ b/behave.ini @@ -13,5 +13,8 @@ log_level_http = WARN log_level_mock_dashcam = INFO # collects code coverage data (direct mode only) collect_coverage = false +# runs the suite against the legacy camera protocol instead of V1.009+; +# scenarios tagged @legacy only run when this is true +legacy_api = false # execution implementation: direct (subprocess) or docker (container) implementation = direct diff --git a/features/environment.py b/features/environment.py index 8f73762..d9a989c 100644 --- a/features/environment.py +++ b/features/environment.py @@ -10,6 +10,7 @@ from testcontainers.core.image import DockerImage from testcontainers.core.network import Network +from features.lib.dashcam import set_legacy_api from features.lib.docker import get_docker_image from features.mock_dashcam import MockDashcam @@ -146,7 +147,8 @@ def after_all(context: Context) -> None: and implementation == "direct" and hasattr(context, "test_run_dir") ): - _combine_coverage(context.test_run_dir) + legacy_api = context.config.userdata.getbool("legacy_api", False) + _combine_coverage(context.test_run_dir, "legacy" if legacy_api else "v1009") # stops mock dashcam (threaded or containerized) if hasattr(context, "mock_dashcam"): @@ -172,8 +174,12 @@ def after_all(context: Context) -> None: handler.flush() -def _combine_coverage(test_run_dir: Path) -> None: - """copies all coverage files from scenario directories to project root.""" +def _combine_coverage(test_run_dir: Path, mode: str) -> None: + """copies all coverage files from scenario directories to project root. + + the mode discriminates the destination filenames so that consecutive runs + against different protocols accumulate instead of overwriting each other. + """ # finds all .coverage files in scenario directories coverage_files = list(test_run_dir.glob("*/coverage/.coverage*")) @@ -186,7 +192,7 @@ def _combine_coverage(test_run_dir: Path) -> None: # copies all coverage files to project root for combining project_root = Path(__file__).parent.parent for i, coverage_file in enumerate(coverage_files): - dest = project_root / f".coverage.behave.{i}" + dest = project_root / f".coverage.behave.{mode}.{i}" shutil.copy2(coverage_file, dest) logger.info("copied %s to %s", coverage_file, dest) @@ -207,9 +213,6 @@ def before_scenario(context: Context, scenario: Scenario) -> None: # skip_metadata defaults to empty; steps that set --skip-metadata will populate it context.skip_metadata = set() - # legacy_api defaults to False (current firmware); the legacy step overrides it - context.legacy_api = False - # logs directory context.log_dir = context.scenario_dir / "logs" context.log_dir.mkdir(parents=True, exist_ok=True) @@ -217,6 +220,15 @@ def before_scenario(context: Context, scenario: Scenario) -> None: # generates scenario affinity token (unique id for this scenario) context.scenario_token = f"{scenario_name}_{uuid.uuid4()}" + # legacy_api defaults to the launcher-level protocol mode; the legacy step overrides it + context.legacy_api = context.config.userdata.getbool("legacy_api", False) + if context.legacy_api: + set_legacy_api(context.mock_dashcam_url, context.scenario_token, True) + + # scenarios tagged @legacy exercise protocol features absent on V1.009+ + if "legacy" in scenario.effective_tags and not context.legacy_api: + scenario.skip("legacy protocol only") + def after_scenario(context: Context, scenario: Scenario) -> None: """after scenario""" diff --git a/features/lib/dashcam.py b/features/lib/dashcam.py new file mode 100644 index 0000000..276e5b1 --- /dev/null +++ b/features/lib/dashcam.py @@ -0,0 +1,15 @@ +"""mock dashcam session configuration helpers""" + +import requests + + +def set_legacy_api( + mock_dashcam_url: str, scenario_token: str, legacy_api: bool +) -> None: + """configures whether the mock dashcam session behaves as a legacy camera or V1.009+.""" + url = f"{mock_dashcam_url}/mock/legacy-api" + headers = {"X-Affinity-Key": scenario_token} + data = {"legacy_api": legacy_api} + + response = requests.post(url, json=data, headers=headers, timeout=10) + response.raise_for_status() diff --git a/features/steps/dashcam_recordings_steps.py b/features/steps/dashcam_recordings_steps.py index 85c43b0..f694a35 100644 --- a/features/steps/dashcam_recordings_steps.py +++ b/features/steps/dashcam_recordings_steps.py @@ -4,6 +4,7 @@ from behave import given from behave.runner import Context +from features.lib.dashcam import set_legacy_api from features.lib.recordings import filter_recording_filenames_by_period @@ -117,9 +118,4 @@ def dashcam_recordings_same_as_downloaded_past(context: Context, period: str) -> def dashcam_legacy_api(context: Context, legacy_api: str) -> None: """configures whether the mock dashcam behaves as a legacy camera or V1.009+.""" context.legacy_api = legacy_api.lower() == "true" - url = f"{context.mock_dashcam_url}/mock/legacy-api" - headers = {"X-Affinity-Key": context.scenario_token} - data = {"legacy_api": context.legacy_api} - - response = requests.post(url, json=data, headers=headers, timeout=10) - response.raise_for_status() + set_legacy_api(context.mock_dashcam_url, context.scenario_token, context.legacy_api) diff --git a/features/steps/downloaded_recordings_steps.py b/features/steps/downloaded_recordings_steps.py index 0036cf1..68a1233 100644 --- a/features/steps/downloaded_recordings_steps.py +++ b/features/steps/downloaded_recordings_steps.py @@ -165,11 +165,15 @@ def assert_downloaded_recordings_exist(context: Context) -> None: @then('the destination contains no "{extension}" files') def assert_no_extension_files(context: Context, extension: str) -> None: - """verifies that no files with the given extension exist in the destination.""" + """verifies that no files with the given extension exist in the destination, + including failure markers left by download attempts.""" matching_files = [ f.name for f in context.dest_dir.rglob("*") - if f.is_file() and f.name.endswith(f".{extension}") + if f.is_file() + and ( + f.name.endswith(f".{extension}") or f.name.endswith(f".{extension}.failed") + ) ] assert_that( matching_files, diff --git a/features/sync_api_parity.feature b/features/sync_api_parity.feature deleted file mode 100644 index 036764e..0000000 --- a/features/sync_api_parity.feature +++ /dev/null @@ -1,23 +0,0 @@ -Feature: API version parity - - Scenario Outline: sync yields the same recordings whether the camera is legacy - Given the dashcam is legacy: - Given recordings for the past "1d" of types "NE", directions "FR" - When blackvuesync runs - Then blackvuesync exits with code 0 - Then all the recordings are downloaded - - Examples: - | legacy_api | - | true | - | false | - - Scenario: sync retrieves thumbnail and gps metadata on V1.009 cameras - Given the dashcam is legacy: false - Given recordings for the past "1d" of types "N", directions "F" - When blackvuesync runs - Then blackvuesync exits with code 0 - Then all the recordings are downloaded - Then the downloaded "thm" files match the mock fixture - Then the downloaded "gps" files match the mock fixture - Then the destination contains no "3gf" files diff --git a/features/sync_basic.feature b/features/sync_basic.feature index 0478cf7..8d89cea 100644 --- a/features/sync_basic.feature +++ b/features/sync_basic.feature @@ -5,6 +5,9 @@ Feature: Basic sync operations When blackvuesync runs Then blackvuesync exits with code 0 Then all the recordings are downloaded + Then the downloaded "thm" files match the mock fixture + Then the downloaded "gps" files match the mock fixture + Then no failure markers exist Scenario: Sync when destination already has some recordings Given downloaded recordings between "2d" and "1d" ago of types "NE", directions "FR" diff --git a/features/sync_skip_metadata.feature b/features/sync_skip_metadata.feature index d6e36e4..48591b9 100644 --- a/features/sync_skip_metadata.feature +++ b/features/sync_skip_metadata.feature @@ -16,8 +16,8 @@ Feature: Sync with skip-metadata option Then all the recordings are downloaded Then the destination contains no "thm" files + @legacy Scenario: Sync recordings skipping only accelerometer files - Given the dashcam is legacy: true Given recordings for the past "1d" of types "N", directions "F" When blackvuesync runs with skip-metadata "3" Then blackvuesync exits with code 0 From cf6cfe5ff224608416e5ea4a5fe8ad5a8c571aba Mon Sep 17 00:00:00 2001 From: Alessandro Colomba Date: Wed, 8 Jul 2026 07:06:49 -0400 Subject: [PATCH 3/7] docs: remove planning documents Removes the docs/plans directory. Plan documents are point-in-time artifacts; the shipped code, tests, and README are the durable record. --- docs/plans/2026-02-24-filter-rework.md | 794 ---------------- docs/plans/2026-05-02-metrics-export.md | 249 ----- ...6-07-02-firmware-v1009-listing-api-plan.md | 414 --------- .../2026-07-02-firmware-v1009-listing-api.md | 172 ---- ...5-firmware-v1009-download-metadata-plan.md | 849 ------------------ ...-07-05-firmware-v1009-download-metadata.md | 170 ---- 6 files changed, 2648 deletions(-) delete mode 100644 docs/plans/2026-02-24-filter-rework.md delete mode 100644 docs/plans/2026-05-02-metrics-export.md delete mode 100644 docs/plans/2026-07-02-firmware-v1009-listing-api-plan.md delete mode 100644 docs/plans/2026-07-02-firmware-v1009-listing-api.md delete mode 100644 docs/plans/2026-07-05-firmware-v1009-download-metadata-plan.md delete mode 100644 docs/plans/2026-07-05-firmware-v1009-download-metadata.md diff --git a/docs/plans/2026-02-24-filter-rework.md b/docs/plans/2026-02-24-filter-rework.md deleted file mode 100644 index 7e08ecd..0000000 --- a/docs/plans/2026-02-24-filter-rework.md +++ /dev/null @@ -1,794 +0,0 @@ -# --include / --exclude filter rework implementation plan - -> **For Claude:** REQUIRED SUB-SKILL: Use superpowers:executing-plans to -> implement this plan task-by-task. - -**Goal:** Replace the half-implemented `--filter` option with validated -`--include` and `--exclude` options that support comma-separated type/direction -codes. - -**Architecture:** Extract recording type and direction characters into shared -constants. Add a `parse_filter` argparse type function (following the existing -`parse_skip_metadata` pattern). Replace `get_filtered_recordings` with a -function that applies include then exclude logic. Wire through `sync()` and -update Docker wrapper, integration tests, and README. - -**Tech Stack:** Python 3.9+ stdlib only (argparse, re). Pytest for unit tests. -Behave for integration tests. Bash for Docker wrapper. - ---- - -## Task 1: Extract shared constants - -Files: - -- Modify: `blackvuesync.py:212-219` (filename_re) and surrounding area - -### Step 1: Add constants before filename_re - -Add two constants right before `filename_re` (line 212), after the existing -`RECORDING_EXTENSIONS` constant area. These are the single source of truth for -valid type and direction characters: - -```python -RECORDING_TYPES = "NEPMIOATBRXGDLYF" -RECORDING_DIRECTIONS = "FRIO" -``` - -### Step 2: Update filename_re to use the constants - -Change the hard-coded character classes in `filename_re` to reference the new -constants: - -```python -filename_re = re.compile( - rf"""(?P(?P\d\d\d\d)(?P\d\d)(?P\d\d) - _(?P\d\d)(?P\d\d)(?P\d\d)) - _(?P[{RECORDING_TYPES}]) - (?P[{RECORDING_DIRECTIONS}]) - (?P[LS]?) - \.(?Pmp4)""", - re.VERBOSE, -) -``` - -### Step 3: Run existing tests to verify no regression - -Run: `pytest test/blackvuesync_test.py -v` -Expected: All existing tests pass (especially `test_to_recording`). - -### Step 4: Commit - -```bash -git add blackvuesync.py -git commit -m "Extract recording type and direction constants" -``` - ---- - -## Task 2: Add parse_filter and unit tests (TDD) - -Files: - -- Modify: `blackvuesync.py` (near `parse_skip_metadata` at line 121) -- Modify: `test/blackvuesync_test.py` (near `test_parse_skip_metadata` at - line 664) - -### Step 1: Write failing tests for parse_filter - -Add parametrized tests after the `test_parse_skip_metadata_invalid` tests -(around line 686). Follow the same pattern as `test_parse_skip_metadata`: - -```python -@pytest.mark.parametrize( - "value, expected", - [ - ("PF", ("PF",)), - ("P", ("P",)), - ("PF,PR", ("PF", "PR")), - ("P,NF", ("P", "NF")), - ("N", ("N",)), - ("NF,NR,NI,NO", ("NF", "NR", "NI", "NO")), - ], -) -def test_parse_filter(value: str, expected: tuple[str, ...]) -> None: - assert blackvuesync.parse_filter(value) == expected - - -@pytest.mark.parametrize( - "value", - ["ZZ", "PX", "ABC", "", "P,", ",P", "P,,N", "pf", "1F"], -) -def test_parse_filter_invalid(value: str) -> None: - with pytest.raises(argparse.ArgumentTypeError): - blackvuesync.parse_filter(value) -``` - -### Step 2: Run tests to verify they fail - -Run: `pytest test/blackvuesync_test.py::test_parse_filter -v` -Expected: FAIL with `AttributeError: module 'blackvuesync' has no attribute -'parse_filter'` - -### Step 3: Implement parse_filter - -Add `parse_filter` right after `parse_skip_metadata` (line 130) in -`blackvuesync.py`: - -```python -def parse_filter(value: str) -> tuple[str, ...]: - """parses and validates a comma-separated filter of recording type/direction codes""" - codes = value.split(",") - for code in codes: - if len(code) == 1: - if code not in RECORDING_TYPES: - raise argparse.ArgumentTypeError( - f"invalid filter code '{code}': unknown recording type" - f" (valid: {', '.join(RECORDING_TYPES)})" - ) - elif len(code) == 2: - if code[0] not in RECORDING_TYPES: - raise argparse.ArgumentTypeError( - f"invalid filter code '{code}': unknown recording type" - f" '{code[0]}' (valid: {', '.join(RECORDING_TYPES)})" - ) - if code[1] not in RECORDING_DIRECTIONS: - raise argparse.ArgumentTypeError( - f"invalid filter code '{code}': unknown direction" - f" '{code[1]}' (valid: {', '.join(RECORDING_DIRECTIONS)})" - ) - else: - raise argparse.ArgumentTypeError( - f"invalid filter code '{code}': must be 1 or 2 characters" - " (type, or type + direction)" - ) - return tuple(codes) -``` - -### Step 4: Run tests to verify they pass - -Run: `pytest test/blackvuesync_test.py::test_parse_filter test/blackvuesync_test.py::test_parse_filter_invalid -v` -Expected: All pass. - -### Step 5: Commit - -```bash -git add blackvuesync.py test/blackvuesync_test.py -git commit -m "Add parse_filter with validation and unit tests" -``` - ---- - -## Task 3: Replace get_filtered_recordings and unit tests (TDD) - -Files: - -- Modify: `blackvuesync.py:727-735` (`get_filtered_recordings`) -- Modify: `test/blackvuesync_test.py` - -### Step 1: Write failing tests for the new matching function - -Add tests after the `test_parse_filter_invalid` tests. Use `Recording` -objects with known type/direction values. The function name changes to -`apply_recording_filters`: - -```python -def _recording(type: str, direction: str) -> blackvuesync.Recording: - """creates a minimal Recording for filter tests""" - return blackvuesync.Recording( - filename=f"20250101_120000_{type}{direction}.mp4", - base_filename="20250101_120000", - group_name=None, - datetime=datetime.datetime(2025, 1, 1, 12, 0, 0), - type=type, - direction=direction, - ) - - -class TestApplyRecordingFilters: - """tests for apply_recording_filters""" - - def test_no_filters_returns_all(self) -> None: - recordings = [_recording("N", "F"), _recording("P", "R")] - result = blackvuesync.apply_recording_filters(recordings, None, None) - assert result == recordings - - def test_include_type_only(self) -> None: - recordings = [ - _recording("N", "F"), - _recording("N", "R"), - _recording("P", "F"), - ] - result = blackvuesync.apply_recording_filters( - recordings, ("N",), None - ) - assert result == [recordings[0], recordings[1]] - - def test_include_type_and_direction(self) -> None: - recordings = [ - _recording("N", "F"), - _recording("N", "R"), - _recording("P", "F"), - ] - result = blackvuesync.apply_recording_filters( - recordings, ("NF",), None - ) - assert result == [recordings[0]] - - def test_include_multiple_codes(self) -> None: - recordings = [ - _recording("N", "F"), - _recording("P", "F"), - _recording("E", "F"), - ] - result = blackvuesync.apply_recording_filters( - recordings, ("N", "PF"), None - ) - assert result == [recordings[0], recordings[1]] - - def test_exclude_type_only(self) -> None: - recordings = [ - _recording("N", "F"), - _recording("P", "F"), - _recording("E", "F"), - ] - result = blackvuesync.apply_recording_filters( - recordings, None, ("P",) - ) - assert result == [recordings[0], recordings[2]] - - def test_exclude_type_and_direction(self) -> None: - recordings = [ - _recording("N", "F"), - _recording("N", "R"), - ] - result = blackvuesync.apply_recording_filters( - recordings, None, ("NR",) - ) - assert result == [recordings[0]] - - def test_include_and_exclude_combined(self) -> None: - recordings = [ - _recording("N", "F"), - _recording("N", "R"), - _recording("P", "F"), - ] - result = blackvuesync.apply_recording_filters( - recordings, ("N",), ("NR",) - ) - assert result == [recordings[0]] - - def test_empty_result(self) -> None: - recordings = [_recording("N", "F")] - result = blackvuesync.apply_recording_filters( - recordings, ("P",), None - ) - assert result == [] -``` - -### Step 2: Run tests to verify they fail - -Run: `pytest test/blackvuesync_test.py::TestApplyRecordingFilters -v` -Expected: FAIL with `AttributeError: module 'blackvuesync' has no attribute -'apply_recording_filters'` - -### Step 3: Implement apply_recording_filters - -Replace `get_filtered_recordings` (line 727) with: - -```python -def _matches_filter(recording: Recording, code: str) -> bool: - """checks if a recording matches a single filter code""" - if len(code) == 1: - return recording.type == code - return f"{recording.type}{recording.direction}" == code - - -def apply_recording_filters( - recordings: list[Recording], - include: tuple[str, ...] | None, - exclude: tuple[str, ...] | None, -) -> list[Recording]: - """returns recordings filtered by include/exclude codes""" - result = recordings - if include is not None: - result = [ - r for r in result if any(_matches_filter(r, c) for c in include) - ] - if exclude is not None: - result = [ - r - for r in result - if not any(_matches_filter(r, c) for c in exclude) - ] - return result -``` - -### Step 4: Run tests to verify they pass - -Run: `pytest test/blackvuesync_test.py::TestApplyRecordingFilters -v` -Expected: All pass. - -### Step 5: Commit - -```bash -git add blackvuesync.py test/blackvuesync_test.py -git commit -m "Add apply_recording_filters with include/exclude logic" -``` - ---- - -## Task 4: Wire up argparse and sync() - -Files: - -- Modify: `blackvuesync.py:787-815` (`sync` function) -- Modify: `blackvuesync.py:937-943` (argparse `--filter` definition) -- Modify: `blackvuesync.py:1063` (`main` call to `sync`) - -### Step 1: Replace --filter argparse definition with --include and --exclude - -Remove the `--filter` argument (lines 937-943) and replace with: - -```python - arg_parser.add_argument( - "-i", - "--include", - default=None, - type=parse_filter, - help="downloads only recordings matching the given codes; each code" - " is a recording type optionally followed by a camera direction;" - " e.g. --include P,NF downloads all Parking and Normal Front" - " recordings", - ) - arg_parser.add_argument( - "-e", - "--exclude", - default=None, - type=parse_filter, - help="excludes recordings matching the given codes; takes priority" - " over --include; e.g. --include N --exclude NR downloads all Normal" - " recordings except Normal Rear", - ) -``` - -### Step 2: Update sync() signature and body - -Change the `sync` function signature to accept `include` and `exclude` -instead of `recording_filter`. Update the call to `apply_recording_filters`: - -```python -def sync( - address: str, - destination: str, - grouping: str, - download_priority: str, - include: tuple[str, ...] | None, - exclude: tuple[str, ...] | None, -) -> None: -``` - -And the filter call becomes: - -```python - # filters recordings according to include/exclude options - current_dashcam_recordings = apply_recording_filters( - current_dashcam_recordings, include, exclude - ) -``` - -### Step 3: Update main() call to sync() - -Change line 1063 from: - -```python -sync(args.address, destination, grouping, args.priority, args.filter) -``` - -to: - -```python -sync(args.address, destination, grouping, args.priority, args.include, args.exclude) -``` - -### Step 4: Update test_main mocks - -In `test/blackvuesync_test.py`, find the existing mock for `sync` (around -lines 768 and 786). The mock lambda signatures need to accept the new -parameters. Change `_filter` to `_include, _exclude`. Also update the -`filter=None` in `parse_args` return to `include=None, exclude=None`. - -### Step 5: Run all tests - -Run: `pytest test/blackvuesync_test.py -v` -Expected: All pass. - -### Step 6: Commit - -```bash -git add blackvuesync.py test/blackvuesync_test.py -git commit -m "Replace --filter with --include and --exclude options" -``` - ---- - -## Task 5: Docker wrapper support - -Files: - -- Modify: `blackvuesync.sh` - -### Step 1: Add INCLUDE and EXCLUDE env var handling - -Add after the existing `skip_metadata` line (near end of variable -declarations): - -```bash -# include option if INCLUDE set -include=${INCLUDE:+--include $INCLUDE} - -# exclude option if EXCLUDE set -exclude=${EXCLUDE:+--exclude $EXCLUDE} -``` - -### Step 2: Add to command line - -Append `${include} ${exclude}` to the final command invocation. - -### Step 3: Commit - -```bash -git add blackvuesync.sh -git commit -m "Add INCLUDE and EXCLUDE env vars to Docker wrapper" -``` - ---- - -## Task 6: Integration test step definitions - -Files: - -- Modify: `features/steps/blackvuesync_steps.py` - -### Step 1: Update execute_blackvuesync and helpers - -In all three functions (`execute_blackvuesync`, `_execute_direct`, -`_execute_docker`): - -- Replace `filter_list: list[str] | None = None` parameter with - `include: str | None = None` and `exclude: str | None = None` -- In `_execute_direct`: replace the `-f` / `filter_list` cmd building with - `-i` / `include` and `-e` / `exclude` -- In `_execute_docker`: replace the `NotImplementedError` for filter with - `INCLUDE` and `EXCLUDE` env vars - -For `_execute_direct`, the filter command building changes from: - -```python - if filter_list: - cmd.append("-f") - cmd.extend(filter_list) -``` - -to: - -```python - if include: - cmd.extend(["-i", include]) - - if exclude: - cmd.extend(["-e", exclude]) -``` - -For `_execute_docker`, replace: - -```python - if filter_list: - raise NotImplementedError( - "filter option not supported in docker implementation" - ) -``` - -with: - -```python - if include: - container.with_env("INCLUDE", include) - - if exclude: - container.with_env("EXCLUDE", exclude) -``` - -### Step 2: Run existing integration tests to check nothing breaks - -Run: `behave` -Expected: All existing scenarios pass (no scenario uses filter yet). - -### Step 3: Commit - -```bash -git add features/steps/blackvuesync_steps.py -git commit -m "Update integration test steps for include/exclude" -``` - ---- - -## Task 7: Integration test scenarios - -Files: - -- Create: `features/sync_filter.feature` -- Create: `features/steps/filter_steps.py` - -### Step 1: Create step definitions - -Create `features/steps/filter_steps.py`: - -```python -"""include/exclude filter step definitions""" - -from __future__ import annotations - -from behave import when -from behave.runner import Context - -from features.steps.blackvuesync_steps import execute_blackvuesync - - -@when('blackvuesync runs with include "{include}"') -def run_blackvuesync_with_include(context: Context, include: str) -> None: - """executes blackvuesync with --include option.""" - execute_blackvuesync( - context, - context.mock_dashcam_address, - str(context.dest_dir), - context.scenario_token, - include=include, - ) - - -@when('blackvuesync runs with include "{include}" exclude "{exclude}"') -def run_blackvuesync_with_include_exclude( - context: Context, include: str, exclude: str -) -> None: - """executes blackvuesync with --include and --exclude options.""" - execute_blackvuesync( - context, - context.mock_dashcam_address, - str(context.dest_dir), - context.scenario_token, - include=include, - exclude=exclude, - ) - - -@when('blackvuesync runs with exclude "{exclude}"') -def run_blackvuesync_with_exclude(context: Context, exclude: str) -> None: - """executes blackvuesync with --exclude option.""" - execute_blackvuesync( - context, - context.mock_dashcam_address, - str(context.dest_dir), - context.scenario_token, - exclude=exclude, - ) -``` - -### Step 2: Create feature file - -Create `features/sync_filter.feature`. Use types and directions from the -`Given` step that creates recordings, then verify only the expected subset -is downloaded. The existing step `recordings for the past "{period}" of types -"{recording_types}", directions "{recording_directions}"` sets up recordings -with specific types and directions. The existing `then` step -`all the recordings are downloaded` checks all expected recordings are present. -We need to use the more specific verification steps. - -Look at existing feature files for the assertion pattern. We need to verify -that only recordings matching the filter are present. The existing `then` -steps in `downloaded_recordings_steps.py` check for recordings by listing -files. We should check that the destination only contains recordings matching -our filter. - -```gherkin -Feature: Sync with include/exclude filters - - Scenario: Include by type downloads all directions - Given recordings for the past "1d" of types "N,P", directions "F,R" - When blackvuesync runs with include "N" - Then blackvuesync exits with code 0 - Then the destination contains "N" recordings - Then the destination does not contain "P" recordings - - Scenario: Include by type and direction - Given recordings for the past "1d" of types "N,P", directions "F,R" - When blackvuesync runs with include "NF" - Then blackvuesync exits with code 0 - Then the destination contains "NF" recordings - Then the destination does not contain "NR" recordings - Then the destination does not contain "P" recordings - - Scenario: Exclude by type - Given recordings for the past "1d" of types "N,P,E", directions "F" - When blackvuesync runs with exclude "P" - Then blackvuesync exits with code 0 - Then the destination contains "N" recordings - Then the destination contains "E" recordings - Then the destination does not contain "P" recordings - - Scenario: Include and exclude combined - Given recordings for the past "1d" of types "N", directions "F,R" - When blackvuesync runs with include "N" exclude "NR" - Then blackvuesync exits with code 0 - Then the destination contains "NF" recordings - Then the destination does not contain "NR" recordings - - Scenario: Include multiple codes - Given recordings for the past "1d" of types "N,P,E", directions "F" - When blackvuesync runs with include "N,E" - Then blackvuesync exits with code 0 - Then the destination contains "N" recordings - Then the destination contains "E" recordings - Then the destination does not contain "P" recordings -``` - -Note: the `then` steps for checking recording type/direction presence will -need to be added to the step definitions. Add to `filter_steps.py`: - -```python -import os -import re - - -_recording_filename_re = re.compile( - r"^\d{8}_\d{6}_([NEPMIOATBRXGDLYF])([FRIO])[LS]?\.(mp4|thm|3gf|gps)$" -) - - -@then('the destination contains "{code}" recordings') -def destination_contains_recordings(context: Context, code: str) -> None: - """verifies that the destination contains recordings matching the code.""" - matching = _find_recordings_matching(context.dest_dir, code) - assert matching, f"expected recordings matching '{code}' but found none" - - -@then('the destination does not contain "{code}" recordings') -def destination_does_not_contain_recordings( - context: Context, code: str -) -> None: - """verifies that the destination does not contain recordings matching the - code.""" - matching = _find_recordings_matching(context.dest_dir, code) - assert not matching, ( - f"expected no recordings matching '{code}' but found: {matching}" - ) - - -def _find_recordings_matching(dest_dir: str, code: str) -> list[str]: - """finds recording files in dest_dir matching a filter code.""" - matching = [] - for root, _dirs, files in os.walk(str(dest_dir)): - for filename in files: - m = _recording_filename_re.match(filename) - if m: - rec_type, rec_direction = m.group(1), m.group(2) - if len(code) == 1 and rec_type == code: - matching.append(filename) - elif len(code) == 2 and rec_type == code[0] and rec_direction == code[1]: - matching.append(filename) - return matching -``` - -Add the `then` import at the top: - -```python -from behave import then, when -``` - -### Step 3: Run integration tests - -Run: `behave features/sync_filter.feature` -Expected: All scenarios pass. - -### Step 4: Run full test suite - -Run: `pytest test/blackvuesync_test.py -v && behave` -Expected: All pass. - -### Step 5: Commit - -```bash -git add features/sync_filter.feature features/steps/filter_steps.py -git commit -m "Add integration tests for include/exclude filters" -``` - ---- - -## Task 8: README documentation - -Files: - -- Modify: `README.md` (around line 130, after `--skip-metadata`) - -### Step 1: Add --include and --exclude documentation - -Add after the `--skip-metadata` bullet (line 130) and before `--quiet`: - -```markdown -* `--include`: Downloads only recordings matching the given codes. Each code is - a recording type letter optionally followed by a camera direction letter, - comma-separated. For example, `--include P,NF` downloads all Parking - recordings and Normal Front recordings. See the table below for valid codes. -* `--exclude`: Excludes recordings matching the given codes, same format as - `--include`. Takes priority over `--include`. For example, - `--include N --exclude NR` downloads all Normal recordings except Normal - Rear. -``` - -Also add a reference table after the options list and before the "Unattended -Usage" section. This table lists the type and direction codes: - -```markdown -#### Recording type and direction codes - -Recording type codes: - -| Code | Type | -| ---- | ---- | -| N | Normal | -| E | Event | -| P | Parking | -| M | Manual | -| I | Impact | -| O | Overspeed | -| A | Acceleration | -| T | Cornering | -| B | Braking | -| R | Geofence (R) | -| X | Geofence (X) | -| G | Geofence (G) | -| D | DMS (D) | -| L | DMS (L) | -| Y | DMS (Y) | -| F | DMS (F) | - -Direction codes: - -| Code | Direction | -| ---- | --------- | -| F | Front | -| R | Rear | -| I | Interior | -| O | Optional | -``` - -### Step 2: Update Docker documentation - -Find the Docker environment variable documentation section and add `INCLUDE` -and `EXCLUDE` env vars. - -### Step 3: Commit - -```bash -git add README.md -git commit -m "Document --include and --exclude options in README" -``` - ---- - -## Task 9: Final verification - -### Step 1: Run full test suite - -Run: `pytest test/blackvuesync_test.py -v && behave` -Expected: All pass. - -### Step 2: Manual smoke test - -Run: `python3 blackvuesync.py --help` -Expected: Shows `--include` and `--exclude` in help output, no `--filter`. - -### Step 3: Test validation error - -Run: `python3 blackvuesync.py 192.168.1.99 --include ZZ` -Expected: Error message about invalid filter code. diff --git a/docs/plans/2026-05-02-metrics-export.md b/docs/plans/2026-05-02-metrics-export.md deleted file mode 100644 index 77f0bf3..0000000 --- a/docs/plans/2026-05-02-metrics-export.md +++ /dev/null @@ -1,249 +0,0 @@ -# Metrics export implementation plan - -**Goal:** Add opt-in Prometheus-format metrics for sync health, file pull -failures, and time since the last successful file pull while preserving the -current cron-style execution model. - -**Architecture:** Keep `blackvuesync.py` as a one-shot CLI. During each run, -collect metrics in a small stats object and emit them at process shutdown to one -or both configured sinks: - -- `--metrics-file PATH`: writes Prometheus text exposition format atomically. -- `--metrics-pushgateway-url URL`: pushes the same exposition payload to a - Prometheus Pushgateway for Kubernetes CronJob-style deployments. -- `--metrics-state-file PATH`: persists cross-run metric state. Defaults to a - hidden state file under the destination directory when metrics are enabled. - -**Tech Stack:** Python 3.9+ stdlib only. Use `urllib.request` for Pushgateway -HTTP calls and hand-render the limited Prometheus text format needed here. -Pytest for unit tests. Bash for Docker wrapper env wiring. - ---- - -## Metrics - -Primary metrics: - -- `blackvuesync_last_run_timestamp_seconds`: completion timestamp for the most - recent run. -- `blackvuesync_last_run_success`: sync result for the last run. This is `0` - when the sync could not reach/list/download from the camera, even if `--cron` - maps the process exit code to `0`. -- `blackvuesync_last_run_exit_code`: process exit code for the last run. -- `blackvuesync_last_successful_file_pull_timestamp_seconds`: timestamp of the - most recent successful file download, persisted across runs. -- `blackvuesync_file_download_failures_last_run{reason="..."}`: count of file - download failures observed in the most recent run. -- `blackvuesync_files_downloaded_last_run`: count of files downloaded in the - most recent run. - -Secondary metrics: - -- `blackvuesync_run_duration_seconds`: elapsed runtime for the sync command. -- `blackvuesync_dashcam_recordings_seen`: recording count returned by the - dashcam index. -- `blackvuesync_recordings_selected`: recording count after cutoff and - include/exclude filtering. -- `blackvuesync_bytes_downloaded_last_run`: bytes downloaded in the current run - when content length is available. -- `blackvuesync_destination_disk_used_ratio`: destination disk usage ratio. -- `blackvuesync_failed_marker_files`: count of `.failed` marker files under the - destination. - -Failure reasons should start coarse and stable: `http`, `network`, `timeout`, -`disk`, and `unknown`. - -Do not expose per-process `*_total` counters in the initial implementation. -Because the program exits after each run, download and failure counts should be -last-run gauges unless cumulative counters are explicitly persisted in a future -change. - -Dry runs should emit run metrics but must not update -`blackvuesync_last_successful_file_pull_timestamp_seconds`. - ---- - -## Task 1: Add metrics configuration - -Files: - -- Modify: `blackvuesync.py` -- Modify: `test/blackvuesync_test.py` - -Steps: - -1. Add argparse options: - - `--metrics-file PATH` - - `--metrics-pushgateway-url URL` - - `--metrics-job NAME`, defaulting to `blackvuesync` - - `--metrics-instance NAME`, defaulting to the dashcam address - - `--metrics-state-file PATH`, defaulting to a file under the destination - when metrics are enabled. -2. Validate the Pushgateway URL with `urllib.parse`. -3. Default the state file to - `/.blackvuesync.metrics-state.json` when any metrics sink is - enabled and no explicit state file is provided. -4. Add unit tests for defaults, explicit state override, and invalid - Pushgateway URLs. - ---- - -## Task 2: Add metrics data model - -Files: - -- Modify: `blackvuesync.py` -- Modify: `test/blackvuesync_test.py` - -Steps: - -1. Add a `SyncMetrics` dataclass with fields for timestamps, run result, - last-run gauges, and per-reason failure counts. -2. Add helper methods for: - - recording a successful file download, - - recording a failed file download, - - recording selected/seen recording counts, - - finalizing run outcome and duration. -3. Add tests for the helper methods without doing network or filesystem work. - ---- - -## Task 3: Persist last successful file pull timestamp - -Files: - -- Modify: `blackvuesync.py` -- Modify: `test/blackvuesync_test.py` - -Steps: - -1. Add small JSON state load/save helpers. -2. Store only stable cross-run values initially: - - `last_successful_file_pull_timestamp_seconds` -3. Load state before sync starts when metrics are enabled. -4. Save state after metrics are finalized, but only after successful state - serialization. -5. Do not update last-successful state during dry runs. -6. Add tests for missing, valid, corrupt, and dry-run state behavior. Corrupt - state should not fail the sync; it should be ignored with a warning. - ---- - -## Task 4: Instrument sync flow - -Files: - -- Modify: `blackvuesync.py` -- Modify: `test/blackvuesync_test.py` - -Steps: - -1. Thread the metrics object through `main()`, `sync()`, - `download_recording()`, and `download_file()`. -2. Count dashcam recordings seen after parsing the dashcam index. -3. Count recordings selected after cutoff and include/exclude filters. -4. Record destination disk usage before each recording download check. -5. Count successful file downloads and bytes downloaded in `download_file()`. -6. Count file download failures by reason in the existing exception handlers. -7. Count failed marker files during finalization or destination cleanup. -8. Record sync success separately from process exit code. In cron mode, expected - dashcam unavailability may return exit code `0` but must still emit - `blackvuesync_last_run_success 0`. - ---- - -## Task 5: Render Prometheus text exposition - -Files: - -- Modify: `blackvuesync.py` -- Modify: `test/blackvuesync_test.py` - -Steps: - -1. Add a renderer that emits `# HELP`, `# TYPE`, and metric samples. -2. Keep labels low-cardinality. Do not include recording filenames as labels. -3. Escape label values according to Prometheus text format requirements. -4. Add snapshot-style unit tests for the rendered output. - ---- - -## Task 6: Add metrics file sink - -Files: - -- Modify: `blackvuesync.py` -- Modify: `test/blackvuesync_test.py` - -Steps: - -1. Write metrics to a temporary sibling file. -2. Flush and `fsync` the temporary file. -3. Rename it atomically over the target path. -4. Add tests proving failed writes do not leave partial target files. - ---- - -## Task 7: Add Pushgateway sink - -Files: - -- Modify: `blackvuesync.py` -- Modify: `test/blackvuesync_test.py` - -Steps: - -1. Build the Pushgateway endpoint as - `/metrics/job//instance/`. -2. Use HTTP `PUT` so each run replaces the job's metric group. -3. Use a short timeout tied to the existing socket timeout where practical. -4. Treat Pushgateway failures as warnings by default so metrics delivery does - not mask the sync exit result. -5. Add unit tests for the generated endpoint path and request method with a fake - `urllib.request.urlopen`. - ---- - -## Task 8: Wire Docker and documentation - -Files: - -- Modify: `blackvuesync.sh` -- Modify: `Dockerfile` -- Modify: `docker-compose.yml` -- Modify: `README.md` - -Steps: - -1. Add env-to-CLI wiring for: - - `METRICS_FILE` - - `METRICS_PUSHGATEWAY_URL` - - `METRICS_JOB` - - `METRICS_INSTANCE` - - `METRICS_STATE_FILE` -2. Document host/Docker textfile usage. -3. Document Kubernetes CronJob Pushgateway usage. -4. Include example alert expressions for: - - last run failed, - - no run completed recently, - - no successful file pull recently, - - file download failures in the last hour. -5. Document that metrics delivery failures are warning-only and do not replace - the sync exit result. - ---- - -## Task 9: Validate end to end - -Steps: - -1. Run unit tests: - - `python -m pytest -q` -2. Run Ruff: - - `ruff check blackvuesync.py test/blackvuesync_test.py` - - `ruff format --check blackvuesync.py test/blackvuesync_test.py` -3. Smoke-test metrics file output with an unreachable dashcam. -4. Smoke-test Pushgateway output against a small local HTTP server or test - fixture. -5. Confirm existing text and JSON logging behavior still works. -6. Confirm dry-run metrics do not update persisted last-success state. diff --git a/docs/plans/2026-07-02-firmware-v1009-listing-api-plan.md b/docs/plans/2026-07-02-firmware-v1009-listing-api-plan.md deleted file mode 100644 index 18f274f..0000000 --- a/docs/plans/2026-07-02-firmware-v1009-listing-api-plan.md +++ /dev/null @@ -1,414 +0,0 @@ -# Firmware V1.009 listing API Implementation Plan - -> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers-extended-cc:subagent-driven-development (recommended) or superpowers-extended-cc:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. - -**Goal:** Make blackvuesync sync recordings from BlackVue firmware V1.009+ cameras -(JSON `/vodList` index) while keeping older cameras (plaintext `blackvue_vod.cgi`) -working, verified end-to-end by the mock dashcam in both modes. - -**Architecture:** The only behavioral change is how the recording index is -fetched and parsed. `get_dashcam_filenames` probes `GET /accessible`; when absent -it returns early to the legacy `blackvue_vod.cgi` path (a backwards-compatibility -appendix), otherwise it fetches and parses `GET /vodList` JSON. Everything -downstream (the `Recording` structure, grouping, retention, `/Record/` -downloads) is untouched because it consumes an identical `list[str]` of filenames. - -**Tech Stack:** Python 3.9+ stdlib only (`json` already imported). Flask mock -dashcam for integration. Behave for integration coverage; pytest unchanged. - -**User decisions (already made):** - -- "one dedicated parity feature" -- not a full both-ways matrix over every feature. -- "default the emulator to the new version"; "the new style is _the_ style, the rest is legacy yes/no". -- "use 'legacy' for the old style"; "avoid references to 'restful'". -- Detection: probe `/accessible` first (option A). -- "flip the logic ... to is_legacy_camera and have an early exit/return to get_dashcam_filenames_legacy". -- Mock: "a legacy_api flag ... a binary is sufficient". -- Parity examples: "legacy_api true false, and the given is 'the dashcam is legacy'". -- "minimize the changes to blackvuesync down to the parser of the file list". -- Work in a feature branch (`firmware-v1009-listing-api`, already created). - -**Ordering rationale (why blackvuesync changes land before the emulator flips):** -The current emulator has no `/accessible` route, so after Task 1 blackvuesync's -probe receives a 404 and falls back to legacy -- the existing suite stays green -with no emulator change. Task 2 then flips the emulator default to the new style, -which is green because blackvuesync already speaks it. No temporary defaults; -every commit is green. - -**Note:** `import json` is already present at `blackvuesync.py:33` -- do NOT re-add -it. There is no `from typing import Any`; annotate the JSON parser param as bare -`dict` (no new import needed). - ---- - -## Task 1: blackvuesync -- /accessible detection, /vodList parsing, legacy appendix - -**Goal:** `get_dashcam_filenames` speaks the new `/vodList` JSON API, falling back -to the legacy `blackvue_vod.cgi` path when `/accessible` is absent; the existing -behave suite stays green (emulator unchanged, so `/accessible` 404s → legacy). - -**Files:** - -- Modify: `blackvuesync.py` (around `get_filenames`/`get_dashcam_filenames`, lines 758-805) - -**Acceptance Criteria:** - -- [ ] `is_legacy_camera(base_url)` returns `False` on a `200` from `/accessible`, `True` on `HTTPError` (e.g. 404). -- [ ] The new-style branch fetches `/vodList`, parses JSON, and returns `[entry["filename"] for entry in data["filelist"]]`. -- [ ] The legacy branch is reached by an early return and preserves the existing `blackvue_vod.cgi` plaintext behavior byte-for-byte. -- [ ] Connection-level errors during the `/accessible` probe still surface as "Dashcam unavailable" via the existing handler. -- [ ] Existing `pytest` and `behave` suites remain green (blackvuesync falls back to legacy against the unchanged emulator). - -**Verify:** `pytest test/blackvuesync_test.py -q && behave --no-capture --format progress` → both green. - -**Steps:** - -- [ ] **Step 1: Add helpers and the new parser above `get_dashcam_filenames`.** - -Insert immediately after `get_filenames` (after line 766), before `get_dashcam_filenames`: - -```python -def _build_request(url: str) -> urllib.request.Request: - """builds a GET request, adding the affinity header when configured""" - request = urllib.request.Request(url) - if affinity_key: - request.add_header("X-Affinity-Key", affinity_key) - return request - - -def is_legacy_camera(base_url: str) -> bool: - """returns True for older cameras that lack the /accessible endpoint (pre-V1.009)""" - url = urllib.parse.urljoin(base_url, "accessible") - try: - with urllib.request.urlopen(_build_request(url)) as response: - return response.getcode() != 200 - except urllib.error.HTTPError: - return True - - -def get_filenames_from_json(data: dict) -> list[str]: - """extracts the recording filenames from the dashcam /vodList JSON response""" - return [entry["filename"] for entry in data["filelist"]] -``` - -- [ ] **Step 2: Rewrite `get_dashcam_filenames` to detect then branch.** - -Replace the body of `get_dashcam_filenames` (lines 769-805) with: - -```python -def get_dashcam_filenames(base_url: str) -> list[str]: - """gets the recording filenames from the dashcam""" - try: - if is_legacy_camera(base_url): - return get_dashcam_filenames_legacy(base_url) - - url = urllib.parse.urljoin(base_url, "vodList") - with urllib.request.urlopen(_build_request(url)) as response: - response_status_code = response.getcode() - if response_status_code != 200: - raise RuntimeError( - f"Error response from : {base_url} ; status code : {response_status_code}" - ) - - data = json.load(response) - - return get_filenames_from_json(data) - except urllib.error.URLError as e: - if isinstance(e.reason, OSError) and ( - isinstance(e.reason, (TimeoutError, socket.timeout)) - or e.reason.errno in dashcam_unavailable_errno_codes - ): - raise UserWarning(f"Dashcam unavailable : {e}") from e - - raise RuntimeError( - f"Cannot obtain list of recordings from dashcam at address : {base_url}; error : {e}" - ) from e - except socket.timeout as e: - raise UserWarning( - f"Timeout communicating with dashcam at address : {base_url}; error : {e}" - ) from e - except http.client.RemoteDisconnected as e: - raise UserWarning( - f"Dashcam disconnected without a response; address : {base_url}; error : {e}" - ) from e -``` - -- [ ] **Step 3: Add the legacy appendix function immediately after `get_dashcam_filenames`.** - -```python -def get_dashcam_filenames_legacy(base_url: str) -> list[str]: - """gets recording filenames from an older dashcam via the blackvue_vod.cgi index""" - url = urllib.parse.urljoin(base_url, "blackvue_vod.cgi") - with urllib.request.urlopen(_build_request(url)) as response: - response_status_code = response.getcode() - if response_status_code != 200: - raise RuntimeError( - f"Error response from : {base_url} ; status code : {response_status_code}" - ) - - charset = response.info().get_param("charset", "UTF-8") - file_lines = [x.decode(charset) for x in response.readlines()] - - return get_filenames(file_lines) -``` - -- [ ] **Step 4: Run the suites to confirm no regression.** - -Run: `pytest test/blackvuesync_test.py -q` -Expected: PASS (unchanged count). - -Run: `behave --no-capture --format progress` -Expected: PASS. blackvuesync now probes `/accessible`, the unchanged emulator returns 404, so it falls back to `blackvue_vod.cgi` -- identical end behavior. - -- [ ] **Step 5: Commit.** - -```bash -git add blackvuesync.py -git commit -m "Support the V1.009 /vodList index with legacy fallback" -``` - -(Include the repo's standard Co-Authored-By / Claude-Session trailers.) - ---- - -## Task 2: Mock dashcam -- /accessible, /vodList, legacy switch; default to new style - -**Goal:** The mock serves the new-style API by default (`/accessible` + JSON -`/vodList`), deprecates `blackvue_vod.cgi` with `415 "mp4 only"` when not legacy, -and exposes a per-session `legacy_api` switch. The full behave suite passes -against the new default because blackvuesync (Task 1) already speaks it. - -**Files:** - -- Modify: `features/mock_dashcam/server.py` - -**Acceptance Criteria:** - -- [ ] Per-session `legacy_api` boolean, defaulting to `False` (new style), with thread-safe getter/setter. -- [ ] `POST /mock/legacy-api` `{"legacy_api": bool}` sets the session flag; `clear_session` resets it. -- [ ] `GET /accessible` returns `200 {"accessible":"OK","message":"Access possible."}` when not legacy, `404` when legacy. -- [ ] `GET /vodList` returns `200 {"filelist":[{"filename": ...}, ...]}` when not legacy, `404` when legacy. -- [ ] `GET /blackvue_vod.cgi` returns the plaintext index when legacy, `415` body `mp4 only` when not legacy. -- [ ] Full `behave` suite green with the new default. - -**Verify:** `behave --no-capture --format progress` → green (existing features now exercise `/vodList` end-to-end). - -**Steps:** - -- [ ] **Step 1: Add per-session `legacy_api` state in `__init__`.** - -In `MockDashcam.__init__`, after the `self._download_errors_by_session` line (around line 92), add: - -```python - self._legacy_api_by_session: defaultdict[str, bool] = defaultdict(bool) -``` - -- [ ] **Step 2: Add thread-safe getter/setter** next to `_get_download_errors`/`_set_download_errors` (after line 124): - -```python - def _get_legacy_api(self, affinity_key: str) -> bool: - """thread-safe read access to the session-specific legacy-api flag""" - with self._sessions_lock: - return self._legacy_api_by_session[affinity_key] - - def _set_legacy_api(self, affinity_key: str, legacy_api: bool) -> None: - """thread-safe write access to the session-specific legacy-api flag""" - with self._sessions_lock: - self._legacy_api_by_session[affinity_key] = legacy_api -``` - -- [ ] **Step 3: Add `/accessible` and `/vodList` routes** inside `_setup_routes`, just before the existing `/blackvue_vod.cgi` route (before line 134): - -```python - @self.app.route("/accessible", methods=["GET"]) - def accessible() -> flask.Response | tuple[dict[str, str], int]: - """reports the new-style API is reachable; absent on legacy firmware""" - logger.debug("GET /accessible") - affinity_key = self._get_affinity_key() - if self._get_legacy_api(affinity_key): - return flask.abort(404) - return {"accessible": "OK", "message": "Access possible."}, 200 - - @self.app.route("/vodList", methods=["GET"]) - def vod_list() -> flask.Response | tuple[dict[str, Any], int]: - """returns the index of recordings as JSON (new style)""" - logger.debug("GET /vodList") - affinity_key = self._get_affinity_key() - if self._get_legacy_api(affinity_key): - return flask.abort(404) - recordings = self._get_recordings(affinity_key) - filelist = [{"filename": filename} for filename in recordings] - return {"filelist": filelist}, 200 -``` - -- [ ] **Step 4: Gate the existing `/blackvue_vod.cgi` route on the legacy flag.** - -In the `vod()` handler, right after `affinity_key = self._get_affinity_key()` (line 138), add: - -```python - if not self._get_legacy_api(affinity_key): - # firmware V1.009+ deprecated this endpoint - return flask.Response("mp4 only", status=415, mimetype="text/plain") -``` - -(The route's return type annotation becomes `flask.Response | str`.) - -- [ ] **Step 5: Add the `POST /mock/legacy-api` control route** next to the other `/mock/...` routes (after the `set_download_errors` block, around line 259): - -```python - @self.app.route("/mock/legacy-api", methods=["POST"]) - def set_legacy_api() -> tuple[dict[str, Any], int]: - """configures whether the session serves the legacy blackvue_vod.cgi index""" - data = flask.request.get_json() or {} - logger.debug("POST /mock/legacy-api") - logger.debug("Request body: %s", data) - affinity_key = self._get_affinity_key() - - legacy_api = bool(data.get("legacy_api", False)) - self._set_legacy_api(affinity_key, legacy_api) - - response = {"status": "configured", "legacy_api": legacy_api} - logger.debug("Response body: %s", response) - - return response, 201 -``` - -- [ ] **Step 6: Reset the flag in `clear_session`.** - -In `clear_session`, add to the `if affinity_key:` branch: - -```python - self._legacy_api_by_session[affinity_key] = False -``` - -and to the `else:` branch: - -```python - self._legacy_api_by_session.clear() -``` - -- [ ] **Step 7: Run the full suite.** - -Run: `behave --no-capture --format progress` -Expected: PASS. Existing features now default to the new style: blackvuesync probes `/accessible` (200), fetches `/vodList`, downloads via `/Record/`. - -- [ ] **Step 8: Commit.** - -```bash -git add features/mock_dashcam/server.py -git commit -m "Add new-style dashcam API to the mock with a legacy switch" -``` - ---- - -## Task 3: Parity feature and step - -**Goal:** A dedicated feature runs the identical sync over both the legacy and -new styles and asserts the same recordings download. - -**Files:** - -- Create: `features/sync_api_parity.feature` -- Modify: `features/steps/dashcam_recordings_steps.py` - -**Acceptance Criteria:** - -- [ ] `features/sync_api_parity.feature` is a Scenario Outline with `Examples` rows `true` and `false`. -- [ ] A `@given('the dashcam is legacy: {legacy_api}')` step POSTs `/mock/legacy-api` with the scenario affinity key. -- [ ] Both example rows pass: all recordings download under legacy and new styles. - -**Verify:** `behave features/sync_api_parity.feature --no-capture --format progress` → both scenarios pass. - -**Steps:** - -- [ ] **Step 1: Add the step** to `features/steps/dashcam_recordings_steps.py` (append after the last `@given`): - -```python -@given('the dashcam is legacy: {legacy_api}') -def dashcam_legacy_api(context: Context, legacy_api: str) -> None: - """configures whether the mock dashcam serves the legacy blackvue_vod.cgi index.""" - url = f"{context.mock_dashcam_url}/mock/legacy-api" - headers = {"X-Affinity-Key": context.scenario_token} - data = {"legacy_api": legacy_api.lower() == "true"} - - response = requests.post(url, json=data, headers=headers, timeout=10) - response.raise_for_status() -``` - -- [ ] **Step 2: Create `features/sync_api_parity.feature`:** - -```gherkin -Feature: API version parity - - Scenario Outline: sync yields the same recordings whether the camera is legacy - Given the dashcam is legacy: - Given recordings for the past "1d" of types "NE", directions "FR" - When blackvuesync runs - Then blackvuesync exits with code 0 - Then all the recordings are downloaded - - Examples: - | legacy_api | - | true | - | false | -``` - -- [ ] **Step 3: Run the parity feature.** - -Run: `behave features/sync_api_parity.feature --no-capture --format progress` -Expected: PASS -- 2 scenarios (legacy true and false), all steps passing. - -- [ ] **Step 4: Run the whole suite to confirm nothing else moved.** - -Run: `behave --no-capture --format progress` -Expected: PASS. - -- [ ] **Step 5: Commit.** - -```bash -git add features/sync_api_parity.feature features/steps/dashcam_recordings_steps.py -git commit -m "Add API version parity integration feature" -``` - ---- - -## Task 4: Changelog entry - -**Goal:** Record the new firmware support in the changelog. - -**Files:** - -- Modify: `CHANGELOG.md` - -**Acceptance Criteria:** - -- [ ] A bullet documents V1.009 `/vodList` JSON listing support with legacy fallback, referencing issue #87. - -**Verify:** `head -8 CHANGELOG.md` shows the new entry; `git diff --stat` shows only `CHANGELOG.md`. - -**Steps:** - -- [ ] **Step 1: Add a new version section** at the top of `CHANGELOG.md`, above `## 2.2.0`: - -```markdown -## 2.3.0 - -* Support BlackVue firmware V1.009+ cameras, which serve the recording index as JSON from `/vodList` instead of the legacy `blackvue_vod.cgi` endpoint. Older cameras continue to work unchanged. (#87) -``` - -(If `2.2.0` is still unreleased at execution time, fold the bullet into it instead of adding `2.3.0`.) - -- [ ] **Step 2: Commit.** - -```bash -git add CHANGELOG.md -git commit -m "Note V1.009 listing API support in the changelog" -``` - ---- - -## Verification summary - -- `pytest test/blackvuesync_test.py -q` -- unchanged, green. -- `behave --no-capture --format progress` -- green (existing features exercise the new style by default; parity feature covers both). -- `behave -D implementation=docker --no-capture --format progress` -- green (containerized mock). diff --git a/docs/plans/2026-07-02-firmware-v1009-listing-api.md b/docs/plans/2026-07-02-firmware-v1009-listing-api.md deleted file mode 100644 index 04d78fe..0000000 --- a/docs/plans/2026-07-02-firmware-v1009-listing-api.md +++ /dev/null @@ -1,172 +0,0 @@ -# Firmware V1.009 listing API support - -**Goal:** Restore recording sync for BlackVue cameras running firmware V1.009+ -(e.g. Elite 9), which replaced the plaintext `blackvue_vod.cgi` index with a JSON -`/vodList` endpoint, while keeping older cameras working unchanged. See issue #87. - -**Architecture:** The firmware change affects exactly one thing -- how the -recording index is fetched and parsed. The current style becomes the main path -in `get_dashcam_filenames`; the old `blackvue_vod.cgi` path becomes a -backwards-compatibility appendix reached by an early return. Everything -downstream (the `Recording` structure, `to_recording`, the filename regex, -grouping, retention, and file downloads via `/Record/`) is untouched, -because it all operates on a plain `list[str]` of filenames that is identical -between the two firmware styles. - -**Tech Stack:** Python 3.9+ stdlib only (`json` is added for parsing `/vodList`; -no third-party dependency). The mock dashcam server (Flask) gains the new -endpoints. Behave drives integration coverage; pytest is unchanged. - ---- - -## Background - -Findings were confirmed against a live V1.009 camera in issue #87: - -- The camera exposes `GET /accessible` for firmware detection. It returns - `200 {"accessible":"OK","message":"Access possible."}` on V1.009 and does not - exist (404) on older firmware. -- The recording index moved from `GET /blackvue_vod.cgi` (plaintext lines - `n:/Record/,s:`) to `GET /vodList`, which returns - `{"filelist":[{"filename":"20260702_141045_PF.mp4"}, ...]}`. Note the live - response carries only `filename` -- no `size`. -- On V1.009 the old `blackvue_vod.cgi` returns HTTP 415 with body `mp4 only`. -- File downloads are unchanged: recordings remain available at - `GET /Record/`, and direct file access confirmed working on V1.009. - -## Naming - -The current style is *the* style and carries no special name. The old style is -"legacy", expressed as a binary yes/no. Do not use the name "restful". The -endpoint URL literals -`/accessible` and `/vodList` are kept verbatim only because they are the actual -firmware wire endpoints, not names chosen here. - -## `blackvuesync.py` - -`get_dashcam_filenames(base_url) -> list[str]` keeps its public contract. -**Superseded:** the download follow-up changed the return to -`tuple[bool, list[str]]` so the legacy flag reaches the download path; see -`2026-07-05-firmware-v1009-download-metadata.md`. It is restructured to detect -once, then branch: - -```python -def get_dashcam_filenames(base_url): - try: - if is_legacy_camera(base_url): - return get_dashcam_filenames_legacy(base_url) # early return: appendix - - # the current style: GET /vodList -> JSON - url = urllib.parse.urljoin(base_url, "vodList") - with urllib.request.urlopen(_build_request(url)) as response: - response_status_code = response.getcode() - if response_status_code != 200: - raise RuntimeError(...) - data = json.load(response) - return get_filenames_from_json(data) - except urllib.error.URLError as e: - ...unchanged unavailable / RuntimeError handling... - except socket.timeout as e: - ...unchanged... - except http.client.RemoteDisconnected as e: - ...unchanged... -``` - -New and changed units: - -- `is_legacy_camera(base_url) -> bool` -- issues `GET /accessible` and returns - `True` when the response is not `200` (an `HTTPError` such as 404 means older - firmware). Connection-level errors (`URLError` without an HTTP status, - timeouts) are not caught here; they propagate to the shared handler in - `get_dashcam_filenames` and are reported as "dashcam unavailable" exactly as - today. -- `get_dashcam_filenames_legacy(base_url) -> list[str]` -- the existing - `blackvue_vod.cgi` request and `get_filenames(file_lines)` body, moved into a - plain function with no try/except of its own; it raises into the shared - handler above. -- `get_filenames_from_json(data) -> list[str]` -- pure parser returning - `[f["filename"] for f in data["filelist"]]`, mirroring the existing - `get_filenames`. A malformed response (missing `filelist`/`filename`) raises - and is surfaced as an error rather than silently returning an empty list; a - valid camera always returns the documented shape. (Inlined during - implementation: the comprehension lives directly in `get_dashcam_filenames`.) -- `_build_request(url)` -- small helper that builds a `urllib.request.Request` - and conditionally adds the `X-Affinity-Key` header. Used by the three - index-related requests (`/accessible`, `/vodList`, `blackvue_vod.cgi`) to - avoid copy-paste. The download path is left untouched. -- `get_filenames` and `file_line_re` (plaintext) are unchanged, used only by the - legacy appendix. -- Add `import json`. - -This is the whole blackvuesync change. The only genuinely new logic is the -`/accessible` probe and the JSON parse, both inherent to obtaining the index. - -## Mock dashcam server (`features/mock_dashcam/server.py`) - -Add a per-session `legacy_api` boolean (keyed by affinity key, like recordings), -**defaulting to `False`** (current style). A binary is sufficient. - -- `POST /mock/legacy-api` with `{"legacy_api": true|false}` sets the session flag. -- `GET /accessible` → `200 {"accessible":"OK","message":"Access possible."}` - when not legacy; `404` when legacy. -- `GET /vodList` → `200 {"filelist":[{"filename": f}, ...]}` when not legacy; - `404` when legacy. -- `GET /blackvue_vod.cgi` → existing plaintext when legacy; `415` with body - `mp4 only` when not legacy (faithful to real V1.009, and guards against - accidental use). -- `GET /Record/` -- unchanged. - -Because the default is the current style, all five existing feature files run -against `/vodList` with no edits, and their assertions still pass (identical -filenames). - -## Integration tests - -New `features/sync_api_parity.feature` runs the identical sync over both styles: - -```gherkin -Feature: API version parity - - Scenario Outline: sync yields the same recordings whether the camera is legacy - Given the dashcam is legacy: - Given recordings for the past "1d" of types "NE", directions "FR" - When blackvuesync runs - Then blackvuesync exits with code 0 - Then all the recordings are downloaded - - Examples: - | legacy_api | - | true | - | false | -``` - -New step `@given('the dashcam is legacy: {legacy_api}')` parses the boolean and -POSTs `/mock/legacy-api` with the scenario's affinity key. It lives in -`dashcam_recordings_steps.py` (dashcam setup) and follows the feature -conventions (lowercase, present tense, "the dashcam"). - -## Testing and verification - -- Primary coverage is integration, matching the current suite (which has no - `get_filenames` unit tests -- index parsing is integration-tested): - - The parity feature exercises both parsers end-to-end. - - The default (current-style) suite exercises the new path across all - behaviors (basic, filter, retention, retry, skip-metadata). -- Verify: `behave` green; `behave -D implementation=docker` green; existing - `pytest test/blackvuesync_test.py` unchanged. - -## Docs - -- `CHANGELOG.md`: one entry noting V1.009 JSON listing support with legacy - fallback (issue #87). - -## Out of scope - -- No change to the download path; `/Record/` is confirmed unchanged on - V1.009. **Superseded:** issue #87 later showed downloads *did* change on - V1.009 (video at the server root, metadata via `/vodMetadata`). See - `2026-07-05-firmware-v1009-download-metadata.md`. -- No auth/HTTPS handling changes. The app probes HTTP then HTTPS; blackvuesync - targets HTTP as today. -- The `size` field is not consumed (it is absent from live `/vodList` anyway); - real file sizes continue to come from `Content-Length` at download time. diff --git a/docs/plans/2026-07-05-firmware-v1009-download-metadata-plan.md b/docs/plans/2026-07-05-firmware-v1009-download-metadata-plan.md deleted file mode 100644 index 20b9344..0000000 --- a/docs/plans/2026-07-05-firmware-v1009-download-metadata-plan.md +++ /dev/null @@ -1,849 +0,0 @@ -# Firmware V1.009 download path and metadata Implementation Plan - -> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers-extended-cc:subagent-driven-development (recommended) or superpowers-extended-cc:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. - -**Goal:** Restore recording downloads on BlackVue firmware V1.009+ cameras by fetching videos from the server root and thumbnail/GPS data from the `/vodMetadata` endpoint, while leaving legacy cameras unchanged. - -**Architecture:** A single `legacy: bool` (produced by the existing `/accessible` probe) drives the download path. `get_dashcam_filenames` returns `(legacy, filenames)` so the flag reaches `download_recording`/`download_file` without a second probe. Non-legacy video downloads drop the `/Record/` prefix; non-legacy thumbnail/GPS come from a new `download_metadata_file` (POST + base64-decode); the `.3gf` accelerometer file is skipped on non-legacy firmware. The Flask mock gains root video serving and a `/vodMetadata` endpoint, and the test suite's expectations become legacy-aware. - -**Tech Stack:** Python 3.9+ stdlib only (`base64` added; `json` already imported). Behave integration tests + Flask mock dashcam; pytest unit tests. - -**User decisions (already made):** - -- "B" -- recover thumbnail + GPS via `/vodMetadata`; skip `.3gf` on V1.009. -- ".3gf" skipped on the new API with a debug-level log ("get cracking" approved this). -- Terminology: "metadata" / "metadata file"; never "sidecar". Helper named `download_metadata_file`. -- No mention of reverse engineering / decompiling in any docs. - ---- - -## Design refinement vs. spec - -The spec (`2026-07-05-firmware-v1009-download-metadata.md`) proposed hoisting `is_legacy_camera()` into `sync()` and passing `legacy` as a parameter to `get_dashcam_filenames`. Implementation review found this breaks two unit tests that monkeypatch `get_dashcam_filenames` by name (the real `is_legacy_camera` would then fire against a dead address). This plan instead has `get_dashcam_filenames` **return** `(legacy, filenames)`, keeping the probe where it already lives and requiring no pytest changes. Behavior is identical; only the internal wiring differs. - -## File structure - -- `blackvuesync.py` -- the application. Adds `legacy` wiring, root video URL, `download_metadata_file`, `.3gf` skip. -- `features/mock_dashcam/server.py` -- mock dashcam. Adds root video route, `/vodMetadata`, and (for fidelity) makes `/Record/` reject non-legacy. -- `features/lib/recordings.py` -- adds one pure helper for legacy-aware expectations. -- `features/environment.py` -- one default (`context.legacy_api`). -- `features/steps/dashcam_recordings_steps.py` -- stores the legacy flag in context. -- `features/steps/downloaded_recordings_steps.py`, `features/steps/retry_failed_steps.py` -- apply the legacy-aware filter. -- `features/steps/metadata_content_steps.py` -- new, one byte-comparison assertion. -- `features/sync_api_parity.feature` -- one focused non-legacy scenario. -- `CHANGELOG.md` -- one entry. - ---- - -### Task 1: Mock dashcam -- add V1.009 download endpoints - -**Goal:** Give the mock the ability to serve videos from the root and thumbnail/GPS from `/vodMetadata`, without changing any existing behavior. - -**Files:** - -- Modify: `features/mock_dashcam/server.py` - -**Acceptance Criteria:** - -- [ ] A shared `_serve_recording_file(affinity_key, filename)` method holds the 404/500/`send_file` logic; `/Record/` uses it. -- [ ] `GET /` serves the video for non-legacy sessions, returns `415 mp4 only` for non-`.mp4` names, and honors configured download errors. -- [ ] `POST /vodMetadata` returns `{"resultcode":"BC_ERR_OK","thumbnail":""}` for `thumbnail` and `{"resultcode":"BC_ERR_OK","metadata":{"gps":""}}` for `gps`, base64-encoding the matching `mock.` fixture. -- [ ] The full existing suite still passes (new routes are unused by current code). - -**Verify:** `behave` → all scenarios pass. - -**Steps:** - -- [ ] **Step 1: Add the base64 import** - -In `features/mock_dashcam/server.py`, add `import base64` with the other stdlib imports (after `import datetime`): - -```python -import base64 -import datetime -``` - -- [ ] **Step 2: Extract the shared file-serving helper** - -Add this method to `MockDashcam` (next to `_set_legacy_api`, before `_setup_routes`): - -```python - def _serve_recording_file( - self, affinity_key: str, filename: str - ) -> flask.Response: - """serves the mock file for a recording, or aborts 404/500 as configured""" - recordings = self._get_recordings(affinity_key) - if filename not in recordings: - logger.debug("Response: 404 Not Found (not in session recordings)") - return flask.abort(404) - - download_errors = self._get_download_errors(affinity_key) - if filename in download_errors: - logger.debug("Response: 500 Internal Server Error (configured error)") - flask.abort(500) - - if recording := to_recording(filename): - files_dir = Path(__file__).parent / "files" - filepath = files_dir / f"mock.{recording.extension}" - if filepath.exists(): - logger.debug( - "Response: %s (%s bytes)", filepath.name, filepath.stat().st_size - ) - return flask.send_file(filepath) - - logger.debug("Response: 404 Not Found") - return flask.abort(404) -``` - -- [ ] **Step 3: Refactor `/Record/` to use the helper** - -Replace the body of the `record` route (currently the recordings-check / download-errors / `to_recording` / `send_file` block) so it reads: - -```python - @self.app.route("/Record/", methods=["GET"]) - def record(filename: str) -> flask.Response: - """serves any file associated to recordings""" - logger.debug("GET /Record/%s", filename) - affinity_key = self._get_affinity_key() - return self._serve_recording_file(affinity_key, filename) -``` - -- [ ] **Step 4: Run the suite -- refactor is behavior-preserving** - -Run: `behave` -Expected: all scenarios pass (the `/Record/` refactor changed nothing observable). - -- [ ] **Step 5: Add the root video route and `/vodMetadata`** - -Add both routes inside `_setup_routes` (e.g. right after the `record` route). The root route must be registered after the static routes it must not shadow -- it is, because all of `/accessible`, `/vodList`, `/blackvue_vod.cgi`, `/vodMetadata`, `/mock/...` are static and win over the single-segment converter. - -```python - @self.app.route("/vodMetadata", methods=["POST"]) - def vod_metadata() -> flask.Response | tuple[dict[str, Any], int]: - """returns base64-encoded thumbnail/gps metadata (new style)""" - logger.debug("POST /vodMetadata") - affinity_key = self._get_affinity_key() - if self._get_legacy_api(affinity_key): - return flask.abort(404) - - data = flask.request.get_json() or {} - logger.debug("Request body: %s", data) - video_filename = data.get("file") - types = data.get("types", []) - metadata_type = types[0] if types else None - - recordings = self._get_recordings(affinity_key) - extension = {"thumbnail": "thm", "gps": "gps"}.get(metadata_type) - if video_filename not in recordings or extension is None: - return {"resultcode": "BC_ERR_NG"}, 200 - - files_dir = Path(__file__).parent / "files" - encoded = base64.b64encode( - (files_dir / f"mock.{extension}").read_bytes() - ).decode("ascii") - - # thumbnail sits at the top level; gps is nested under "metadata" - if metadata_type == "gps": - return {"resultcode": "BC_ERR_OK", "metadata": {"gps": encoded}}, 200 - return {"resultcode": "BC_ERR_OK", "thumbnail": encoded}, 200 - - @self.app.route("/", methods=["GET"]) - def root_record(filename: str) -> flask.Response: - """serves videos from the root on V1.009; rejects non-mp4 with 'mp4 only'""" - logger.debug("GET /%s", filename) - affinity_key = self._get_affinity_key() - if self._get_legacy_api(affinity_key): - # legacy firmware serves downloads only under /Record/ - return flask.abort(404) - if not filename.endswith(".mp4"): - return flask.Response("mp4 only", status=415, mimetype="text/plain") - return self._serve_recording_file(affinity_key, filename) -``` - -- [ ] **Step 6: Run the suite -- new routes are additive** - -Run: `behave` -Expected: all scenarios pass (current blackvuesync still downloads via `/Record/`, so the new routes are unused). - -- [ ] **Step 7: Commit** - -```bash -git add features/mock_dashcam/server.py -git commit -m "test: add V1.009 root video and vodMetadata routes to mock dashcam" -``` - ---- - -### Task 2: Legacy-aware test expectations - -**Goal:** Make the shared "recordings downloaded" assertions expect no `.3gf` on non-legacy cameras (the default), so the suite stays correct once blackvuesync stops fetching it. - -**Files:** - -- Modify: `features/environment.py` -- Modify: `features/steps/dashcam_recordings_steps.py:116-125` -- Modify: `features/lib/recordings.py` -- Modify: `features/steps/downloaded_recordings_steps.py:90-125` -- Modify: `features/steps/retry_failed_steps.py:68-83` - -**Acceptance Criteria:** - -- [ ] `context.legacy_api` defaults to `False` and is set to the scenario's value by the `the dashcam is legacy:` step. -- [ ] A pure `filter_available_metadata_filenames(filenames, legacy)` drops `.3gf` entries when `legacy` is `False`. -- [ ] `all the recordings are downloaded` and `the successful recordings are downloaded` both apply the filter. -- [ ] The full existing suite still passes. - -**Verify:** `behave` → all scenarios pass. - -**Steps:** - -- [ ] **Step 1: Default `context.legacy_api` in `before_scenario`** - -In `features/environment.py`, next to the existing `context.skip_metadata = set()` line, add: - -```python - # legacy_api defaults to False (current firmware); the legacy step overrides it - context.legacy_api = False -``` - -- [ ] **Step 2: Store the flag in the legacy step** - -In `features/steps/dashcam_recordings_steps.py`, update `dashcam_legacy_api` to record the flag in context: - -```python -@given("the dashcam is legacy: {legacy_api}") -def dashcam_legacy_api(context: Context, legacy_api: str) -> None: - """configures whether the mock dashcam serves the legacy blackvue_vod.cgi index.""" - context.legacy_api = legacy_api.lower() == "true" - url = f"{context.mock_dashcam_url}/mock/legacy-api" - headers = {"X-Affinity-Key": context.scenario_token} - data = {"legacy_api": context.legacy_api} - - response = requests.post(url, json=data, headers=headers, timeout=10) - response.raise_for_status() -``` - -- [ ] **Step 3: Add the pure filter helper** - -In `features/lib/recordings.py`, add (e.g. after `get_mock_file_for_extension`): - -```python -def filter_available_metadata_filenames( - filenames: list[str] | set[str], legacy: bool -) -> set[str]: - """returns the filenames a camera actually serves. - - V1.009+ (non-legacy) cameras have no accelerometer endpoint, so .3gf files - are never retrievable there. - """ - if legacy: - return set(filenames) - return {filename for filename in filenames if not filename.endswith(".3gf")} -``` - -- [ ] **Step 4: Apply the filter in `all the recordings are downloaded`** - -In `features/steps/downloaded_recordings_steps.py`, import the helper and filter `expected_recordings` before the skip-metadata filtering. Update the import near the top: - -```python -from features.lib.recordings import ( - create_recording_files, - filter_available_metadata_filenames, -) -``` - -and change the expected-set construction inside `assert_all_recordings_downloaded`: - -```python - # gets expected recordings, dropping metadata the camera can't serve - expected_recordings = filter_available_metadata_filenames( - context.expected_recordings, context.legacy_api - ) -``` - -(Leave the subsequent `skip_extensions` block exactly as-is; it further narrows this set.) - -- [ ] **Step 5: Apply the filter in `the successful recordings are downloaded`** - -In `features/steps/retry_failed_steps.py`, add the import: - -```python -from features.lib.recordings import filter_available_metadata_filenames -``` - -and update the `successful` set in `assert_successful_recordings_downloaded`: - -```python - failed: set[str] = getattr(context, "failed_recordings", set()) - available = filter_available_metadata_filenames( - context.expected_recordings, context.legacy_api - ) - successful = {f for f in available if f not in failed} -``` - -- [ ] **Step 6: Run the suite** - -Run: `behave` -Expected: all scenarios pass. (`.3gf` is still fetched via `/Record/` by current blackvuesync, so it is present but no longer *required* -- `has_items` is a subset check.) - -- [ ] **Step 7: Commit** - -```bash -git add features/environment.py features/steps/dashcam_recordings_steps.py features/lib/recordings.py features/steps/downloaded_recordings_steps.py features/steps/retry_failed_steps.py -git commit -m "test: make recording expectations legacy-aware for .3gf" -``` - ---- - -### Task 3: blackvuesync -- non-legacy download path - -**Goal:** Download videos from the root and thumbnail/GPS from `/vodMetadata` on non-legacy cameras, skip `.3gf` there, and make the mock reject `/Record/` on non-legacy so the change is actually exercised. - -**Files:** - -- Modify: `blackvuesync.py` (imports; `get_dashcam_filenames:803-819`; `download_file:960-1024`; `download_recording:1112-1187`; `sync:1471-1493`) -- Modify: `features/mock_dashcam/server.py` (`record` route) - -**Acceptance Criteria:** - -- [ ] `get_dashcam_filenames` returns `(legacy, filenames)`; `sync` unpacks it and threads `legacy` into every `download_recording`. -- [ ] `download_file` builds the URL at the root when `legacy` is `False`, and under `Record/` when `True`. -- [ ] On non-legacy cameras, thumbnail and GPS are fetched via `download_metadata_file` (POST `/vodMetadata`, base64-decoded), gated by `--skip-metadata` `t`/`g`; `.3gf` is skipped with a `logger.debug` note. -- [ ] The mock returns `400` for `/Record/` on non-legacy sessions. -- [ ] `behave` and `behave -D implementation=docker` pass; `pytest test/blackvuesync_test.py` passes unchanged. - -**Verify:** `behave && pytest test/blackvuesync_test.py -q` - -**Steps:** - -- [ ] **Step 1: Make the mock reject `/Record/` on non-legacy (the failing setup)** - -In `features/mock_dashcam/server.py`, add a legacy guard to the `record` route so V1.009 sessions no longer serve `/Record/`: - -```python - @self.app.route("/Record/", methods=["GET"]) - def record(filename: str) -> flask.Response: - """serves any file associated to recordings (legacy firmware only)""" - logger.debug("GET /Record/%s", filename) - affinity_key = self._get_affinity_key() - if not self._get_legacy_api(affinity_key): - # firmware V1.009+ moved downloads to the root - return flask.abort(400) - return self._serve_recording_file(affinity_key, filename) -``` - -- [ ] **Step 2: Run the suite to confirm it now fails** - -Run: `behave` -Expected: FAIL -- non-legacy scenarios (the default, plus the `legacy: false` parity row) can no longer download via `/Record/`. This is the red state the code change turns green. - -- [ ] **Step 3: Add the base64 import** - -In `blackvuesync.py`, add `import base64` in the stdlib import block (alphabetical, before `import datetime`/`import json`): - -```python -import base64 -``` - -- [ ] **Step 4: Return `(legacy, filenames)` from `get_dashcam_filenames`** - -Change the signature and the two success `return`s (the `try` body only; the `except` handlers are unchanged): - -```python -def get_dashcam_filenames(base_url: str) -> tuple[bool, list[str]]: - """gets whether the camera is legacy and its recording filenames""" - try: - legacy = is_legacy_camera(base_url) - if legacy: - return legacy, get_dashcam_filenames_legacy(base_url) - - url = urllib.parse.urljoin(base_url, "vodList") - with urllib.request.urlopen(_build_request(url)) as response: - response_status_code = response.getcode() - if response_status_code != 200: - raise RuntimeError( - f"Error response from : {base_url} ; status code : {response_status_code}" - ) - - data = json.load(response) - - return legacy, [entry["filename"] for entry in data["filelist"]] - except urllib.error.URLError as e: - # ...unchanged... -``` - -- [ ] **Step 5: Add the `legacy` parameter and root URL to `download_file`** - -Change the signature (add `legacy` after `metrics`) and the URL construction at `blackvuesync.py:1024`: - -```python -def download_file( - base_url: str, - filename: str, - destination: str, - group_name: str | None, - metrics: SyncMetrics | None = None, - legacy: bool = False, -) -> tuple[bool, int | None]: -``` - -```python - # V1.009+ serves videos from the root; legacy firmware under /Record/ - url = urllib.parse.urljoin( - base_url, f"Record/{filename}" if legacy else filename - ) -``` - -- [ ] **Step 6: Add `download_metadata_file`** - -Add this new function immediately before `download_recording` (around `blackvuesync.py:1112`): - -```python -def download_metadata_file( - base_url: str, - video_filename: str, - metadata_filename: str, - metadata_type: str, - destination: str, - group_name: str | None, - metrics: SyncMetrics | None = None, -) -> tuple[bool, int | None]: - """downloads thumbnail or gps data via the /vodMetadata endpoint (V1.009+)""" - # pylint: disable=too-many-branches,too-many-return-statements - if group_name: - ensure_destination(os.path.join(destination, group_name)) - - destination_filepath = get_filepath(destination, group_name, metadata_filename) - if os.path.exists(destination_filepath): - logger.debug( - "Ignoring already downloaded file : %s", - metadata_filename, - extra={ - "event": "file_already_downloaded", - "recording_filename": metadata_filename, - "destination_path": destination_filepath, - }, - ) - return False, None - - if dry_run: - logger.debug( - "DRY RUN Would download file : %s", - metadata_filename, - extra={"event": "file_download_dry_run", "recording_filename": metadata_filename}, - ) - return True, None - - if is_download_blocked_by_failure(destination, group_name, metadata_filename): - logger.debug( - "Skipping recently failed download : %s", - metadata_filename, - extra={"event": "file_download_recently_failed", "recording_filename": metadata_filename}, - ) - return False, None - - remove_download_failed_marker(destination, group_name, metadata_filename) - - temp_filepath = os.path.join(destination, f".{metadata_filename}") - try: - url = urllib.parse.urljoin(base_url, "vodMetadata") - body = json.dumps({"file": video_filename, "types": [metadata_type]}).encode("utf-8") - request = urllib.request.Request(url, data=body, method="POST") - request.add_header("Content-Type", "application/json") - if affinity_key: - request.add_header("X-Affinity-Key", affinity_key) - - with urllib.request.urlopen(request) as response: - data = json.load(response) - - if data.get("resultcode") != "BC_ERR_OK": - cron_logger.warning( - "Could not download file : %s; metadata result : %s; ignoring.", - metadata_filename, - data.get("resultcode"), - extra={ - "event": "file_download_failed", - "recording_filename": metadata_filename, - "error": str(data.get("resultcode")), - }, - ) - return False, None - - # the payload sits at the top level, or nested under "metadata" - encoded = data.get(metadata_type) - if encoded is None and isinstance(data.get("metadata"), dict): - encoded = data["metadata"].get(metadata_type) - if not encoded: - cron_logger.warning( - "Metadata response missing %s data : %s; ignoring.", - metadata_type, - metadata_filename, - extra={ - "event": "file_download_failed", - "recording_filename": metadata_filename, - "error": "missing metadata payload", - }, - ) - return False, None - - content = base64.b64decode(encoded) - with open(temp_filepath, "wb") as f: - f.write(content) - os.rename(temp_filepath, destination_filepath) - - logger.debug( - "Downloaded file : %s", - metadata_filename, - extra={ - "event": "file_downloaded", - "recording_filename": metadata_filename, - "destination_path": destination_filepath, - "content_length_bytes": len(content), - }, - ) - if metrics: - metrics.record_file_download(len(content)) - return True, None - except urllib.error.HTTPError as e: - cron_logger.warning( - "Could not download file : %s; error : %s; ignoring.", - metadata_filename, - e, - extra={ - "event": "file_download_failed", - "recording_filename": metadata_filename, - "error_type": type(e).__name__, - "error": str(e), - }, - ) - if metrics: - metrics.record_file_download_failure("http") - mark_download_failed(destination, group_name, metadata_filename) - return False, None - except urllib.error.URLError as e: - cron_logger.warning( - "Could not download file : %s; error : %s; ignoring.", - metadata_filename, - e, - extra={ - "event": "file_download_failed", - "recording_filename": metadata_filename, - "error_type": type(e).__name__, - "error": str(e), - }, - ) - if metrics: - metrics.record_file_download_failure("network") - return False, None - except socket.timeout as e: - if metrics: - metrics.record_file_download_failure("timeout") - raise UserWarning( - f"Timeout communicating with dashcam at address : {base_url}; error : {e}" - ) from e - except ValueError as e: - # malformed JSON or base64; non-fatal, mirrors a failed metadata fetch - cron_logger.warning( - "Could not parse metadata response : %s; error : %s; ignoring.", - metadata_filename, - e, - extra={ - "event": "file_download_failed", - "recording_filename": metadata_filename, - "error_type": type(e).__name__, - "error": str(e), - }, - ) - return False, None -``` - -- [ ] **Step 7: Thread `legacy` through `download_recording` and route metadata** - -Change the `download_recording` signature to accept `legacy`: - -```python -def download_recording( - base_url: str, - recording: Recording, - destination: str, - metrics: SyncMetrics | None = None, - legacy: bool = False, -) -> None: -``` - -Pass `legacy` to the video download: - -```python - downloaded, speed_bps = download_file( - base_url, filename, destination, recording.group_name, metrics, legacy - ) - any_downloaded |= downloaded -``` - -Route the thumbnail (replace the `download_file` call inside the `"t" not in skip_metadata` branch): - -```python - if "t" not in skip_metadata: - thm_filename = ( - f"{recording.base_filename}_{recording.type}{recording.direction}.thm" - ) - if legacy: - downloaded, _ = download_file( - base_url, thm_filename, destination, recording.group_name, metrics, legacy - ) - else: - downloaded, _ = download_metadata_file( - base_url, - recording.filename, - thm_filename, - "thumbnail", - destination, - recording.group_name, - metrics, - ) - any_downloaded |= downloaded - else: - logger.debug( - "Skipping thumbnail : %s (--skip-metadata)", - recording.base_filename, - extra={ - "event": "metadata_skipped", - "metadata_type": "thumbnail", - "recording_base_filename": recording.base_filename, - }, - ) -``` - -Replace the accelerometer block so it only runs on legacy cameras: - -```python - # downloads the accelerometer data (legacy firmware only; V1.009+ has no endpoint) - if legacy: - if "3" not in skip_metadata: - tgf_filename = f"{recording.base_filename}_{recording.type}.3gf" - downloaded, _ = download_file( - base_url, tgf_filename, destination, recording.group_name, metrics, legacy - ) - any_downloaded |= downloaded - else: - logger.debug( - "Skipping accelerometer : %s (--skip-metadata)", - recording.base_filename, - extra={ - "event": "metadata_skipped", - "metadata_type": "accelerometer", - "recording_base_filename": recording.base_filename, - }, - ) - else: - logger.debug( - "Skipping accelerometer : %s (unavailable on this firmware)", - recording.base_filename, - extra={ - "event": "metadata_skipped", - "metadata_type": "accelerometer", - "recording_base_filename": recording.base_filename, - }, - ) -``` - -Route the gps download (replace the `download_file` call inside the `"g" not in skip_metadata` branch): - -```python - if "g" not in skip_metadata: - gps_filename = f"{recording.base_filename}_{recording.type}.gps" - if legacy: - downloaded, _ = download_file( - base_url, gps_filename, destination, recording.group_name, metrics, legacy - ) - else: - downloaded, _ = download_metadata_file( - base_url, - recording.filename, - gps_filename, - "gps", - destination, - recording.group_name, - metrics, - ) - any_downloaded |= downloaded - else: - logger.debug( - "Skipping gps : %s (--skip-metadata)", - recording.base_filename, - extra={ - "event": "metadata_skipped", - "metadata_type": "gps", - "recording_base_filename": recording.base_filename, - }, - ) -``` - -- [ ] **Step 8: Unpack `legacy` in `sync` and pass it down** - -At `blackvuesync.py:1472` and `:1493`: - -```python - base_url = f"http://{address}" - legacy, dashcam_filenames = get_dashcam_filenames(base_url) -``` - -```python - for recording in current_dashcam_recordings: - download_recording(base_url, recording, destination, metrics, legacy) -``` - -- [ ] **Step 9: Run both test suites** - -Run: `behave` -Expected: all scenarios pass -- non-legacy downloads now use the root and `/vodMetadata`. - -Run: `pytest test/blackvuesync_test.py -q` -Expected: all pass unchanged (the monkeypatched `get_dashcam_filenames` stubs raise before the tuple is unpacked; the direct `download_file` calls use the `legacy=False` default). - -- [ ] **Step 10: Run the docker integration suite** - -Run: `behave -D implementation=docker` -Expected: all scenarios pass (exercises the same paths against the containerized mock). - -- [ ] **Step 11: Commit** - -```bash -git add blackvuesync.py features/mock_dashcam/server.py -git commit -m "feat: download from root and vodMetadata on V1.009 cameras (#87)" -``` - ---- - -### Task 4: Parity feature -- verify metadata content on V1.009 - -**Goal:** Prove the `/vodMetadata` path writes correct thumbnail/GPS bytes and no `.3gf` on a non-legacy camera. - -**Files:** - -- Create: `features/steps/metadata_content_steps.py` -- Modify: `features/sync_api_parity.feature` - -**Acceptance Criteria:** - -- [ ] A `the downloaded "{extension}" files match the mock fixture` step compares each downloaded `.{extension}` file's bytes to `features/mock_dashcam/files/mock.{extension}`. -- [ ] A non-legacy scenario asserts thumbnail and gps files match the fixtures and no `.3gf` files exist. -- [ ] `behave features/sync_api_parity.feature` passes. - -**Verify:** `behave features/sync_api_parity.feature` - -**Steps:** - -- [ ] **Step 1: Add the byte-comparison step** - -Create `features/steps/metadata_content_steps.py`: - -```python -"""metadata content verification step definitions""" - -from pathlib import Path - -from behave import then -from behave.runner import Context -from hamcrest import assert_that, equal_to, not_ - -_MOCK_FILES_DIR = Path(__file__).parent.parent / "mock_dashcam" / "files" - - -@then('the downloaded "{extension}" files match the mock fixture') -def assert_downloaded_files_match_fixture(context: Context, extension: str) -> None: - """verifies every downloaded file of the given extension equals the mock fixture.""" - expected_bytes = (_MOCK_FILES_DIR / f"mock.{extension}").read_bytes() - - downloaded = [ - f for f in context.dest_dir.rglob(f"*.{extension}") if f.is_file() - ] - assert_that(downloaded, not_(equal_to([]))) - for downloaded_file in downloaded: - assert_that(downloaded_file.read_bytes(), equal_to(expected_bytes)) -``` - -- [ ] **Step 2: Add the non-legacy content scenario** - -Append to `features/sync_api_parity.feature` (sentence-case name, lowercase steps, no `And`): - -```gherkin - Scenario: sync retrieves thumbnail and gps metadata on V1.009 cameras - Given the dashcam is legacy: false - Given recordings for the past "1d" of types "N", directions "F" - When blackvuesync runs - Then blackvuesync exits with code 0 - Then all the recordings are downloaded - Then the downloaded "thm" files match the mock fixture - Then the downloaded "gps" files match the mock fixture - Then the destination contains no "3gf" files -``` - -- [ ] **Step 3: Run the parity feature** - -Run: `behave features/sync_api_parity.feature` -Expected: all scenarios pass, including the new content assertions. - -- [ ] **Step 4: Commit** - -```bash -git add features/steps/metadata_content_steps.py features/sync_api_parity.feature -git commit -m "test: verify V1.009 thumbnail/gps content and no .3gf" -``` - ---- - -### Task 5: Documentation - -**Goal:** Record the V1.009 download support in the changelog. - -**Files:** - -- Modify: `CHANGELOG.md` - -**Acceptance Criteria:** - -- [ ] The `2.2.0` section notes root video downloads plus thumbnail/GPS via the metadata endpoint, with accelerometer unavailable on that firmware, referencing #87. - -**Verify:** `git diff CHANGELOG.md` shows the new entry; `behave && pytest test/blackvuesync_test.py -q` still green. - -**Steps:** - -- [ ] **Step 1: Add the changelog entry** - -In `CHANGELOG.md`, under `## 2.2.0`, add a bullet after the existing `/vodList` entry: - -```markdown -* Download recordings from BlackVue firmware V1.009+ cameras, which serve videos from the root path and provide thumbnail and GPS data through a metadata endpoint. Accelerometer data is unavailable on that firmware. (#87) -``` - -- [ ] **Step 2: Final full verification** - -Run: `behave && pytest test/blackvuesync_test.py -q` -Expected: all green. - -- [ ] **Step 3: Commit** - -```bash -git add CHANGELOG.md -git commit -m "docs: note V1.009 download support in changelog (#87)" -``` - ---- - -## Self-Review - -**Spec coverage:** - -- Root video path → Task 3 (Step 5). ✓ -- `/vodMetadata` thumbnail/GPS (base64, tolerant) → Task 3 (Step 6, `download_metadata_file`). ✓ -- `.3gf` skipped on non-legacy with debug log → Task 3 (Step 7). ✓ -- `legacy` from a single probe → Task 3 (Step 4, returned tuple; refinement documented). ✓ -- Mock root serving + `/vodMetadata` + non-mp4 `415` + `/Record/` fidelity → Tasks 1 & 3. ✓ -- Parity coverage incl. metadata outcome and `.3gf` absence → Tasks 2 & 4. ✓ -- CHANGELOG + earlier-doc correction → Task 5 (correction already applied in the spec commit). ✓ -- Risk (vodMetadata not browser-verified; tolerant path) → realized as non-fatal returns in `download_metadata_file`. ✓ - -**Placeholder scan:** No TBD/TODO; every code step shows complete code. ✓ - -**Type consistency:** `get_dashcam_filenames -> tuple[bool, list[str]]` unpacked as `legacy, filenames` in `sync`. `download_file(..., legacy=False)` and `download_recording(..., legacy=False)` signatures match their call sites. `download_metadata_file` returns `tuple[bool, int | None]`, consumed as `downloaded, _`. `filter_available_metadata_filenames(filenames, legacy) -> set[str]` matches both call sites. Mock helper `_serve_recording_file(affinity_key, filename)` matches its two callers. ✓ diff --git a/docs/plans/2026-07-05-firmware-v1009-download-metadata.md b/docs/plans/2026-07-05-firmware-v1009-download-metadata.md deleted file mode 100644 index 19762f2..0000000 --- a/docs/plans/2026-07-05-firmware-v1009-download-metadata.md +++ /dev/null @@ -1,170 +0,0 @@ -# Firmware V1.009 download path and metadata support - -**Goal:** Restore actual recording downloads for BlackVue cameras running -firmware V1.009+ (e.g. Elite 9). The listing side (`/vodList`) already landed -(see `2026-07-02-firmware-v1009-listing-api.md`), but downloads still fail on -V1.009 because the firmware moved video files and dropped plain metadata-file -access. See issue #87. - -**Architecture:** The firmware change affects the download path only. Video -files moved from `/Record/` to the server root `/`, and the -`.thm`/`.gps`/`.3gf` metadata files are no longer served as plain files -- a -plain `GET` of a non-`.mp4` name returns `415 mp4 only`. Thumbnail and GPS data -are instead obtained from the firmware's `POST /vodMetadata` endpoint (the same -endpoint the BlackVue mobile app uses), which returns the data base64-encoded in -JSON. The accelerometer `.3gf` has no `/vodMetadata` counterpart and is not -retrievable on V1.009; it is skipped there. Legacy cameras are unchanged. - -**Tech Stack:** Python 3.9+ stdlib only (`base64` is added for decoding the -metadata payloads; `json` is already imported). The mock dashcam server (Flask) -gains root-served videos and the `/vodMetadata` endpoint. Behave drives -integration coverage; pytest is unchanged. - ---- - -## Background - -The listing-API doc assumed downloads were unchanged on V1.009. Issue #87 has -since falsified that. Confirmed against the reporter's live V1.009 camera -(browser address-bar `GET`s) and consistent with the BlackVue mobile app's -behavior: - -- **Video moved to root.** `GET /Record/.mp4` returns `400`; - `GET /.mp4` (server root) downloads the video. The reporter also - confirmed raw stream access works. -- **Metadata files are not served as plain files.** `GET /.thm` and - `GET /.3gf` at the root both return `415 mp4 only` -- the root file - server serves `.mp4` only. -- **Thumbnail and GPS come from `POST /vodMetadata`.** For each recording the - app issues one request per metadata type (they are never bundled): - - ```text - POST /vodMetadata Content-Type: application/json - {"file": "20260704_102813_IF.mp4", "types": ["thumbnail"]} - -> {"metadata": {"thumbnail": ""}} - ``` - - and the same with `"types": ["gps"]` -> `{"metadata": {"gps": ""}}`. - Both payloads sit under a nested `metadata` object and there is no - `resultcode` field (the app's known protocol). The base64 payload decodes to - the same bytes the legacy `.thm` / `.gps` files contained. -- **The accelerometer `.3gf` has no metadata type.** Only `thumbnail` and `gps` - are retrievable through `/vodMetadata`; there is no `.3gf` counterpart. On - V1.009 the accelerometer data is unavailable and is skipped. - -## Naming - -Consistent with the listing-API doc: the current style is *the* style and the -old style is "legacy", a binary yes/no. Do not use the name "restful". The -endpoint literals `/vodMetadata`, `/vodList`, `/accessible` are kept verbatim -because they are the actual firmware wire endpoints, not names chosen here. The -`.thm`/`.gps`/`.3gf` files are "metadata files", matching the existing -`--skip-metadata` vocabulary. - -## `blackvuesync.py` - -The download path currently ignores firmware version: `download_recording` -builds four `download_file` calls (video + `.thm` + `.3gf` + `.gps`), and -`download_file` hardcodes `urljoin(base_url, f"Record/{filename}")`. The change -threads a single `legacy: bool` into the download path and branches on it. - -New and changed units: - -- **Surface the legacy flag from `get_dashcam_filenames`.** The `/accessible` - probe already lives inside `get_dashcam_filenames`, where its result is - currently discarded. Instead of hoisting the probe into `sync` (which would - fire the real probe during unit tests that monkeypatch `get_dashcam_filenames` - by name), `get_dashcam_filenames` returns `(legacy, filenames)`. `sync` unpacks - the flag and passes it into the download loop, so the probe still happens - exactly once and no unit test wiring changes. -- **`download_recording(..., legacy: bool)`** -- routes each part of a recording: - - *Video:* `download_file(..., legacy)`. - - *Thumbnail / GPS:* on legacy, the existing plain-`GET` `download_file` path; - on V1.009, `download_metadata_file(...)` (below). Both remain gated by the - existing `--skip-metadata` flags (`t`, `g`). - - *Accelerometer (`.3gf`):* on legacy, unchanged; on V1.009, skipped with a - single `logger.debug` note that the accelerometer data is unavailable on this - firmware. (The `--skip-metadata` `3` flag is moot there.) -- **`download_file(..., legacy: bool)`** -- the only behavioral change is the URL: - `urljoin(base_url, f"Record/{filename}")` when legacy, `urljoin(base_url, - filename)` (root) when not. Everything else (temp dotfile, resume, failure - markers, metrics, tolerant `HTTPError` handling) is untouched. -- **`download_metadata_file(base_url, video_filename, metadata_filename, - metadata_type, destination, group_name, metrics) -> tuple[bool, int | None]`** - -- new. Mirrors `download_file`'s contract (already-downloaded skip, dry-run, - failure markers, temp-file-then-rename) but obtains bytes via `POST - /vodMetadata` instead of a `GET`: - - Builds the JSON request `{"file": video_filename, "types": [metadata_type]}` - with `Content-Type: application/json` (and the affinity header when set), - where `metadata_type` is `"thumbnail"` or `"gps"`. - - Parses the response, reads the base64 payload from the nested - `metadata` object (`response["metadata"][metadata_type]`), decodes it, and - writes it to `metadata_filename`. - - A missing `metadata` object, missing/empty payload, or malformed response is - logged, marked failed (throttling retries like a failed metadata `GET`'s - 404 today), and treated as a non-fatal skip, so a metadata hiccup never - aborts the video sync. -- Add `import base64`. - -The response shape follows the app's known protocol: both `thumbnail` and `gps` -sit under a nested `metadata` object, and the camera-direct endpoint carries no -`resultcode`. - -## Mock dashcam server (`features/mock_dashcam/server.py`) - -The server already has a per-session `legacy_api` flag. Extend the non-legacy -side to mirror V1.009 downloads: - -- `GET /.mp4` (root) -- serves the video when not legacy. -- `GET /Record/` -- serves the video when legacy (unchanged); on the - non-legacy side a root `GET` of a non-`.mp4` name returns `415 mp4 only`, for - fidelity and to guard against accidental plain metadata-file fetches. -- `POST /vodMetadata` -- when not legacy, accepts `{"file", "types":[type]}` and - returns `{"metadata": {"": ""}}` (nested for both `thumbnail` - and `gps`, no `resultcode`, matching the app's known protocol), sourced from - the same fixture bytes the legacy metadata files serve. Returns a body with no - `metadata` object for an unknown file so the tolerant path is exercised. -- Legacy behavior (`/Record/`, plain `.thm`/`.gps`/`.3gf`) is - unchanged. - -## Integration tests - -Extend the existing parity coverage (`features/sync_api_parity.feature`) so the -both-firmwares run also asserts the metadata outcome, not just the videos: - -- On both firmwares, the video and the thumbnail/GPS metadata files land. -- On V1.009 specifically, `.3gf` is **absent** (no retrieval path), whereas on - legacy it is present. - -A focused scenario drives a non-legacy sync and checks that `.thm` and `.gps` -exist with the expected bytes and that no `.3gf` is written. - -## Testing and verification - -- Primary coverage is integration, matching the current suite: - - The parity feature exercises both download paths end-to-end, including the - `/vodMetadata` decode. - - The default (current-style) suite continues to exercise the new download path - across all behaviors. -- Verify: `behave` green; `behave -D implementation=docker` green; existing - `pytest test/blackvuesync_test.py` unchanged. - -## Docs - -- `CHANGELOG.md`: note V1.009 downloads -- root video path plus thumbnail/GPS via - the metadata endpoint, accelerometer unavailable on that firmware (issue #87). -- Correct the superseded claim in `2026-07-02-firmware-v1009-listing-api.md` - (the "download path unchanged" out-of-scope note) with a pointer here. - -## Risks and out of scope - -- **`/vodMetadata` is not browser-verifiable by the reporter** (a JSON `POST` - is not address-bar-testable), so it ships against the app's known protocol: - `{"metadata": {"": ""}}`, nested, no `resultcode`. The tolerant, - non-fatal metadata path remains the safety net: if a firmware revision - differs, thumbnail/GPS quietly skip and the video sync -- the reporter's - actual complaint -- still succeeds. -- **Accelerometer `.3gf` is not recovered on V1.009.** No external retrieval path - exists on that firmware (the data appears only embedded in the mp4, whose - client-side extraction is out of scope for a stdlib-only tool). -- No auth/HTTPS changes; blackvuesync targets HTTP as today. From a659012ddf86dd19c9437edca022c6f2403aa58d Mon Sep 17 00:00:00 2001 From: Alessandro Colomba Date: Wed, 8 Jul 2026 07:07:15 -0400 Subject: [PATCH 4/7] fix: harden v1009 download path per code review Addresses code review follow-ups on the V1.009+ firmware support: - raises for unrecognized filenames in the dashcam recording list instead of dropping them silently; new firmware variants now fail loudly rather than never syncing - tolerates line-wrapped base64 in /vodMetadata payloads while still rejecting corrupt data - adds a unit test for the /vodMetadata success path, plain and wrapped - honors configured download errors in the mock /vodMetadata route and covers the metadata failure-marker lifecycle end to end in behave - generates clean mock recordings from comma-separated codes; the comma itself was previously treated as a recording type, which the silent client-side drop masked - lists only video recordings in the mock vodList, matching the documented live V1.009 index - rewords a mock comment that asserted unobserved firmware behavior and refreshes drifted docstrings --- blackvuesync.py | 30 +++++++++++---- features/lib/recordings.py | 14 +++---- features/mock_dashcam/server.py | 30 +++++++++++++-- features/steps/retry_failed_steps.py | 23 ++++++++--- features/sync_retry_failed.feature | 8 ++++ test/blackvuesync_test.py | 57 ++++++++++++++++++++++++++++ 6 files changed, 139 insertions(+), 23 deletions(-) diff --git a/blackvuesync.py b/blackvuesync.py index 37ccc78..b40622c 100755 --- a/blackvuesync.py +++ b/blackvuesync.py @@ -757,6 +757,21 @@ def to_recording(filename: str, grouping: str) -> Recording | None: ) +def to_dashcam_recordings( + dashcam_filenames: list[str], grouping: str +) -> list[Recording]: + """converts filenames listed by the dashcam to recordings; raises for unrecognized filenames""" + dashcam_recordings = [] + for dashcam_filename in dashcam_filenames: + if (recording := to_recording(dashcam_filename, grouping)) is None: + raise RuntimeError( + f"Unexpected filename in the recording list from the dashcam : {dashcam_filename}" + ) + dashcam_recordings.append(recording) + + return dashcam_recordings + + # pattern of a recording filename as returned in each line from from the dashcam index page file_line_re = re.compile(r"n:/Record/(?P.*\.mp4),s:1000000\r\n") @@ -1155,7 +1170,8 @@ def download_metadata_file( group_name: str | None, metrics: SyncMetrics | None = None, ) -> tuple[bool, int | None]: - """downloads thumbnail or gps data via the /vodMetadata endpoint (V1.009+)""" + """downloads thumbnail or gps data via the /vodMetadata endpoint (V1.009+); returns whether data was + transferred (download speed is not measured)""" # pylint: disable=too-many-arguments,too-many-positional-arguments,too-many-branches,too-many-return-statements,too-many-locals,too-many-statements if group_name: ensure_destination(os.path.join(destination, group_name)) @@ -1249,7 +1265,8 @@ def download_metadata_file( ) return False, None - content = base64.b64decode(encoded, validate=True) + # strips whitespace so line-wrapped payloads decode; validation still rejects corrupt base64 + content = base64.b64decode("".join(encoded.split()), validate=True) with open(temp_filepath, "wb") as f: f.write(content) os.rename(temp_filepath, destination_filepath) @@ -1319,8 +1336,9 @@ def download_recording( metrics: SyncMetrics | None = None, legacy: bool = False, ) -> None: - """downloads the set of recordings, including gps data, for the given filename from the dashcam to the destination - directory""" + """downloads the video and metadata files for the given recording from the dashcam to the destination directory; + on V1.009+ cameras, retrieves thumbnail and gps data via /vodMetadata and skips the unavailable accelerometer + data""" # pylint: disable=too-many-branches,too-many-locals # first checks that we have enough room left disk_usage = shutil.disk_usage(destination) @@ -1723,9 +1741,7 @@ def sync( # pylint: disable=too-many-arguments,too-many-positional-arguments base_url = f"http://{address}" legacy, dashcam_filenames = get_dashcam_filenames(base_url) - dashcam_recordings = [ - r for x in dashcam_filenames if (r := to_recording(x, grouping)) is not None - ] + dashcam_recordings = to_dashcam_recordings(dashcam_filenames, grouping) if metrics: metrics.dashcam_recordings_seen = len(dashcam_recordings) diff --git a/features/lib/recordings.py b/features/lib/recordings.py index 04f3258..03361a2 100644 --- a/features/lib/recordings.py +++ b/features/lib/recordings.py @@ -69,16 +69,14 @@ def generate_recording_filenames( # calculates number of days to generate (inclusive range) day_range = (end_date - start_date).days + 1 - # parses recording types - recording_types = list(recording_types_str) if recording_types_str else [] + # parses recording types, tolerating comma-separated codes + recording_types = list(recording_types_str.replace(",", "")) - # parses recording directions - recording_directions = ( - list(recording_directions_str) if recording_directions_str else [] - ) + # parses recording directions, tolerating comma-separated codes + recording_directions = list(recording_directions_str.replace(",", "")) - # parses other flags - recording_others = list(recording_others_str) if recording_others_str else [] + # parses other flags, tolerating comma-separated codes + recording_others = list(recording_others_str.replace(",", "")) # generates 5-10 recordings per day for each type random.seed(42) # deterministic generation diff --git a/features/mock_dashcam/server.py b/features/mock_dashcam/server.py index 0ca63b8..5253061 100644 --- a/features/mock_dashcam/server.py +++ b/features/mock_dashcam/server.py @@ -184,7 +184,12 @@ def vod_list() -> flask.Response | tuple[dict[str, Any], int]: if self._get_legacy_api(affinity_key): return flask.abort(404) recordings = self._get_recordings(affinity_key) - filelist = [{"filename": filename} for filename in recordings] + # the live V1.009 index lists only video recordings + filelist = [ + {"filename": filename} + for filename in recordings + if filename.endswith(".mp4") + ] response = {"filelist": filelist} logger.debug("Response body: %s", response) return response, 200 @@ -242,9 +247,28 @@ def vod_metadata() -> flask.Response | tuple[dict[str, Any], int]: else None ) if video_filename not in recordings or extension is None: - # a missing metadata object signals the error, mirroring the firmware + # returns no metadata object for unknown files so the client's tolerant path is exercised return {}, 200 + # honors configured download errors under the metadata filename the client + # stores, matching the legacy file-serving routes + video_recording = to_recording(video_filename) + if video_recording is not None: + if extension == "thm": + metadata_filename = ( + f"{video_recording.base_filename}_{video_recording.type}" + f"{video_recording.direction}.thm" + ) + else: + metadata_filename = ( + f"{video_recording.base_filename}_{video_recording.type}.gps" + ) + if metadata_filename in self._get_download_errors(affinity_key): + logger.debug( + "Response: 500 Internal Server Error (configured error)" + ) + flask.abort(500) + files_dir = Path(__file__).parent / "files" encoded = base64.b64encode( (files_dir / f"mock.{extension}").read_bytes() @@ -255,7 +279,7 @@ def vod_metadata() -> flask.Response | tuple[dict[str, Any], int]: @self.app.route("/", methods=["GET"]) def root_record(filename: str) -> flask.Response: - """serves videos from the root on V1.009; rejects non-mp4 with 'mp4 only'""" + """serves videos from the root on V1.009+; rejects non-mp4 with 'mp4 only'""" logger.debug("GET /%s", filename) affinity_key = self._get_affinity_key() if self._get_legacy_api(affinity_key): diff --git a/features/steps/retry_failed_steps.py b/features/steps/retry_failed_steps.py index 6ea198b..e5609c3 100644 --- a/features/steps/retry_failed_steps.py +++ b/features/steps/retry_failed_steps.py @@ -17,16 +17,17 @@ ) -@given("the first {count:d} mp4 recordings are configured to fail") -def download_errors(context: Context, count: int) -> None: - """configures the first N mp4 recordings to return download errors.""" +def _configure_download_errors(context: Context, extension: str, count: int) -> None: + """configures the first N files with the given extension to return download errors.""" if not hasattr(context, "expected_recordings"): raise RuntimeError( "Cannot configure download errors: no recordings configured yet." ) - mp4_files = [f for f in context.expected_recordings if f.endswith(".mp4")] - failed_filenames = mp4_files[:count] + matching_files = [ + f for f in context.expected_recordings if f.endswith(f".{extension}") + ] + failed_filenames = matching_files[:count] url = f"{context.mock_dashcam_url}/mock/downloads/errors" headers = {"X-Affinity-Key": context.scenario_token} @@ -38,6 +39,18 @@ def download_errors(context: Context, count: int) -> None: context.failed_recordings = set(failed_filenames) +@given("the first {count:d} mp4 recordings are configured to fail") +def download_errors(context: Context, count: int) -> None: + """configures the first N mp4 recordings to return download errors.""" + _configure_download_errors(context, "mp4", count) + + +@given('the first {count:d} "{extension}" metadata files are configured to fail') +def download_errors_for_metadata(context: Context, count: int, extension: str) -> None: + """configures the first N metadata files with the given extension to return download errors.""" + _configure_download_errors(context, extension, count) + + @when("download errors are cleared") def clear_download_errors(context: Context) -> None: """clears all configured download errors.""" diff --git a/features/sync_retry_failed.feature b/features/sync_retry_failed.feature index a52a13e..d3d72a0 100644 --- a/features/sync_retry_failed.feature +++ b/features/sync_retry_failed.feature @@ -8,6 +8,14 @@ Feature: Retry failed downloads Then the successful recordings are downloaded Then failure markers exist for the failed recordings + Scenario: Failed metadata downloads leave markers without failing the run + Given recordings for the past "1d" of types "N", directions "F" + Given the first 1 "thm" metadata files are configured to fail + When blackvuesync runs + Then blackvuesync exits with code 0 + Then the successful recordings are downloaded + Then failure markers exist for the failed recordings + Scenario: Failed downloads are skipped on next sync Given recordings for the past "1d" of types "N", directions "F" Given the first 2 mp4 recordings are configured to fail diff --git a/test/blackvuesync_test.py b/test/blackvuesync_test.py index 33fe86a..caf65ed 100644 --- a/test/blackvuesync_test.py +++ b/test/blackvuesync_test.py @@ -1,6 +1,7 @@ from __future__ import annotations import argparse +import base64 import datetime import email.message import errno @@ -202,6 +203,26 @@ def test_to_recording( assert expected_recording == recording +def test_to_dashcam_recordings() -> None: + """verifies dashcam filenames convert to recordings.""" + recordings = blackvuesync.to_dashcam_recordings( + ["20181029_131513_NF.mp4", "20181029_131513_NR.mp4"], "none" + ) + + assert [r.filename for r in recordings] == [ + "20181029_131513_NF.mp4", + "20181029_131513_NR.mp4", + ] + + +def test_to_dashcam_recordings_raises_on_unrecognized_filename() -> None: + """verifies an unrecognized filename in the dashcam recording list raises instead of being dropped.""" + with pytest.raises(RuntimeError, match="recording list"): + blackvuesync.to_dashcam_recordings( + ["20181029_131513_NF.mp4", "bogus.txt"], "none" + ) + + @pytest.mark.parametrize( "filename, expected_recording", [ @@ -1029,6 +1050,42 @@ def fake_urlopen(_request: urllib.request.Request) -> _FakeUrlResponse: assert not glob.glob(os.path.join(destination, "*.failed")) +@pytest.mark.parametrize("wrapped", [False, True]) +def test_download_metadata_file_writes_decoded_payload( + monkeypatch: pytest.MonkeyPatch, wrapped: bool +) -> None: + """verifies a valid /vodMetadata response, plain or line-wrapped, is decoded and written to the destination.""" + content = b"thumbnail data" + encoded = base64.b64encode(content).decode("ascii") + if wrapped: + encoded = "\r\n".join(encoded[i : i + 4] for i in range(0, len(encoded), 4)) + body = json.dumps({"metadata": {"thumbnail": encoded}}).encode("utf-8") + + with tempfile.TemporaryDirectory() as destination: + monkeypatch.setattr( + urllib.request, "urlopen", lambda _request: _FakeUrlResponse(body) + ) + monkeypatch.setattr(blackvuesync, "dry_run", False) + metrics = _sync_metrics() + + downloaded, speed = blackvuesync.download_metadata_file( + "http://dashcam/", + "20181029_131513_NF.mp4", + "20181029_131513_NF.thm", + "thumbnail", + destination, + None, + metrics, + ) + + assert (downloaded, speed) == (True, None) + destination_filepath = os.path.join(destination, "20181029_131513_NF.thm") + with open(destination_filepath, "rb") as f: + assert f.read() == content + assert not os.path.exists(os.path.join(destination, ".20181029_131513_NF.thm")) + assert not glob.glob(os.path.join(destination, "*.failed")) + + @pytest.mark.parametrize( "body", [ From 799f576b24ac9990791a3ed0a64dca0269f8a139 Mon Sep 17 00:00:00 2001 From: Alessandro Colomba Date: Wed, 8 Jul 2026 07:07:37 -0400 Subject: [PATCH 5/7] docs: reference release version and issue number Points the README firmware callout at 2.2.0 instead of a prerelease build, and adds the missing issue reference to the CHANGELOG entry. --- CHANGELOG.md | 2 +- README.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 01f004e..fafb063 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,7 @@ ## 2.2.0 -* Support BlackVue firmware V1.009+ cameras. +* Support BlackVue firmware V1.009+ cameras. (#87) * Replace undocumented `--filter` with `--include` and `--exclude` options for filtering recordings by type and direction. Codes are comma-separated, direction is optional. (#61) * Add `--retry-failed-after` option to retry failed downloads after a configurable delay. (#58) * Add `--skip-metadata` option to skip downloading metadata files (thumbnails, accelerometer, GPS). (#14) diff --git a/README.md b/README.md index 55f6da7..db67c74 100644 --- a/README.md +++ b/README.md @@ -19,7 +19,7 @@ A typical setup would be a periodic cron job or a Docker container running on a ## Features > [!IMPORTANT] -> As of version 2.2.0a7, supports the new API in BlackVue firmware V1.009+. +> As of version 2.2.0, supports the new API in BlackVue firmware V1.009+. * **Portable runtimes:** * A [single, self-contained Python script](https://github.com/acolomba/blackvuesync/blob/master/blackvuesync.py) with no third-party dependencies. It can be copied and run anywhere, either [manually](#manual-usage) or [periodically](#unattended-usage). From 4179ceac5fe3b14484e8abefeeb221ca15d374ea Mon Sep 17 00:00:00 2001 From: Alessandro Colomba Date: Wed, 8 Jul 2026 07:12:29 -0400 Subject: [PATCH 6/7] fix: fail firmware probe on unexpected responses Narrows is_legacy_camera: a 404 from /accessible selects the legacy protocol, a 200 selects V1.009+, and anything else fails the run like a recording-list error instead of silently misdetecting the firmware. Previously a V1.009+ camera answering 5xx on the probe was rerouted onto the legacy protocol, producing a misattributed listing failure. --- blackvuesync.py | 19 ++++++++++----- test/blackvuesync_test.py | 50 +++++++++++++++++++++++++++++++++++---- 2 files changed, 59 insertions(+), 10 deletions(-) diff --git a/blackvuesync.py b/blackvuesync.py index b40622c..807a1b4 100755 --- a/blackvuesync.py +++ b/blackvuesync.py @@ -796,16 +796,23 @@ def _build_request(url: str) -> urllib.request.Request: def is_legacy_camera(base_url: str) -> bool: - """returns True when the /accessible probe does not answer 200; pre-V1.009 - cameras lack the endpoint""" + """returns True when the /accessible probe answers 404, since pre-V1.009 cameras lack the + endpoint; any other error fails the run like a recording-list failure""" url = urllib.parse.urljoin(base_url, "accessible") try: with urllib.request.urlopen(_build_request(url)) as response: - legacy = bool(response.getcode() != 200) - probe_result = f"status code : {response.getcode()}" + response_status_code = response.getcode() + if response_status_code != 200: + raise RuntimeError( + f"Error response from : {base_url} ; status code : {response_status_code}" + ) + legacy = False + probe_result = f"status code : {response_status_code}" except urllib.error.HTTPError as e: - # any http error means the camera answered but does not support the endpoint; - # connection-level failures propagate to the shared unavailable/disconnected handling + # a 404 means the camera answered but lacks the endpoint, i.e. legacy firmware; + # other http errors and connection-level failures propagate to the shared handling + if e.code != 404: + raise legacy = True probe_result = str(e) logger.debug( diff --git a/test/blackvuesync_test.py b/test/blackvuesync_test.py index caf65ed..345bdc1 100644 --- a/test/blackvuesync_test.py +++ b/test/blackvuesync_test.py @@ -927,18 +927,60 @@ def test_is_legacy_camera_current_on_ok_response( assert blackvuesync.is_legacy_camera("http://dashcam/") is False -@pytest.mark.parametrize("status_code", [404, 500, 503]) -def test_is_legacy_camera_legacy_on_http_error( +def test_is_legacy_camera_legacy_on_not_found( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """verifies a 404 from the /accessible probe selects the legacy api.""" + + def fake_urlopen(_request: urllib.request.Request) -> _FakeUrlResponse: + raise _http_error("http://dashcam/accessible", 404) + + monkeypatch.setattr(urllib.request, "urlopen", fake_urlopen) + + assert blackvuesync.is_legacy_camera("http://dashcam/") is True + + +@pytest.mark.parametrize("status_code", [401, 500, 503]) +def test_is_legacy_camera_raises_on_unexpected_http_error( monkeypatch: pytest.MonkeyPatch, status_code: int ) -> None: - """verifies any http error from the /accessible probe selects the legacy api.""" + """verifies http errors other than 404 from the /accessible probe propagate instead of selecting an api.""" def fake_urlopen(_request: urllib.request.Request) -> _FakeUrlResponse: raise _http_error("http://dashcam/accessible", status_code) monkeypatch.setattr(urllib.request, "urlopen", fake_urlopen) - assert blackvuesync.is_legacy_camera("http://dashcam/") is True + with pytest.raises(urllib.error.HTTPError): + blackvuesync.is_legacy_camera("http://dashcam/") + + +def test_is_legacy_camera_raises_on_unexpected_status_code( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """verifies a non-200 success response from the /accessible probe fails instead of selecting an api.""" + monkeypatch.setattr( + urllib.request, + "urlopen", + lambda _request: _FakeUrlResponse(status_code=204), + ) + + with pytest.raises(RuntimeError, match="status code : 204"): + blackvuesync.is_legacy_camera("http://dashcam/") + + +def test_get_dashcam_filenames_runtime_error_on_probe_http_error( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """verifies an unexpected probe response fails the run like any recording-list error.""" + + def fake_urlopen(_request: urllib.request.Request) -> _FakeUrlResponse: + raise _http_error("http://dashcam/accessible", 500) + + monkeypatch.setattr(urllib.request, "urlopen", fake_urlopen) + + with pytest.raises(RuntimeError, match="Cannot obtain list of recordings"): + blackvuesync.get_dashcam_filenames("http://dashcam/") def test_is_legacy_camera_propagates_dropped_connection( From 61b660425caa4dae4fbacf1d460feb6aa77f90f3 Mon Sep 17 00:00:00 2001 From: Alessandro Colomba Date: Wed, 8 Jul 2026 07:27:05 -0400 Subject: [PATCH 7/7] fix: mark metadata download failed on any unusable response Widens the /vodMetadata parse handler from JSONDecodeError and binascii.Error to their common ancestor ValueError, which also covers UnicodeDecodeError from non-UTF-8 response bodies. Previously one such response aborted the whole run without writing a failure marker, so every following cron run stalled at the same file and recordings sorted after it never synced. Also adds a unit test covering the grouped metadata download path, whose cross-directory rename was previously untested. --- blackvuesync.py | 7 +++---- test/blackvuesync_test.py | 36 ++++++++++++++++++++++++++++++++++++ 2 files changed, 39 insertions(+), 4 deletions(-) diff --git a/blackvuesync.py b/blackvuesync.py index 807a1b4..65035a7 100755 --- a/blackvuesync.py +++ b/blackvuesync.py @@ -25,7 +25,6 @@ import argparse import base64 -import binascii import contextlib import datetime import errno @@ -1321,9 +1320,9 @@ def download_metadata_file( raise UserWarning( f"Timeout communicating with dashcam at address : {base_url}; error : {e}" ) from e - except (json.JSONDecodeError, binascii.Error) as e: - # malformed JSON or base64; the camera answered, so marks failed like - # the other unusable responses + except ValueError as e: + # malformed json, encoding or base64; the camera answered, so marks + # failed like the other unusable responses _handle_download_failure( metadata_filename, e, diff --git a/test/blackvuesync_test.py b/test/blackvuesync_test.py index 345bdc1..9bd4bd1 100644 --- a/test/blackvuesync_test.py +++ b/test/blackvuesync_test.py @@ -1128,6 +1128,41 @@ def test_download_metadata_file_writes_decoded_payload( assert not glob.glob(os.path.join(destination, "*.failed")) +def test_download_metadata_file_writes_decoded_payload_into_group_directory( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """verifies a valid /vodMetadata response lands in the group directory with no temp dotfile left behind.""" + content = b"thumbnail data" + encoded = base64.b64encode(content).decode("ascii") + body = json.dumps({"metadata": {"thumbnail": encoded}}).encode("utf-8") + + with tempfile.TemporaryDirectory() as destination: + monkeypatch.setattr( + urllib.request, "urlopen", lambda _request: _FakeUrlResponse(body) + ) + monkeypatch.setattr(blackvuesync, "dry_run", False) + metrics = _sync_metrics() + + downloaded, speed = blackvuesync.download_metadata_file( + "http://dashcam/", + "20181029_131513_NF.mp4", + "20181029_131513_NF.thm", + "thumbnail", + destination, + "2018-10-29", + metrics, + ) + + assert (downloaded, speed) == (True, None) + destination_filepath = os.path.join( + destination, "2018-10-29", "20181029_131513_NF.thm" + ) + with open(destination_filepath, "rb") as f: + assert f.read() == content + assert not glob.glob(os.path.join(destination, ".*")) + assert not glob.glob(os.path.join(destination, "2018-10-29", ".*")) + + @pytest.mark.parametrize( "body", [ @@ -1136,6 +1171,7 @@ def test_download_metadata_file_writes_decoded_payload( b'{"metadata": {}}', b'{"metadata": {"thumbnail": "aGVs!bG8="}}', b"not json", + b"\x80\x81\x82", ], ) def test_download_metadata_file_marks_failed_on_unusable_response(