diff --git a/frame/tracking/pulse.py b/frame/tracking/pulse.py index 1170e91..6878bff 100644 --- a/frame/tracking/pulse.py +++ b/frame/tracking/pulse.py @@ -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): diff --git a/scripts/heart-check.py b/scripts/heart-check.py index 46de7e0..1f0c3cd 100644 --- a/scripts/heart-check.py +++ b/scripts/heart-check.py @@ -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 diff --git a/tests/test_pulse.py b/tests/test_pulse.py index 81eb91d..10a8ad1 100644 --- a/tests/test_pulse.py +++ b/tests/test_pulse.py @@ -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: