mirror of
https://github.com/maziggy/bambuddy.git
synced 2026-09-30 03:01:21 +02:00
Review follow-ups on applying camera_rotation to finish photos and layer-timelapse frames. Rotating the frame popped from _stage22_finish_frames rotated one of its sources twice. The cache has two kinds of feeder: live grabs, which are raw, and the #1867 in-print bank, whose bytes come from _capture_snapshot_for_notification and have already been rotated on the way in. The consumer cannot tell them apart, so on the finish_state trigger - the path the bank exists to serve, on firmware that never emits stg_cur=22 - a 180 degree rotation cancelled itself out and the photo was upside-down again, which is the reported symptom exactly; 90 and 270 landed 180 out. Rotation now happens where each frame is captured, so every entry in the cache carries one rotation whatever produced it, and the invariant is stated both where the cache is declared and where it is consumed. Two finish-photo sources were still writing unrotated files: the built-in camera's own capture_finish_photo, and the still extracted from a printer-recorded timelapse - which is the *preferred* source for a built-in camera print, so a user with a rotation set got a correctly oriented photo or not depending on which source happened to win. Neither ever holds the frame as bytes; ffmpeg writes the file and they return a filename. apply_camera_rotation_to_file handles that case and is best-effort - a failed rotate leaves the unrotated file rather than losing a delivered photo. The archived video itself is the printer's own file and is not re-encoded, so it still plays at the camera's native orientation; the CHANGELOG says so rather than leaving it to be discovered. apply_camera_rotation logs at debug, not info. It was on a path that runs once per layer, where a tall print would have put hundreds of lines in the log for something the surrounding capture already reports at debug. The moved rotation logic had no test of its own - every existing test patches it out and asserts the call, so a flipped sign or a dropped expand=True would have shipped green. test_camera_rotation.py drives the real round trip: a corner marker pins which way it turns, the dimensions pin that the frame is not cropped, and an undecodable frame comes back by identity because a capture path must not lose a frame to a failed rotate. Tests for the fix itself sit on both sides of the cache. The producer half is driven directly; the consumer half is a closure nested inside on_print_complete with nothing able to reach it, so it is pinned by an AST guard - checked against the source because the alternative is no check at all. Reverting main.py to the pre-fix shape fails three of the five, the guard among them. The three new tests used Path("/tmp/test") for a patched base_dir, which Bandit flagged (B108); they take tmp_path now.
248 lines
9.7 KiB
Python
248 lines
9.7 KiB
Python
"""Tests for _capture_finish_photo_from_timelapse (#1397).
|
|
|
|
The polling helper runs in parallel with _scan_for_timelapse_with_retries —
|
|
it waits for archive.timelapse_path to land in the DB, then extracts the
|
|
last frame as the finish photo. These tests exercise the four shapes the
|
|
helper has to handle correctly:
|
|
|
|
1. timelapse never lands within timeout → return None (caller falls back)
|
|
2. timelapse lands, extraction succeeds → return filename
|
|
3. timelapse lands, extraction fails → return None (caller falls back)
|
|
4. timelapse_path is set but the file doesn't exist on disk → keep polling
|
|
|
|
DB access is patched at the session-maker boundary so these tests run in
|
|
~50ms each without standing up a real engine.
|
|
"""
|
|
|
|
from contextlib import asynccontextmanager
|
|
from pathlib import Path
|
|
from types import SimpleNamespace
|
|
from unittest.mock import AsyncMock, patch
|
|
|
|
import pytest
|
|
|
|
from backend.app import main as main_module
|
|
from backend.app.main import _capture_finish_photo_from_timelapse
|
|
|
|
|
|
@asynccontextmanager
|
|
async def _fake_session(archive):
|
|
"""A fake session whose execute().scalar_one_or_none() returns `archive`.
|
|
|
|
`archive` is mutated by the test mid-poll to simulate the real flow:
|
|
the timelapse-attach background task setting `timelapse_path` after a
|
|
few poll cycles.
|
|
"""
|
|
result = SimpleNamespace(scalar_one_or_none=lambda: archive)
|
|
session = SimpleNamespace(execute=AsyncMock(return_value=result))
|
|
yield session
|
|
|
|
|
|
@pytest.fixture
|
|
def fake_archive():
|
|
"""Mutable archive stand-in. Tests flip `.timelapse_path` to simulate
|
|
the timelapse-attach task writing to the DB."""
|
|
return SimpleNamespace(id=42, timelapse_path=None)
|
|
|
|
|
|
@pytest.fixture(autouse=True)
|
|
def _fast_poll(monkeypatch):
|
|
"""Shrink poll interval + timeout so tests don't sleep for real."""
|
|
monkeypatch.setattr(main_module, "_FINISH_PHOTO_TIMELAPSE_POLL_INTERVAL_SECONDS", 0.01)
|
|
monkeypatch.setattr(main_module, "_FINISH_PHOTO_TIMELAPSE_POLL_TIMEOUT_SECONDS", 0.2)
|
|
|
|
|
|
@pytest.fixture
|
|
def patched_session(fake_archive, monkeypatch):
|
|
"""Patch main.async_session so the helper reads our fake archive."""
|
|
monkeypatch.setattr(main_module, "async_session", lambda: _fake_session(fake_archive))
|
|
return fake_archive
|
|
|
|
|
|
async def test_returns_none_when_timelapse_never_lands(tmp_path: Path, patched_session):
|
|
"""Print finished without a timelapse — bail after timeout so the caller
|
|
falls back to the live-camera grab."""
|
|
result, pending = await _capture_finish_photo_from_timelapse(
|
|
archive_id=42,
|
|
archive_dir=tmp_path,
|
|
)
|
|
assert result is None
|
|
# Ran out of time rather than concluded: the video may still be on its way,
|
|
# which is what tells the caller to schedule a background upgrade (#2704).
|
|
assert pending is True
|
|
|
|
|
|
async def test_extracts_frame_when_timelapse_lands(tmp_path: Path, patched_session, monkeypatch):
|
|
"""Simulate the timelapse landing after one poll cycle and extraction
|
|
succeeding — should return a filename matching the finish_*.jpg pattern."""
|
|
# Lay down a stub timelapse file relative to base_dir so the path
|
|
# join works the way the helper expects.
|
|
monkeypatch.setattr(main_module.app_settings, "base_dir", tmp_path)
|
|
video_relpath = Path("archive/1/print/timelapse.mp4")
|
|
video_abspath = tmp_path / video_relpath
|
|
video_abspath.parent.mkdir(parents=True, exist_ok=True)
|
|
video_abspath.write_bytes(b"x" * 100) # non-empty so the size check passes
|
|
|
|
# Patch extraction to succeed unconditionally — the actual ffmpeg
|
|
# codepath has its own test file.
|
|
async def fake_extract(src, dst):
|
|
dst.write_bytes(b"\xff\xd8" + b"\x00" * 50) # JPEG SOI
|
|
return True
|
|
|
|
monkeypatch.setattr(main_module, "_FINISH_PHOTO_TIMELAPSE_POLL_INTERVAL_SECONDS", 0.0)
|
|
|
|
# Flip the archive into the "timelapse landed" state before the first
|
|
# poll — the helper picks it up on its initial read.
|
|
patched_session.timelapse_path = str(video_relpath)
|
|
|
|
with patch(
|
|
"backend.app.services.camera.extract_video_last_frame",
|
|
new=fake_extract,
|
|
):
|
|
result, pending = await _capture_finish_photo_from_timelapse(
|
|
archive_id=42,
|
|
archive_dir=tmp_path / "archive_dir",
|
|
)
|
|
|
|
assert result is not None
|
|
assert result.startswith("finish_")
|
|
assert result.endswith(".jpg")
|
|
assert (tmp_path / "archive_dir" / "photos" / result).exists()
|
|
assert pending is False
|
|
|
|
|
|
async def test_returns_none_when_extraction_fails(tmp_path: Path, patched_session, monkeypatch):
|
|
"""Timelapse landed but ffmpeg said no — we don't keep retrying on the
|
|
same broken file; return None so the caller falls back."""
|
|
monkeypatch.setattr(main_module.app_settings, "base_dir", tmp_path)
|
|
video_relpath = Path("archive/1/print/timelapse.mp4")
|
|
video_abspath = tmp_path / video_relpath
|
|
video_abspath.parent.mkdir(parents=True, exist_ok=True)
|
|
video_abspath.write_bytes(b"x" * 100)
|
|
|
|
async def fake_extract_fails(src, dst):
|
|
return False
|
|
|
|
patched_session.timelapse_path = str(video_relpath)
|
|
|
|
with patch(
|
|
"backend.app.services.camera.extract_video_last_frame",
|
|
new=fake_extract_fails,
|
|
):
|
|
result, pending = await _capture_finish_photo_from_timelapse(
|
|
archive_id=42,
|
|
archive_dir=tmp_path / "archive_dir",
|
|
)
|
|
|
|
assert result is None
|
|
# The video arrived and ffmpeg refused it — waiting longer cannot help, so
|
|
# this must NOT ask for a background retry.
|
|
assert pending is False
|
|
|
|
|
|
async def test_polls_until_file_appears(tmp_path: Path, patched_session, monkeypatch):
|
|
"""timelapse_path is set, but the file isn't on disk yet (the attach
|
|
background task hasn't finished writing). Should keep polling — and
|
|
succeed once the file materialises."""
|
|
monkeypatch.setattr(main_module.app_settings, "base_dir", tmp_path)
|
|
monkeypatch.setattr(main_module, "_FINISH_PHOTO_TIMELAPSE_POLL_INTERVAL_SECONDS", 0.05)
|
|
monkeypatch.setattr(main_module, "_FINISH_PHOTO_TIMELAPSE_POLL_TIMEOUT_SECONDS", 1.0)
|
|
|
|
video_relpath = Path("archive/1/print/timelapse.mp4")
|
|
patched_session.timelapse_path = str(video_relpath)
|
|
|
|
# File not present yet. Schedule it to land after ~150ms.
|
|
import asyncio
|
|
|
|
async def materialise_later():
|
|
await asyncio.sleep(0.15)
|
|
video_abspath = tmp_path / video_relpath
|
|
video_abspath.parent.mkdir(parents=True, exist_ok=True)
|
|
video_abspath.write_bytes(b"x" * 100)
|
|
|
|
async def fake_extract(src, dst):
|
|
dst.write_bytes(b"\xff\xd8")
|
|
return True
|
|
|
|
materialise = asyncio.create_task(materialise_later())
|
|
try:
|
|
with patch(
|
|
"backend.app.services.camera.extract_video_last_frame",
|
|
new=fake_extract,
|
|
):
|
|
result, pending = await _capture_finish_photo_from_timelapse(
|
|
archive_id=42,
|
|
archive_dir=tmp_path / "archive_dir",
|
|
)
|
|
finally:
|
|
materialise.cancel()
|
|
|
|
assert result is not None
|
|
assert result.startswith("finish_")
|
|
|
|
|
|
async def test_extracted_frame_is_rotated_when_configured(tmp_path: Path, patched_session, monkeypatch):
|
|
"""#2708: this source hands a path to ffmpeg and never holds the bytes, so
|
|
it was the one finish-photo source that ignored camera_rotation entirely.
|
|
A built-in-camera print with a timelapse prefers this source over the live
|
|
grab, so leaving it out meant the orientation depended on which source won.
|
|
"""
|
|
import io
|
|
|
|
from PIL import Image
|
|
|
|
monkeypatch.setattr(main_module.app_settings, "base_dir", tmp_path)
|
|
video_relpath = Path("archive/1/print/timelapse.mp4")
|
|
video_abspath = tmp_path / video_relpath
|
|
video_abspath.parent.mkdir(parents=True, exist_ok=True)
|
|
video_abspath.write_bytes(b"x" * 100)
|
|
|
|
async def fake_extract(src, dst):
|
|
buf = io.BytesIO()
|
|
Image.new("RGB", (64, 32), (0, 0, 255)).save(buf, format="JPEG")
|
|
dst.write_bytes(buf.getvalue())
|
|
return True
|
|
|
|
monkeypatch.setattr(main_module, "_FINISH_PHOTO_TIMELAPSE_POLL_INTERVAL_SECONDS", 0.0)
|
|
patched_session.timelapse_path = str(video_relpath)
|
|
|
|
with patch("backend.app.services.camera.extract_video_last_frame", new=fake_extract):
|
|
result, _ = await _capture_finish_photo_from_timelapse(
|
|
archive_id=42,
|
|
archive_dir=tmp_path / "archive_dir",
|
|
rotation=90,
|
|
)
|
|
|
|
assert result is not None
|
|
written = tmp_path / "archive_dir" / "photos" / result
|
|
# 64x32 turned a quarter turn: the file on disk is the rotated one, not
|
|
# what ffmpeg wrote.
|
|
assert Image.open(io.BytesIO(written.read_bytes())).size == (32, 64)
|
|
|
|
|
|
async def test_extracted_frame_is_untouched_without_a_rotation(tmp_path: Path, patched_session, monkeypatch):
|
|
"""The default path must not decode and re-encode ffmpeg's output for
|
|
nothing — that would cost a generation of JPEG quality on every print."""
|
|
monkeypatch.setattr(main_module.app_settings, "base_dir", tmp_path)
|
|
video_relpath = Path("archive/1/print/timelapse.mp4")
|
|
video_abspath = tmp_path / video_relpath
|
|
video_abspath.parent.mkdir(parents=True, exist_ok=True)
|
|
video_abspath.write_bytes(b"x" * 100)
|
|
|
|
extracted = b"\xff\xd8" + b"\x00" * 50
|
|
|
|
async def fake_extract(src, dst):
|
|
dst.write_bytes(extracted)
|
|
return True
|
|
|
|
monkeypatch.setattr(main_module, "_FINISH_PHOTO_TIMELAPSE_POLL_INTERVAL_SECONDS", 0.0)
|
|
patched_session.timelapse_path = str(video_relpath)
|
|
|
|
with patch("backend.app.services.camera.extract_video_last_frame", new=fake_extract):
|
|
result, _ = await _capture_finish_photo_from_timelapse(
|
|
archive_id=42,
|
|
archive_dir=tmp_path / "archive_dir",
|
|
)
|
|
|
|
assert (tmp_path / "archive_dir" / "photos" / result).read_bytes() == extracted
|