mirror of
https://github.com/saphid/frame-control.git
synced 2026-10-06 08:00:32 +02:00
fix(tracking): make eye-image cleanup exhaustive and confirm capture ownership
Follow-up review findings: remove() now tries every capture directory and reports any it could not delete; each cleanup step runs even if an earlier one fails; the directory the capture tool reports (once its output is flushed) must match the one we reduced, and is always removed. heart-check keeps readings with an unrecognised flag and reports unreadable references without a traceback. Part of #27 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
1 parent
016d2b8c2d
commit
1a439f05c2
3 files changed
+86
-14
No files matched your search
+44
-10
@@ -70,6 +70,8 @@ class Capture:
|
||||
NAME = re.compile(r"^(left|right)_(\d+)\.png$")
|
||||
# Only SteamVR's own capture directories are read and removed.
|
||||
PREFIX = "/tmp/etcalib_"
|
||||
# Printed by the capture tool; its output is only flushed when it exits.
|
||||
WRITING = re.compile(r"Writing capture to: (/tmp/etcalib_[\w-]+)")
|
||||
# About five seconds of frames (~65 MB in RAM-backed /tmp). If reduction
|
||||
# falls further behind than this, the capture stops rather than letting
|
||||
# eye images pile up.
|
||||
@@ -127,18 +129,42 @@ class Capture:
|
||||
if process.returncode or not self.directory:
|
||||
reason = "cameras unavailable" if "Failed to" in text else f"exit {process.returncode}"
|
||||
raise RuntimeError(f"eye-camera capture failed ({reason})")
|
||||
named = self.written(log)
|
||||
if named and named != self.directory:
|
||||
raise RuntimeError("the capture wrote somewhere else; not using those images")
|
||||
return self.frames()
|
||||
finally:
|
||||
if process and process.poll() is None:
|
||||
process.terminate()
|
||||
# Each step runs even if an earlier one failed: eye images must
|
||||
# be removed whatever else went wrong.
|
||||
failed = False
|
||||
for step in (lambda: self.stop(process),
|
||||
lambda: self.pool and self.pool.shutdown(wait=True, cancel_futures=True)):
|
||||
try:
|
||||
process.wait(timeout=5)
|
||||
except subprocess.TimeoutExpired:
|
||||
process.kill()
|
||||
process.wait()
|
||||
if self.pool:
|
||||
self.pool.shutdown(wait=True, cancel_futures=True)
|
||||
step()
|
||||
except Exception:
|
||||
failed = True
|
||||
named = self.written(log)
|
||||
if named:
|
||||
self.new.add(named) # ours by the tool's own account
|
||||
self.remove()
|
||||
if failed:
|
||||
raise RuntimeError("the eye-camera capture did not stop cleanly")
|
||||
|
||||
@staticmethod
|
||||
def stop(process):
|
||||
if process and process.poll() is None:
|
||||
process.terminate()
|
||||
try:
|
||||
process.wait(timeout=5)
|
||||
except subprocess.TimeoutExpired:
|
||||
process.kill()
|
||||
process.wait()
|
||||
|
||||
def written(self, log):
|
||||
"""The directory the capture tool reported, once its output is flushed."""
|
||||
log.seek(0)
|
||||
match = self.WRITING.search(log.read())
|
||||
return Path(match.group(1)) if match else None
|
||||
|
||||
def reduce(self, final=False):
|
||||
"""Reduce and delete every complete image; an image is complete once a
|
||||
@@ -184,9 +210,15 @@ class Capture:
|
||||
def remove(self):
|
||||
"""Remove every capture directory that appeared during the run. Eye
|
||||
images must not outlive it, so a failure to delete is an error."""
|
||||
left = []
|
||||
for directory in self.new | ({self.directory} if self.directory else set()):
|
||||
if str(directory).startswith(self.PREFIX) and directory.is_dir() and not directory.is_symlink():
|
||||
shutil.rmtree(directory)
|
||||
try:
|
||||
shutil.rmtree(directory)
|
||||
except OSError:
|
||||
left.append(str(directory))
|
||||
if left:
|
||||
raise RuntimeError("could not delete eye images in " + ", ".join(sorted(left)))
|
||||
|
||||
|
||||
# --------------------------------------------------------------- analysis ---
|
||||
@@ -291,7 +323,9 @@ def snr(spec, bpm, width=4.0):
|
||||
"""Power near the pulse and its first harmonic against the rest of the band."""
|
||||
near = sum(p for f, p in spec if abs(f - bpm) <= width or abs(f - 2 * bpm) <= width)
|
||||
rest = sum(p for f, p in spec) - near
|
||||
return near / rest if near > 0 and rest > 0 else 0.0
|
||||
if near <= 0:
|
||||
return 0.0
|
||||
return near / rest if rest > 0 else float("inf")
|
||||
|
||||
|
||||
def normalised(spec):
|
||||
|
||||
@@ -118,9 +118,12 @@ def read_csv(path):
|
||||
continue
|
||||
try:
|
||||
when, bpm = parse_time(row[0]), int(float(row[1]))
|
||||
flags = int(float(row[2])) if len(row) > 2 and row[2].strip() else None
|
||||
except ValueError:
|
||||
continue # header or unparseable line
|
||||
try:
|
||||
flags = int(float(row[2])) if len(row) > 2 else None
|
||||
except ValueError:
|
||||
flags = None # an unrecognised flag leaves the reading as it is
|
||||
if flags is not None and flags & 4 and not flags & 2:
|
||||
bpm = None # contact supported and not detected: no reading
|
||||
samples.append((when, bpm))
|
||||
@@ -261,8 +264,8 @@ def main(argv=None):
|
||||
try:
|
||||
reference = read_any(args.reference, ours[0][0] - 60, ours[-1][0] + 60)
|
||||
result = compare(ours, reference, args.max_lag)
|
||||
except ValueError as error:
|
||||
print(str(error))
|
||||
except (ValueError, OSError, ElementTree.ParseError, zipfile.BadZipFile) as error:
|
||||
print(f"Could not compare: {error}")
|
||||
return 1
|
||||
return 0 if report(result, args.tolerance) else 1
|
||||
|
||||
|
||||
+36
-1
@@ -134,6 +134,7 @@ class FakeCaptureTool:
|
||||
self.polls = 0
|
||||
self.returncode = None
|
||||
self.stdout = None
|
||||
self.announce = None # a different directory to report, if set
|
||||
|
||||
def __call__(self, command, cwd, stdout, stderr):
|
||||
self.command = command
|
||||
@@ -160,6 +161,9 @@ class FakeCaptureTool:
|
||||
"tsMono": 100 + i / 90, "valid": i != 2} for eye in ("left", "right")}
|
||||
for i in range(self.frames)]}
|
||||
(self.directory / "meta.json").write_text(json.dumps(meta))
|
||||
# The real tool's buffered output only appears once it exits.
|
||||
self.stdout.write(f"Writing capture to: {self.announce or self.directory}\nCaptured images\n")
|
||||
self.stdout.flush()
|
||||
self.returncode = 0
|
||||
return 0
|
||||
|
||||
@@ -190,6 +194,7 @@ class CaptureLifecycle(unittest.TestCase):
|
||||
class TestCapture(pulse.Capture):
|
||||
# A temporary directory stands in for /tmp/etcalib_*.
|
||||
PREFIX = str(self.root / "etcalib_")
|
||||
WRITING = __import__("re").compile(r"Writing capture to: (\S+)")
|
||||
capture = TestCapture(20, runner=tool, loader=self.loader, workers=0)
|
||||
with patch.object(pulse.time, "sleep", lambda s: None):
|
||||
return capture, capture.run()
|
||||
@@ -221,6 +226,7 @@ class CaptureLifecycle(unittest.TestCase):
|
||||
tool = Stalled(self.directory)
|
||||
class TestCapture(pulse.Capture):
|
||||
PREFIX = str(self.root / "etcalib_")
|
||||
WRITING = __import__("re").compile(r"Writing capture to: (\S+)")
|
||||
MAX_BACKLOG = 50
|
||||
capture = TestCapture(20, runner=tool, loader=self.loader, workers=0)
|
||||
capture.reduce = lambda final=False, original=capture.reduce: (
|
||||
@@ -247,6 +253,35 @@ class CaptureLifecycle(unittest.TestCase):
|
||||
self.assertFalse(foreign.exists()) # no eye image outlives the run
|
||||
self.assertTrue((self.root / "etcalib_older").exists())
|
||||
|
||||
def test_images_elsewhere_are_not_used_but_are_removed(self):
|
||||
tool = FakeCaptureTool(self.directory)
|
||||
tool.announce = self.root / "etcalib_reported"
|
||||
tool.announce.mkdir()
|
||||
(tool.announce / "left_0.png").write_bytes(b"png")
|
||||
with self.assertRaises(RuntimeError) as raised:
|
||||
self.capture(tool)
|
||||
self.assertIn("somewhere else", str(raised.exception))
|
||||
self.assertFalse(tool.announce.exists())
|
||||
self.assertFalse(self.directory.exists())
|
||||
|
||||
def test_cleanup_tries_every_directory(self):
|
||||
capture = pulse.Capture(20)
|
||||
capture.PREFIX = str(self.root / "etcalib_")
|
||||
first, second = self.root / "etcalib_a", self.root / "etcalib_b"
|
||||
for directory in (first, second):
|
||||
directory.mkdir()
|
||||
(directory / "left_0.png").write_bytes(b"png")
|
||||
capture.new = {first, second}
|
||||
real = pulse.shutil.rmtree
|
||||
def flaky(path):
|
||||
if Path(path) == first:
|
||||
raise OSError("busy")
|
||||
real(path)
|
||||
with patch.object(pulse.shutil, "rmtree", flaky), self.assertRaises(RuntimeError) as raised:
|
||||
capture.remove()
|
||||
self.assertIn("etcalib_a", str(raised.exception))
|
||||
self.assertFalse(second.exists())
|
||||
|
||||
def test_only_removes_capture_directories(self):
|
||||
capture = pulse.Capture(20)
|
||||
capture.directory = self.root
|
||||
@@ -328,7 +363,7 @@ class HeartCheck(unittest.TestCase):
|
||||
|
||||
def test_unusual_flags_and_missing_export(self):
|
||||
path = self.write("ref.csv", "time,bpm,flags\n1000,70,6.0\n1001,71,yes\n1002,72,4\n")
|
||||
self.assertEqual(check.read_csv(path), [(1000.0, 70), (1002.0, None)])
|
||||
self.assertEqual(check.read_csv(path), [(1000.0, 70), (1001.0, 71), (1002.0, None)])
|
||||
with tempfile.TemporaryDirectory() as directory:
|
||||
archive = Path(directory) / "other.zip"
|
||||
with zipfile.ZipFile(archive, "w") as z:
|
||||
|
||||
Reference in new issue
Block a user