Fresh-install shakedown: installer, capture, video and retention fixes, plus a full doc audit - #17
Merged
Merged
Conversation
cleanup_staging() ended on [ -n "$STAGING" ], and bash takes the EXIT trap's status as the script's -- so --check, --dry-run and --uninstall all exited 1 on success, and anything gating on $? wrongly aborted. The if-form returns 0 on the empty case. UpgradeOldRaspilapse.md documented the bug as a trap to read around; it now records the fix (and keeps the still-true note that --check only warns about optional modules). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011uhRMmJgKzCJad6PNUihq3
RASPILAPSE_IMAGE_DIR and RASPILAPSE_KEEP_DAYS env overrides, documented as a systemd drop-in in the unit template so a changed window survives reinstall and uninstall. KEEP_DAYS is validated: non-numeric refuses to delete anything, and 0 short-circuits -- under find -mtime +0 it meant "older than 24 hours", the opposite of "keep everything". A missing image directory is a clean no-op instead of a set -e abort. Also drops the unit's [Install] section: install.sh only ever enables the timer, and the section contradicted the installer's own design note that the batch services cannot be enabled directly. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011uhRMmJgKzCJad6PNUihq3
The date subdirectory, image filename, sidecar name and capture_timestamp came from four separate datetime.now() calls: a frame straddling midnight landed in yesterday's directory under today's filename, where find_images_in_range() never looks, and a sidecar could carry a different second than its image. One shared timestamp now covers all four -- and it is timezone-aware, because during the DST fall-back hour a naive string repeats and the database's INSERT OR REPLACE destroyed the first pass's rows. A repeated wall-time filename now keeps both files under a _dst suffix instead of overwriting. Metering: a measured 0.0 was falsy under 'or 128' and read as "no measurement", clearing the underexposure flag at the exact moment it mattered (4.7x slower recovery from a blackout); the working target seeded from the dead transition_mode.target_brightness key instead of brightness_target.base; and highlight protection now defaults on, as CONFIG-REFERENCE always claimed. The lores brightness reader slices the size libcamera actually granted instead of assuming 320x240, a half-opened camera is closed before the retry, and close() nulls the handle even when picamera2's close raises -- a half-dead handle pinned the device and every later open failed with "Camera in use". Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011uhRMmJgKzCJad6PNUihq3
- Every failed Picamera2() open leaks a pipe fd inside picamera2; the loop retried forever while systemd showed healthy, hitting RLIMIT_NOFILE after ~8 hours. Five consecutive camera or decision failures now exit so Restart=always hands back a clean process. (A decision failure used to end the loop on the first transient -- with exit code 0.) - take_test_shot restored controls and save_metadata only on success; one busy camera -- ordinary right after a teardown -- left save_metadata False for the process lifetime, nulling every later row's exposure columns and starving restart seeding. The restore is in a finally. - Restart seeding: rows older than 20 intervals are not seeded at all (a night row seeded into a bright morning cost ~24 pure-white frames), and at the shutter ceiling the gain column is only trusted above 1.2 -- below that the sensor's 1.1228 floor is indistinguishable from a commanded 1.0. - The grid re-anchors when the clock steps backwards (no RTC: fake-hwclock plus late NTP), instead of sleeping out the whole step; the slot sleep runs in 1 s slices so SIGTERM stops the loop now (PEP 475). - --test shoots immediately, skips the trailing slot sleep, and exits 1 when no frame was captured -- it is an install check, and it always said yes. - status.jpg swaps by atomic rename; unlink-then-symlink left a window with no file, 2880 times a day, under a webserver that polls it. - With no lores metrics the controller holds its last measurement; the old fallback re-read the overlaid JPEG from disk and biased the loop bright. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011uhRMmJgKzCJad6PNUihq3
A Type=oneshot start that never returns never fails: under TimeoutStartSec=infinity one wedged ffmpeg left the unit 'activating' forever, the next morning's timer merged into the stuck job, and daily videos ended silently with nothing marked red. The unit now caps at 4h, the renderer caps its own ffmpeg at 3h, daily.py caps the renderer subprocess, and lock contention prints how long the holder has been at it. "No images -- nothing to render" moves from exit 2 to exit 10: argparse also exits 2 on usage errors, so a broken invocation reported nightly success. The upload finder's loose globs are gone -- keogram*<date>* matched the END date in yesterday's _0500_to_ filename, so a night whose keogram failed uploaded the previous day's file as today's. The image range is half-open (a frame exactly at 05:00:00 belongs to one window, not two), concat list entries escape single quotes, and the dead _daily_ naming branch -- which produced names neither the uploader nor retention recognised -- is removed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011uhRMmJgKzCJad6PNUihq3
mtime is the wrong clock on an RTC-less Pi -- fake-hwclock plus a late NTP sync stamps fresh renders with month-old times, and re-rendering an old day reset its window. Files now expire by the date in their name, mtime only as a fallback for undated names. The patterns anchor to output.project_name: a bare *.mp4 swept up anything parked in the web tree, including a second camera's videos. Empty-directory removal is limited to directories this prune emptied. And with uploads configured, a *missing* queue database now refuses to delete -- an absent file is indistinguishable from a mispointed database.path, and answering "nothing is protected" deletes the only copy of every day that never uploaded. The database path resolves through get_db_path, so pruning no longer depends on the caller's cwd. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011uhRMmJgKzCJad6PNUihq3
The connection context managers yielded from inside their except: an sqlite3.Error raised in the caller's body was thrown in at the yield, caught, and answered with a second yield -- which contextlib reports as "generator didn't stop after throw()", burying the real "database is locked". The yield now sits outside the connect guard, in both database.py and upload.py. db --prune reported its pre-count as deletions even when the DELETE failed; it now reports rowcount, and only dry runs use the count. The toolbelt upload test skips when requests-toolbelt is not installed, as on a lean camera. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011uhRMmJgKzCJad6PNUihq3
overlay.enabled: true with no other keys drew nothing: the default position was a corner layout, corner layouts need overlay.content lines, and the miss logged at DEBUG. top-bar -- which has built-in content -- is now the default position, the empty-corner case logs a WARNING that says what to set, and the default font is DejaVuSans-Bold: PIL's built-in bitmap font ignores the computed size and rendered 10px text on a 4K frame. Image.save opens its destination w+b, so overlaying in place truncated the good capture first -- an ENOSPC mid-encode left a partial JPEG that was then symlinked, recorded and uploaded. The overlay now writes beside the target and renames into place, and its re-encode quality fallback matches output.quality (85, was 95 on the one path that bypasses the merge). formats: setlocale restored from the environment rather than the previous value (process-global leak) and a missing locale warned once per frame -- 2880 journal lines a day; it restores in a finally and warns once. The exposure formatter's 1-second boundary produced an 8-char "1000.0ms" against " 20.0s" -- the exact sideways jump the fixed-width module exists to prevent -- and its 1/Ns branch was unreachable. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011uhRMmJgKzCJad6PNUihq3
Both graph test modules imported matplotlib at collection time and took the whole run down with them -- on the lean install the docs themselves recommend. importorskip instead; CI installs matplotlib and still runs them. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011uhRMmJgKzCJad6PNUihq3
CONFIG-REFERENCE promised "every setting, with its default" while twenty-two values disagreed with the code. The header now says what the file is -- a working camera's values -- and every behaviour-relevant divergence names the code default in its comment (quality, retention windows, organize_by_date, hdr, highlight speeds, the video_upload enabled-by- presence trap). video.codec.bitrate, the only rate control the hardware encoder uses, was undocumented; the /www/ overlay-data paths pointed at a directory that exists on no machine. Dead keys removed together with their DEFAULTS entries where present: test_shot.frequency (never read), video.filename_pattern (video names are hardcoded), transition_mode.target_brightness (inert since the dynamic target). The example config drops reference_lux for brightness_target.base and gains the settings everyone was missing: video.organize_by_date (code default false files every video flat), retention pointers, symlink_latest and overlay.camera_name shapes -- still under the drift tests' 120 lines. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011uhRMmJgKzCJad6PNUihq3
EXPOSURE.md rewritten around the ladder: its mode-selection section --
thresholds, civil-twilight override, hysteresis -- described a system that
no longer exists, and its tuning table sent readers to dead keys.
TIMELAPSE_VIDEO.md brought to the shipped encoder (veryfast/crf 20/3
threads), the real CLI defaults (05:00, not 00:00/now), the real ffmpeg
argument order, real filenames, measured timings, and a retention section it
lacked. README/INSTALL: trixie supported, the raspi-config camera step is
gone since Bookworm, netwatch is listed with the other installer options,
cleanup also expires videos, 6-8 GB/day, newcam.md and UpgradeOldRaspilapse
join the doc tables. OVERLAY/WEATHER: the commands and placeholders that did
not exist (--overwrite, {night}, center) removed, fixed-width examples
padded as rendered, N/A semantics stated. CONTRIBUTING: the pre-commit hook
builds its own black from the pinned rev -- the PATH advice pointed at the
one thing that cannot be the cause. config/README quotes the error string
the code actually raises.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011uhRMmJgKzCJad6PNUihq3
newcam.md described the pre-1.5.0 layout: src/auto_timelapse.py, installer scripts that do not exist, a deleted config key presented as mandatory, the wrong overlay key, links to documents that were never written. It is now the fresh-Pi walkthrough validated on sigerfjordcam-2 today -- OS to packages to config to services to a same-day video test -- with the disk budget, the sparse-interval arithmetic, and the traps that actually bite. The sibling repos compress to a pointer at the upgrade runbook. CHANGELOG records this branch: the installer exit codes, the overlay that drew nothing, the midnight/DST filing, the wedged-ffmpeg bound, the exit-10 contract, retention by covered day, the fd-leak exit, seeding guards, and the documentation audit. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011uhRMmJgKzCJad6PNUihq3
request.save() encodes at picamera2's own default quality (90) unless the options dict says otherwise, so output.quality only ever applied to the overlay's re-encode -- and not at all with the overlay off. Set it where the camera is configured. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011uhRMmJgKzCJad6PNUihq3
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 17 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (18)
📝 WalkthroughWalkthroughThe PR updates adaptive exposure, capture timestamps, overlay output, video rendering, retention, storage errors, installation behavior, configuration defaults, documentation, and related tests. ChangesRaspilapse audit fixes
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
The status display carried its own fallbacks -- "bottom-left", "default", 0.02 -- and reported settings the renderer was not using. Mirror render.py's defaults instead. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011uhRMmJgKzCJad6PNUihq3
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
raspilapse/daemon.py (1)
1151-1157: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winReset the failure count only after a successful frame.
Line 1157 clears
consecutive_failuresafter camera initialization. Ifcapture_frame()then fails on every slot, the next successful initialization clears the count again. The daemon can log failed captures forever without reaching the five-failure restart threshold.Increment the counter in the frame-capture exception path. Reset it only after capture, observation, and recording complete.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@raspilapse/daemon.py` around lines 1151 - 1157, Update the timelapse flow around camera initialization and the frame-capture exception handling: remove the consecutive_failures reset from the successful ImageCapture initialization block, increment it when capture_frame() fails, and reset it only after capture, observation, and recording complete successfully.tests/test_daily_timelapse.py (1)
444-461: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winUpdate the no-images test documentation.
The docstring says exit code
2means no images. The test andraspilapse.video.timelapse.main()now use exit code10.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_daily_timelapse.py` around lines 444 - 461, Update the docstring in test_main_no_images_exits_zero to state that exit code 10 from make_timelapse indicates no images and is not a failure, matching the mocked return code and timelapse.main behavior.raspilapse/video/daily.py (1)
159-179: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winPreserve the lock-holder timestamp.
Every contending invocation opens
daily.lockwith"w"beforeflock(). That updates its mtime. Line 173 then reports the contender's new mtime, not the holder's start time.Open the file with
"a+". Write or refresh the timestamp only after this process acquires the lock.Proposed fix
- lock_file = open(lock_path, "w") # noqa: SIM115 -- must outlive this scope + lock_file = open(lock_path, "a+") # noqa: SIM115 -- must outlive this scope try: fcntl.flock(lock_file, fcntl.LOCK_EX | fcntl.LOCK_NB) + lock_file.seek(0) + lock_file.truncate() + lock_file.write(f"{datetime.now().timestamp()}\n") + lock_file.flush()🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@raspilapse/video/daily.py` around lines 159 - 179, Update the lock-file acquisition flow around fcntl.flock to open lock_path in append/read mode ("a+") so contenders do not modify its mtime. After this process successfully acquires the lock, refresh the timestamp, preserving the existing contention reporting based on lock_path.stat().st_mtime.
🧹 Nitpick comments (1)
docs/TIMELAPSE_VIDEO.md (1)
196-205: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueSpecify the fenced-code language.
Line 199 opens a fenced block without a language. Use
textfor this sample output to satisfy MD040.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/TIMELAPSE_VIDEO.md` around lines 196 - 205, Update the fenced code block containing the abridged example output in the timelapse documentation to specify the text language, preserving the sample content unchanged.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/OVERLAY.md`:
- Line 49: Remove the leading and trailing padding inside the inline code spans
in the table rows containing `{lux}` and the additionally affected rows 56–58.
Preserve the documented values while formatting each inline code span without
surrounding whitespace.
In `@docs/WEATHER.md`:
- Around line 70-72: Update WeatherData.format_fields() so missing readings use
N/A values padded to the same fixed widths as populated temperature and pressure
fields, preserving the bar’s layout; alternatively, revise the documented
fixed-width behavior to match the actual formatting.
In `@newcam.md`:
- Around line 151-153: Update the validation instructions around the expected
MP4, keogram, and slitscan outputs to make HTTP 200 checks conditional on web
serving being configured. When Apache/nginx is disabled, verify the files
directly instead; retain HTTP checks for all three outputs when a web server is
enabled.
- Around line 54-56: Update the optional dependency list in the installation
documentation to include python3-requests alongside python3-requests-toolbelt,
while preserving the existing matplotlib entry and descriptions.
In `@raspilapse/camera/capture.py`:
- Around line 483-493: Update the capture naming flow around output_path
collision handling and metadata generation so duplicate DST captures receive one
renderer-supported, collision-free identifier shared by the image and sidecar
files. Include an offset or DST-fold component in the timestamp-derived name,
use that same name when selecting/rendering images, and ensure metadata paths at
the logic around the sidecar generation do not reuse the unchanged local
timestamp or overwrite the first capture.
In `@raspilapse/daemon.py`:
- Around line 493-530: Initialize image_path immediately after constructing
metadata_dir in the test-shot flow, before metadata_dir.mkdir(), ImageCapture
initialization, or request capture can raise. Preserve the existing assignment
to the test shot path on success so the original camera or filesystem exception
propagates instead of being replaced by an UnboundLocalError.
- Around line 1083-1088: Update the initial wait in the daemon startup flow
around _next_slot to use the existing callback-enabled scheduler path instead of
direct time.sleep, passing a callback that observes self.running so SIGTERM
interrupts the wait. Preserve the test_mode behavior and the computed next-slot
delay.
In `@raspilapse/overlay/formats.py`:
- Around line 112-125: Add a module-level lock and wrap the entire LC_TIME
lifecycle in the locale-formatting function: reading the current locale, setting
locale_str, executing strftime, and restoring previous. Ensure all callers
serialize this process-global locale access while preserving the existing
formatting and restoration behavior.
In `@raspilapse/overlay/render.py`:
- Around line 920-935: The overlay save logic in apply_overlay must avoid the
shared “.tmp” path when concurrent calls target the same output_path. Create a
unique temporary file with tempfile.mkstemp() in the destination directory, save
the JPEG to that path, replace output_path with it, and retain cleanup of the
unique temporary path on failure.
In `@systemd/raspilapse-cleanup.service.in`:
- Around line 15-23: Document both cleanup overrides: in
systemd/raspilapse-cleanup.service.in lines 15-23, add an
Environment=RASPILAPSE_IMAGE_DIR=... example and state it must match
output.directory; in newcam.md lines 161-165, add the same image-directory
override to the retention instructions alongside RASPILAPSE_KEEP_DAYS.
---
Outside diff comments:
In `@raspilapse/daemon.py`:
- Around line 1151-1157: Update the timelapse flow around camera initialization
and the frame-capture exception handling: remove the consecutive_failures reset
from the successful ImageCapture initialization block, increment it when
capture_frame() fails, and reset it only after capture, observation, and
recording complete successfully.
In `@raspilapse/video/daily.py`:
- Around line 159-179: Update the lock-file acquisition flow around fcntl.flock
to open lock_path in append/read mode ("a+") so contenders do not modify its
mtime. After this process successfully acquires the lock, refresh the timestamp,
preserving the existing contention reporting based on lock_path.stat().st_mtime.
In `@tests/test_daily_timelapse.py`:
- Around line 444-461: Update the docstring in test_main_no_images_exits_zero to
state that exit code 10 from make_timelapse indicates no images and is not a
failure, matching the mocked return code and timelapse.main behavior.
---
Nitpick comments:
In `@docs/TIMELAPSE_VIDEO.md`:
- Around line 196-205: Update the fenced code block containing the abridged
example output in the timelapse documentation to specify the text language,
preserving the sample content unchanged.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1764345e-f6ce-47bf-8d90-b302986b3c09
📒 Files selected for processing (37)
CHANGELOG.mdCONTRIBUTING.mdREADME.mdUpgradeOldRaspilapse.mdconfig/README.mdconfig/config.example.ymldocs/CONFIG-REFERENCE.ymldocs/EXPOSURE.mddocs/INSTALL.mddocs/OVERLAY.mddocs/TIMELAPSE_VIDEO.mddocs/TROUBLESHOOTING.mddocs/WEATHER.mdnewcam.mdraspilapse/camera/capture.pyraspilapse/camera/metering.pyraspilapse/config.pyraspilapse/daemon.pyraspilapse/overlay/formats.pyraspilapse/overlay/render.pyraspilapse/storage/database.pyraspilapse/storage/upload.pyraspilapse/video/daily.pyraspilapse/video/retention.pyraspilapse/video/timelapse.pyscripts/cleanup_old_images.shscripts/install.shsystemd/raspilapse-cleanup.service.insystemd/raspilapse-daily-video.service.intests/test_auto_timelapse.pytests/test_daily_timelapse.pytests/test_db_graphs.pytests/test_graph_solar_patterns.pytests/test_make_timelapse.pytests/test_make_timelapse_daily.pytests/test_upload_service.pytests/test_video_retention.py
Applied:
- The pre-loop grid alignment goes through the scheduler, so SIGTERM
interrupts it like every other wait (it was the one bare time.sleep left).
- Overlay saves use mkstemp instead of a predictable .tmp: the daemon and a
hand-run apply_overlay on the same frame could truncate or rename each
other's half-written file.
- LC_TIME cycle serialized behind a module lock -- setlocale is
process-global and not thread-safe.
- Missing weather readings pad to the same width as populated ones
(temperature 7, wind 8, rain 7, pressure 8), so the bar keeps its width
when a module drops out -- which is what WEATHER.md already promised.
- The DST-fold sidecar now carries the image's _dst suffix; the second
pass's sidecar no longer overwrites the first's, and the
{stem}_metadata.json pairing holds for the suffixed image.
- RASPILAPSE_IMAGE_DIR documented beside RASPILAPSE_KEEP_DAYS in the
cleanup unit and newcam.md, with the match-output.directory warning.
- newcam.md lists python3-requests beside the toolbelt and makes the HTTP
checks conditional on the webserver step.
- OVERLAY.md loses the padded code spans (MD038).
Answered on the PR: the test-shot UnboundLocalError claim (the exception
propagates; the return is unreachable on those paths) and the request for a
renderer-parseable duplicate identifier (any suffix or offset breaks the
frozen filename_pattern contract; the fold hour is preserved on disk and
excluded from the video by design).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011uhRMmJgKzCJad6PNUihq3
Ten one-line property accessors in metering and the weather cache were undocumented, holding the PR's docstring coverage at 75% against the 80% gate. Each now says what it answers. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011uhRMmJgKzCJad6PNUihq3
The atomic-save hardening switched the overlay's temp file to mkstemp, which creates 0600 by design, and os.replace carried that mode onto every final frame -- so the first camera to deploy it answered 403 for every image the webserver was asked for, status.jpg included. fchmod the temp file to 0644 before writing; a regression test now pins the mode. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011uhRMmJgKzCJad6PNUihq3
…ound Seven new tests for the branch's defensive branches: the backward-clock re-anchor, the SIGTERM-interruptible sleep slices, the five-consecutive- failures exit (with no frame captured on the way), a failed camera open releasing the half-opened handle, a failed overlay save leaving neither a truncated frame nor a temp file, and the ffmpeg / renderer timeout paths. Writing the lock tests found two real defects in the lock itself: - A contender opened the lock file with "w", truncating the holder's file and re-stamping its mtime -- so the new holder-age report always said "~0 min". Contenders now open "a" and the holder stamps acquisition time explicitly; the age is finally the holder's. - The suite shared the repo's real data/daily.lock across tests, and flock is per open-file-description -- whichever test's lock file happened to outlive its call (gc timing decides) blocked every later main() in the process. Each test now locks in its own tmp_path, and the suite stops writing into the repo's data/ directory at all. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011uhRMmJgKzCJad6PNUihq3
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Setting up sigerfjordcam-2 from a blank SD card put the whole path under audit: three correctness passes (capture daemon, video pipeline, storage) and a claim-by-claim review of every document, with everything confirmed fixed here. The camera this branch was validated on is live: 30 s captures, 4K daily video at 05:00, all tests green on the Pi itself.
Correctness fixes
--check/--dry-run/--uninstallexit 1 on success.contentlines, logged at DEBUG; and the default font was PIL's 10px bitmap. Defaults are nowtop-bar+ DejaVuSans-Bold; saves are atomic (in-placeImage.savetruncated the good frame on a failed encode);status.jpgswaps by rename instead of unlink+symlink.capture_timestampcame from fourdatetime.now()calls — a frame straddling midnight filed where the renderer never looks. One shared, timezone-aware timestamp; the DST fall-back hour no longer destroys rows (INSERT OR REPLACEon a repeated naive timestamp) or overwrites JPEGs.TimeoutStartSec=infinityon aType=oneshotunit never fails, and the next timer merges into the stuck job. Caps at every layer now; "no images" moved from exit 2 (shared with argparse usage errors, so a broken invocation reported nightly success) to exit 10._0500_to_filename.output.project_name(a bare*.mp4swept a second camera's files); empty-dir removal scoped to what the prune emptied; with uploads configured, a missing queue DB refuses to delete instead of treating everything as unprotected.Picamera2()opens leak an fd inside picamera2 — the retry-forever loop hitRLIMIT_NOFILEin ~8 h while systemd showed healthy; five consecutive failures now exit for a clean respawn.take_test_shotrestores config in afinally(one busy camera used to disable metadata sidecars for the process lifetime). Stale seed rows (>20 intervals) cold-start instead; ceiling gain only trusted above the sensor's 1.1228 floor band. Backward clock steps re-anchor the grid; SIGTERM stops the slot sleep in ≤1 s;--testexits 1 on zero frames and no longer takes a minute.db --prunereported pre-counts as deletions even when the DELETE failed.output.qualitynow applies at capture (picamera2 encoded at its own 90 otherwise).Behavior changes
cleanup_old_images.shtakesRASPILAPSE_IMAGE_DIR/RASPILAPSE_KEEP_DAYS(validated; 0 = off — underfind -mtime +0it meant "older than a day").test_shot.frequency,video.filename_pattern,transition_mode.target_brightness.Docs
Every file audited against the code. EXPOSURE.md rewritten around the ladder (its threshold/hysteresis/civil-twilight system no longer exists); TIMELAPSE_VIDEO.md brought to the shipped encoder and real CLI defaults; CONFIG-REFERENCE's 22 drifted values corrected/annotated and
bitratedocumented; newcam.md rewritten as the fresh-Pi quickstart validated by this very install; trixie noted; the raspi-config camera step (gone since Bookworm) removed; suite runs without matplotlib (importorskip; CI unaffected).Verification
pytest: 1119 passed, 4 skipped on the Pi (plus new tests: seeding staleness/floor guards, retention anchoring/date-expiry/fail-closed, half-open boundary).black --checkandruffclean (pinned black 25.11.0).rpicam-verified Camera Module 3,--testframe in the right path shape, service on the :00/:30 grid, and a same-eveningcli.daily --no-uploadrender probed at 3840x2160@25 and served 200 by Apache.Deferred (deliberately not in this branch)
required_exposurecolumn) would delete the ceiling-inference and its tolerance entirely — bigger schema change.MemoryHigh/MemoryMaxon the daily-video unit are inert on stock Pi OS (memory cgroup off) — documented in the template, left as-is.NamedTemporaryFileleaks on SIGKILL (the OOM path);PrivateTmp=or writing underdata/would cover it.video.organize_by_datestays default-false in code for existing installs; the example config now sets it true — flipping the code default is a compat call for another day.🤖 Generated with Claude Code
https://claude.ai/code/session_011uhRMmJgKzCJad6PNUihq3
Summary by CodeRabbit
New Features
Bug Fixes
Documentation