Repository navigation
Fix/console fw safety limits - #322
Merged
Merged
Conversation
Host side of the console firmware safety fixes (console-fw branches fix/cmd-length-bounds, fix/tec-interlock-start and fix/trigger-validation-replies: NEW-1, NEW-2, NEW-3, CFW-05, CFW-06). - MotionUart.send_packet refuses a payload over CONSOLE_MAX_PAYLOAD (2036 bytes, firmware OW_MAX_PAYLOAD) with CommandError instead of sending a frame the console NAKs. - set_trigger_json checks the configuration with the new config.validate_trigger_config (1..40 Hz, trigger pulse < period, laser gate 20..2200 us, delay + gate + skip delay < period) and raises ValueError naming the field. It sends compact JSON and refuses JSON over TRIGGER_JSON_MAX (511 bytes). A config the console rejects is logged and returns None, as before. - start_trigger decodes the status byte of a refused START_TRIG (TRIG_INTERLOCK_HOST_LOSS / _LASER_SAFETY / _TEC, TRIG_START_TIMER_FAILED) into last_start_refusal and last_start_refusal_reason. - ScanWorkflow no longer ignores the SET_TRIG and START_TRIG results: a rejected config or a refused start ends the scan at once through the normal teardown (stop_trigger, disable_camera, source close) and reports the console's reason through last_scan_error / on_error. - tec_status and ConsoleTelemetry docs: tec_good returns True on the first clean poll, but the TEC trigger interlock clears only after 200 clean polls (about 5 s), so a start can be refused while tec_good is True. Tests: tests/test_trigger_limits.py (no hardware) and a ScanWorkflow test for the three trigger failure paths. Bench, console running the three firmware fixes combined (Release-BareMetal): payload limit, config refusals, firmware-rejection path, SDK verdict equal to the firmware verdict for 500 random configs, and the TEC refusal decoded as 0x04 with the start accepted 5.1 s after recovery; 18/18. The SDK console hardware tests pass (35). Older console firmware replies OW_RESP to a refused start, so start_trigger still returns True there. The 40 Hz ceiling matches the firmware fix; the unmerged 60 Hz mode (feature/129-60hz-rate-ll) needs TRIGGER_FREQ_MAX_HZ raised here and in the firmware. Signed-off-by: George Vigelette <george@openwater.cc>
The console firmware trigger frequency range now depends on its build type (console-fw fix/trigger-freq-by-build): Release, including every clinical image, accepts 40 Hz only; Debug and *-dev.* builds accept 1..100 Hz. The SDK cannot tell which build it talks to, so TRIGGER_FREQ_MIN_HZ / _MAX_HZ are the union, 1..100 Hz: - set_trigger_json raises ValueError before sending for anything outside 1..100 Hz (no console build accepts it); - a Release console answers any rate other than 40 Hz with OW_ERROR, and set_trigger_json logs the rejection and returns None, as for any configuration the console rejects. The docstrings and the unit tests use the new range, with frame-fit boundaries at 1, 30, 60 and 100 Hz. Bench, console on fix/trigger-freq-by-build Release-BareMetal, TA trigger and sync out off, no START: 40 Hz -> config dict; 60, 1 and 100 Hz -> None with the console config unchanged; 101 and 0.5 Hz -> ValueError, nothing sent. Software suite: no new failures. Signed-off-by: George Vigelette <george@openwater.cc>
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.
Summary
Licensing layer
license-manifestchange process (and board approval for anything leaving the core). Link the approval.Checklist
git commit -sSPDX-License-Identifierfor their layerCODEOWNERSreview requestedNotes for reviewers