Conversation
Mirrors the firmware's new command. Bounds are the firmware's bounds, and width > period is rejected here so the researcher sees an error rather than a device that silently did nothing.
Old graphs still load; a dropped duration_s is reported rather than silently becoming a different stimulus.
pulse(500, 500, 1, 40) -- the natural way to write one 500 ms pulse at 40% -- passed validation and reached the firmware, which took its continuous-light branch and never read count. The LED came on and stayed on. The Maimu node is fire-and-continue and never writes "off", so one behaviour bout could leave an implanted LED lit for the rest of the session while the command read as bounded. The guard near line 224 only rejected width > period, passing on equality. It now also rejects width >= period with a non-zero count, naming pulse_width_ms and saying that continuous light is spelled with count = 0. Nothing is lost: a bounded single 500 ms pulse is pulse(1000, 500, 1, 40), where count = 1 ends the train at the first pulse end so the trailing gap never elapses. The matching firmware parser change is in the Maimu repo. Also: - test_pulse_schema.py now compares MaimuNode.PROPERTIES_SCHEMA's four pulse fields against MaimuDevice.ACTION_ARGS_SCHEMA["pulse"] field-for-field -- key, min, max, default and type. node.py's comment claimed test_pulse_schema.py asserted this; it did not. It compared the device schema's defaults against the DEFAULT_* constants and its bounds against hardcoded literals, leaving the node's own min/max unguarded. They agreed, but only the comment stood between agreement and a spin box offering a value that raises mid-experiment. - The package docstring still described the retired two-field grammar as the characteristic's contract. It was the last place in either repo doing so. - The help text and README taught "a width equal to the period is continuous light" without the count = 0 half, which is the sentence that made the bug look like the documented way to do it. Verified: 102 passed (was 99) with PYTHONPATH="src:plugins/glider-maimu/src" QT_QPA_PLATFORM=offscreen uv run pytest plugins/glider-maimu/tests/ -v; ruff check clean.
The lint job runs ruff, black --check and mypy; only ruff was run locally. Formatting only -- the error-message f-string is merged into one rather than left as Black's one-line implicit concatenation.
test_camera_panel_rehearsal.py lives in the main suite, not the plugin's, so every plugin-scoped sweep missed it. It set stim.duration_s, which is no longer a node property and so silently became a dead attribute -- the node emitted its defaults and the stale assertion caught it. Also records the grammar change under Unreleased.
# Conflicts: # CHANGELOG.md
This branch has not been deployed
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.
Moves the Maimu device and node to the stimulator's new wire grammar. Companion to LaingLab/Maimu#1 — the two must land together, since this mirrors that firmware's command format byte-for-byte.
The old grammar was
<period_ms>,<duration_s>— a fixed 50%-duty toggle, so the standard optogenetic protocol of 4 ms pulses at 20 Hz (8% duty) could not be expressed. The new one is<period_ms>,<pulse_width_ms>,<count>,<intensity_pct>.What changed
MaimuDevice.pulse()takes four arguments instead of two, with bounds matching the firmware parser exactly:period_ms1…3,600,000,pulse_width_ms1…period_ms,count0…65,535,intensity_pct0…100.count = 0means run until stopped andintensity_pct = 0is a legitimate armed-but-dark control condition — those two are the only fields with a floor of 0.MaimuNodegains Pulse width, Pulses and Intensity properties alongside Period.Validation happens here, before anything is written. The firmware rejects malformed commands silently — it has no read characteristic and so no way to answer back — so this is the only place a researcher can learn that a command was wrong. A rejected pulse writes nothing at all.
This also closes a latent bug: the old
_whole_numberenforced only a floor of 1 and never the maxima its own schema advertised, so a caller could previously sendperiod_ms=99999999.Old graphs
A
.gliderfile saved with the old grammar still loads, but it loads as a different stimulus.set_statenow logs a warning naming the droppedduration_sand what replaced it, so that surfaces at load time rather than in data that doesn't look right. Tested in both directions — it fires on an old file and stays silent on a current one.Safety
shutdown()still writesoffbefore disconnecting, and_on_reconnectedstill writesofffirst on a recovered link. That matters more than before: a train runs autonomously in firmware, and after this changecount = 0is the only spelling of continuous light — so the guarantee that emergency stop, End Experiment, app quit and reconnect all route throughoffis load-bearing rather than one of two paths.One bug worth calling out, found in final review and fixed here:
width == periodwith a non-zerocountwas accepted by both sides and ran forever, because the firmware's continuous branch never readcount.500,500,1,40— the natural way to write one 500 ms pulse — would have left an implanted LED lit for the rest of the session, and the node is fire-and-continue so nothing would have stopped it. Both sides now reject that combination. Nothing is lost:1000,500,1,40still delivers exactly one bounded 500 ms pulse.Tests
102 passing,
ruffclean. Coverage includes the closed-loop chain end to end (classifier frame → bus → Behavior Input node → Maimu node → device → GATT write, asserting on real bytes), the reconnect path, and a new cross-check that the node's schema bounds match the device's — previously a comment claimed a test asserted that, and none did.Still never run against physical hardware. The plugin's suite is built on a fake BLE client, and the firmware it targets needs a board respin before it can drive an LED at all.