fix(gui): rebuild the boards a file describes, not the ones it guesses - #133
Merged
Conversation
File > Open guessed each board's driver as "telemetrix" if the saved driver_type was "arduino" else "pigpio". That is right for exactly the two driver types the shipped examples happen to use and wrong for every other one: a Bluetooth adapter and a serial board both came back as Raspberry Pi GPIO boards. The devices on them were then parented to a driver that cannot talk to them and failed to initialise, so a saved experiment stopped working after a reload with nothing saying why. A Maimu stimulator -- BLE, on a bluetooth board -- was exactly this case. It also had no way to set board_type or auto_reconnect, because add_board passes its kwargs to the driver constructor and board_type is applied after construction. Every example file carries board_type "uno". The correct implementation already existed: GliderCore._create_hardware_from_ session, extracted during the file-format work, builds from the BoardConfig and honours all three. The GUI now calls it instead of carrying a second copy. It is exposed as populate_hardware_from_session and made sync, since neither create_board nor create_device ever awaited anything -- the async spellings remain as thin wrappers so existing callers are unaffected. Clearing before repopulating moves into that method, so opening a second file replaces the first one's hardware on both paths rather than only the one that remembered to do it.
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.
A regression on the closed-loop feature that just shipped in #131/#132. Save an experiment with a Maimu on it, reopen it, and the stimulator stops working.
The bug
_populate_hardware_from_sessionguessed each board's driver:Right for exactly the two driver types the shipped examples happen to use, wrong for every other one:
driver_typearduinoraspberry_pibluetoothserialA Bluetooth adapter came back as a Raspberry Pi GPIO board, so the Maimu on it was parented to a driver that cannot talk to it and failed to initialise. The closed loop silently stopped working after a reload, with nothing saying why. Same for any
GenericSerialdevice.It also dropped
board_typeandauto_reconnect—add_boardforwards its kwargs to the driver constructor, andboard_typeis applied after construction, so it had no way to set it. Every file inexamples/carriesboard_type: "uno".Worth noting
glider --filewas already correct:load_experimentgoes through the core, which builds from theBoardConfig. Only the GUI's own copy guessed.The fix
Delete the guess. The correct implementation already existed —
GliderCore._create_hardware_from_session, extracted during the file-format work — and builds from theBoardConfig, honouring driver type,board_typeandauto_reconnect. The GUI now calls it instead of carrying a second copy.Exposed as
populate_hardware_from_sessionand made sync, because neithercreate_boardnorcreate_deviceever awaited anything — they were async by declaration only. Both async spellings remain as thin wrappers, so existing callers are untouched.Clearing-before-repopulating moves into that method too, so opening a second file replaces the first one's hardware on both paths rather than only the one that remembered to do it.
Tests — 9 new
tests/unit/gui/test_open_restores_board_drivers.py: every driver type rebuilt as itself (parametrised over all four);board_typeandauto_reconnectsurviving; a Maimu reopened from a saved experiment landing on aBLEBoard— the regression that matters; reopening replacing rather than stacking; and a file naming a driver this install doesn't have still opening, with the other boards intact.All nine failed before the fix.
Verification
PYTHONPATH=src QT_QPA_PLATFORM=offscreen pytest tests/— 3651 passed, 3 skipped, ruff and black clean.