fix(checks): hoist camera threshold validations to fail early on came… - #459
fix(checks): hoist camera threshold validations to fail early on came…#459Chama2001 wants to merge 15 commits into
Conversation
|
👋 Hi @Chama2001 — thank you so much for your first contribution to HFlow! A maintainer will review your pull request as soon as possible. In the meantime:
💡 Tip: one open pull request per contributor at a time. Issues with an assignee are taken; everything else is fair game. We are excited to have you here and appreciate your help making the project better! 🙌 |
There was a problem hiding this comment.
Four things.
Nothing tests any of it. None of the five messages appears anywhere in the repo, so removing any line you added leaves the suite green. #447 asks for every case in its table, at least one on a camera-less episode.
Three of the five parameters still accept True. freeze_min_duration_s, freeze_noise_db and bright_luma_threshold pass as 1.
type(x) is bool and TypeError: everywhere else here is isinstance and ValueError. See _field_guards.require_int and #441/#446.
import math inside two function bodies: use the np.isfinite already imported.
Also: #447 asked you to rule on camera_fps_conformance either way. Say which.
…a-less The three float-typed parameters still took True: bool subclasses int, so True reads as 1 and passes both the range comparisons and isfinite. Adds the missing clauses, moves the two TypeErrors to ValueError to match require_int and Hebbian-Robotics#441/Hebbian-Robotics#446, swaps type() is bool for isinstance, and drops math in favour of the np already imported. The signal_quality test now runs on a camera-less episode with match=. On an episode with cameras a bare pytest.raises(ValueError) is also satisfied by a camera-processing failure, so it could not tell whether the guard was still there.
There was a problem hiding this comment.
Getting close. camera_signal_quality is done; camera_frame_stats still has no tests.
Pushed the three mechanical items rather than sending you back: the missing bool clauses, TypeError to ValueError, isinstance, and np.isfinite. All eight parameters now refuse True.
Also rewrote your test. It used an episode with cameras and a bare pytest.raises(ValueError), which a camera-processing failure also satisfies, so it passed whether or not the guard was there. That is the bug #447 is about, inside its own test.
What is left: the same treatment for camera_frame_stats, ten rows.
On camera_fps_conformance: leave it out, I will file it.
2 participants
Closes #447
Summary
Moved threshold guards before the camera loop in
camera_frame_statsandcamera_signal_quality.Why
As discussed in #447, previously these guards were inside the
for topic in selected_cameras:loop, meaning they were skipped entirely on camera-less episodes. This change hoists them to the top of the functions so they fail early and correctly.Validation
Applied the exact pattern requested in the issue description.