[midi] Preserve MIDI bindings when saving device settings#1951
[midi] Preserve MIDI bindings when saving device settings#1951ten9876 merged 1 commit intoten9876:mainfrom
Conversation
jensenpat
changed the title
![aethersdr-agent[bot]](https://avatars.githubusercontent.com/in/3286087?s=60&v=4){kind=small}
There was a problem hiding this comment.
Good catch, @jensenpat — this is a real data-loss bug and the fix is clean.
The fix itself is correct. Moving auto bindings = loadBindings() before the QIODevice::WriteOnly open is exactly right. QFile::open(WriteOnly) truncates immediately on POSIX and Windows, so the old ordering was guaranteed to read back an empty binding list. One-line move, well-scoped — nothing to object to here.
Test coverage looks solid. The regression test creates isolated state via QTemporaryDir + QStandardPaths::setTestModeEnabled(true), exercises the exact save-then-reload sequence that triggered the bug, and cleans up after itself. The CMake target follows the existing standalone-binary pattern (no add_test(), consistent with every other test target in this project).
No scope concerns — all three changed files are directly related to the stated fix.
One observation (not blocking): save() and saveBindings() share the same XML serialization loop for writing <Binding> elements. If you're up for a follow-up, extracting a small helper like writeBindingsToWriter(QXmlStreamWriter&, const QVector<MidiBinding>&) would reduce the duplication — but that's optional cleanup, not something to hold this PR for.
Thanks for the thorough write-up and testing. This is ready for @ten9876 to merge from my perspective.
2 participants
Summary
MidiSettings::save()opensmidi.settingsfor writingmidi_settings_testregression harness for preference-only savesWhat we discovered
MidiSettings::save()was intended to update device preferences such asLastDeviceandAutoConnectwhile keeping the existing<Binding>entries. The code openedmidi.settingswithQIODevice::WriteOnlyand then calledloadBindings()from the same path. OnQFile, opening withWriteOnlytruncates the file as soon asopen()succeeds, so the subsequentloadBindings()read happened after the old XML contents were already gone. That made preference-only actions, like connecting a MIDI device or toggling auto-connect, capable of rewriting the file with an empty bindings list.The fix loads the bindings before opening the file for write, then serializes those preserved bindings into the updated settings document.
Testing
cmake -S . -B build -G Ninjacmake --build build -j 10build/midi_settings_testbuild/client_eq_testbuild/client_comp_testbuild/client_gate_testbuild/client_deess_testbuild/client_tube_testbuild/client_pudu_testbuild/client_reverb_testbuild/passive_spots_policy_testbuild/device_diagnostics_testbuild/help_dialog_testQT_QPA_PLATFORM=offscreen build/container_widget_testQT_QPA_PLATFORM=offscreen build/container_manager_testQT_QPA_PLATFORM=offscreen build/container_nesting_testctest --test-dir build -j 10 --output-on-failure(no registered tests in this build dir)Build link: /tmp/aether-midi-settings-preserve-bindings/build/AetherSDR.app
👨🏼💻 Generated with OpenAI Codex (GPT-5.5 Pro 4/23) and tested by @jensenpat