From aae02f31d87d2518196def5114c300550437175b Mon Sep 17 00:00:00 2001 From: Deva Date: Thu, 3 Sep 2026 16:54:42 +0530 Subject: [PATCH 1/5] Let a kit own its words instead of pointing at the firmware's MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Kit named its id, its pads and their sources, and its dice loops with const char*, which can only point at string literals compiled into the image. A kit read off a card has nowhere to keep its words, so it could never be the same type as the one built in — and two types for one idea would mean every reader of a kit choosing between them. They are fixed arrays now, sized by the same limits the share format uses. The generated header needed no change at all: a char array initialises from a string literal exactly as a pointer did. It costs about a kilobyte, most of it the four dice loops at a section code's full length. Co-Authored-By: Claude Opus 5 --- firmware/src/engine/kit.h | 11 +++++++---- firmware/src/engine/limits.h | 2 ++ tests/sound_support.h | 4 +++- 3 files changed, 12 insertions(+), 5 deletions(-) diff --git a/firmware/src/engine/kit.h b/firmware/src/engine/kit.h index c5d8b47..5497f7d 100644 --- a/firmware/src/engine/kit.h +++ b/firmware/src/engine/kit.h @@ -9,6 +9,9 @@ // is the source and tools/kit_builder.py generates engine/kits/.h from it. The // engine reads sends, templates, progressions, the pluck sequence, dice loops and // the swing/filter/fx defaults; the voice fields are for sound/. +// +// Every string here is a fixed array rather than a pointer, so a kit read off the +// card owns its own words and is the same type as the one compiled in (D-109). namespace engine { enum class Voice : uint8_t { sample, synth }; @@ -20,9 +23,9 @@ struct TapTemplate { }; struct KitPad { - const char* name; + char name[kPadNameLength + 1]; Voice voice; - const char* source; // sample pads: the wav file; synth pads: the preset name + char source[kPadSourceLength + 1]; // sample pads: the wav file; synth pads: the preset name int8_t pitch_semitones; // sample pads float start; // sample pads: start point as a fraction of the sample float decay; // sample pads: 1.0 = full length @@ -47,12 +50,12 @@ struct Sidechain { }; struct Kit { - const char* id; + char id[kKitIdLength + 1]; KitPad pads[kTrackCount]; DegreeList progressions[kModeCount]; // indexed by Mode DegreeList pluck_sequence; uint8_t dice_loop_count; - const char* dice_loops[kMaxDiceLoops]; // share codes; only their tracks are used (D-028) + char dice_loops[kMaxDiceLoops][kSectionCodeCapacity]; // share codes; only their tracks are used (D-028) uint8_t swing_hundredths; Tenths filter; Tenths fx; diff --git a/firmware/src/engine/limits.h b/firmware/src/engine/limits.h index 50839b4..d60ce6b 100644 --- a/firmware/src/engine/limits.h +++ b/firmware/src/engine/limits.h @@ -16,6 +16,8 @@ constexpr int kMaxDiceLoops = 4; // starting loops per kit (§8.2, D-0 constexpr int kMaxTapTemplates = 4; // smart-default taps a kit may define per pad; lofi's kick uses four (§6.6) constexpr int kModeCount = 5; // minor, major, dorian, pentatonic minor, pentatonic major (Appendix B) constexpr int kKitIdLength = 12; // kit id, 1–12 characters (share-format §2) +constexpr int kPadNameLength = 12; // what a pad is called, as the text view and the status line say it +constexpr int kPadSourceLength = 24; // a sample pad's wav file, or a synth pad's preset name constexpr int kLineageLength = 6; // base36 id of the parent loop (§10.1) // Speed 2 plays the step list twice per cycle, so hits and ghost slots both double. diff --git a/tests/sound_support.h b/tests/sound_support.h index 16c60bf..befcc8e 100644 --- a/tests/sound_support.h +++ b/tests/sound_support.h @@ -3,6 +3,8 @@ // samples are loaded only where the scenario is about them. #pragma once +#include + #include #include #include @@ -168,7 +170,7 @@ inline engine::Kit kit_with_sample_chord() { engine::Kit kit = lofi(); engine::KitPad& chord = kit.pads[engine::index_of(engine::Pad::chord)]; chord.voice = engine::Voice::sample; - chord.source = "tone.wav"; + std::strcpy(chord.source, "tone.wav"); chord.pitch_semitones = 0; chord.start = 0.0f; chord.decay = 1.0f; From a615b918eca633deabf8c891199ae39242d57975 Mon Sep 17 00:00:00 2001 From: Deva Date: Thu, 3 Sep 2026 17:02:49 +0530 Subject: [PATCH 2/5] Read the kit off the card, so a kit no longer means a rebuild MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A kit only existed as a C++ header, so changing a sound, a chord progression or a tap template meant rebuilding the firmware. §12 rule 6 asks for an open format the community can write kits in. tools/kit_builder.py now writes kits//kit.txt beside the header, both from the one kit.json, and io::load_kit reads it. One line a field, the five progressions in mode order, the eight pads in the order the share format fixes with each pad's templates under it, and a step spelled the way a share code spells one — engine::read_step is now the only implementation of that table, so the decoder and the kit reader cannot drift. Fractions are whole hundredths, so the device needs no float parser. settings.txt's kit= line says which folder to play. A kit that is not there logs the path and leaves the built-in kit playing, so the device always comes up; the simulator reads its kit off its own card the same way, which is also how app::init came to read everything itself instead of being handed samples by each entry point. An unknown line fails the whole file, unlike an unknown settings row, which is skipped. A setting this firmware ignores costs one row; a kit field it ignored would be an instrument quietly playing something other than what the kit says. The test that the card's kit equals the compiled kit, field for field, is what holds the builder's two outputs together, and CI diffs both. Co-Authored-By: Claude Opus 5 --- .github/workflows/ci.yml | 13 ++- DECISIONS.md | 1 + firmware/src/app/app.cpp | 37 +++--- firmware/src/app/app.h | 13 ++- firmware/src/app/card.cpp | 7 +- firmware/src/app/card.h | 7 +- firmware/src/engine/kit.cpp | 51 +++++++++ firmware/src/engine/kit.h | 5 + firmware/src/engine/share.cpp | 18 ++- firmware/src/engine/share.h | 5 + firmware/src/io/kit.cpp | 208 ++++++++++++++++++++++++++++++++++ firmware/src/io/kit.h | 12 +- firmware/src/io/lines.cpp | 21 ++++ firmware/src/io/lines.h | 12 ++ firmware/src/io/store.cpp | 40 +++---- firmware/src/io/store.h | 3 +- firmware/src/main.cpp | 13 +-- host/main.cpp | 11 +- spec/kits/lofi/kit.txt | 37 ++++++ spec/scenarios.md | 3 +- tests/app_support.h | 30 +++-- tests/io_test.cpp | 86 ++++++++++++-- tests/ui_test.cpp | 3 +- tools/kit_builder.py | 80 ++++++++++++- 24 files changed, 606 insertions(+), 110 deletions(-) create mode 100644 firmware/src/engine/kit.cpp create mode 100644 firmware/src/io/lines.cpp create mode 100644 firmware/src/io/lines.h create mode 100644 spec/kits/lofi/kit.txt diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ffcd029..6522f6f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -17,17 +17,18 @@ jobs: # engine/ and sound/ must compile anywhere: no Arduino, Teensy, SDL or hal/ includes (D-029). run: | ! grep -rnE '#include[[:space:]]*[<"](Arduino|SDL|hal/|WProgram|core_pins|imxrt|usb_|EEPROM|SD\.h|Wire|SPI|Audio)' firmware/src/engine firmware/src/sound - - name: Generated kit headers match spec/kits (PRD §12 rule 6) - # Kits are data: the engine compiles a header generated from each kit.json. - # Regenerate every kit and diff, so an edit to either side cannot hide; the - # intent-to-add makes a header that was never committed show up too. + - name: Generated kit files match spec/kits (PRD §12 rule 6) + # Kits are data: from each kit.json the builder writes the header the engine + # compiles in and the kit.txt the device reads off the card (D-109). Regenerate + # every kit and diff both, so an edit to any side cannot hide; the intent-to-add + # makes a file that was never committed show up too. run: | for kit in spec/kits/*/kit.json; do id="$(basename "$(dirname "$kit")")" python3 tools/kit_builder.py "$kit" "firmware/src/engine/kits/$id.h" done - git add -N firmware/src/engine/kits - git diff --exit-code -- firmware/src/engine/kits + git add -N firmware/src/engine/kits spec/kits + git diff --exit-code -- firmware/src/engine/kits spec/kits - name: Python tools (T-76) # The sample generator and the kit builder's WAV checks, tested where they live. run: python3 -m unittest discover -s tests -p 'tools_test.py' -v diff --git a/DECISIONS.md b/DECISIONS.md index bfd8b97..947c95e 100644 --- a/DECISIONS.md +++ b/DECISIONS.md @@ -112,3 +112,4 @@ What was decided, why, and when to look again. One row per decision. IDs are seq | D-106 | The share-code size cap (T-62) lives in `engine::decode` and `decode_song`, not at each path the bytes arrive on: 512 characters for a section and 2048 for a song, NUL not counted, scanning at most one past the limit so a missing terminator costs a bounded read. | 2026-09-03. The engine's decoder is linear and allocates nothing, so the cap is about refusing a code rather than about safety; putting it where the knowledge is means the card, `tools/render`, and the SysEx, USB and paste-box paths still to come are all covered without each remembering to. Twice the canonical worst case (238 and 988) rather than the buffer sizes, because T-16 says a decoder skips the fields a later version adds and a cap at the buffer would leave almost no room for them. Rejected: a cap at each arriving path (three places to forget, and nothing to write today since none of those paths exists yet). | If a later version's fields need more than the canonical code's length again. | | D-107 | A song slot whose file will not parse (§9.6, T-97): a tap refuses it and says `hold to replace song 2`; a hold on that pad copies the song on screen over it and says `song 2 replaced`. The hold is live only on a slot the device already knows it cannot read, so no hold can destroy a song the player could still open, and a hold anywhere else in the song view still does nothing. | 2026-09-03. D-104 made a file that will not parse a slot rather than a gap, which stopped a pick from silently copying over somebody's song but left the slot unreachable: the only thing that replaced it was the player already being on it, so recovery meant a computer or a factory reset — a dead pad on an instrument that ships with no manual. Press-then-hold is already this device's idiom for a destructive thing, in `hold dice to clear` and `hold play to reset`, and §9.6 said the pads' hold gestures were inactive in the song view, so the gesture was free and consistent rather than new; PRD §9.6 gains the exception. Rejected: a second tap inside the arming timeout (a double tap is easy by accident, and every other confirmation here is a hold); quarantining the file at boot (needs a HAL rename or delete, and silently moves a player's file aside); a settings row to clear a slot (a sub-menu for a rare failure, against §9.6's one screen). | If usability round 1 shows nobody finds the hold, or if a fourth tile state would say it better than the status line. | | D-108 | A kit's samples come off the card, not the firmware image: `io::load_samples` reads `kits//` into the PSRAM `hal::sample_memory()` hands it — 1.5 MB, the eight pads of two seconds D-081 allows each — and fills a `sound::SampleBank` with what it found. Each WAV is read straight into the room left in that memory and parsed where it lands, then moved down over its own header, so nothing is ever staged twice. A file that is missing or is not 16-bit 48 kHz mono PCM costs its own pad its sound and no more. The simulator does the same thing with the same code: `host/CMakeLists.txt` seeds `out/sdcard/kits/` from `spec/kits/` at configure time. | 2026-09-03. §7.5 puts the samples in PSRAM and the kits on the card, and until now the device passed `app::init` an empty bank, so the drum pads were silent on hardware and the simulator used the render tool's host-only loader — two paths, one of them not the product's. Reading in place rather than through a staging buffer because a sample is up to 192 KB and the device has 63 KB of ordinary RAM free; there is nowhere to put a copy. `hal::sample_memory()` answers with nothing when no PSRAM is fitted, which is every board until bring-up, because writing to a section that is not backed would fault rather than fail. Rejected: samples in the firmware image (a kit could never be changed without a rebuild, which is what §12 rule 6 is against); streaming from the card at play time (file I/O in the audio path, forbidden by §12 rule 4). | At bring-up, when a real PSRAM chip says whether reads from it keep up with sixteen voices; or if a kit needs more than eight samples of two seconds. | +| D-109 | A kit is readable text on the card: `tools/kit_builder.py` writes `kits//kit.txt` beside the header it already generates, both from the one `kit.json`, and `io::load_kit` reads it. One line a field, the five progressions in mode order, the eight pads in the order share-format §2 fixes with each pad's tap templates under it, and a step spelled exactly as a share code spells one — `engine::read_step` is now the only implementation of that table. Fractions are whole hundredths, so the device needs no float parser. `engine::Kit` holds its strings in fixed arrays rather than pointers, so a card kit and a compiled kit are one type, and `settings.txt`'s `kit=` line says which folder to play, falling back to the built-in kit when the card has no such kit. | 2026-09-03. §12 rule 6 asks for an open format so the community can make kits, and a kit that only exists as a C++ header can only be made by rebuilding the firmware. Text because §7.6 shows the card over USB and because the songs and settings are already text (D-104): one habit, not three. Rejected: parsing `kit.json` on the device (a JSON parser is a few hundred lines of firmware at a boundary that reads whatever a card holds, against the rule about reaching for the standard library first) and a packed binary (smaller and faster, but then only the tool can make a kit, which is the opposite of open). An unknown line fails the whole file rather than being skipped as an unknown settings row is: a setting the device ignores costs one row, a kit field it ignores would be an instrument quietly playing something other than what the kit says. The test that the card kit equals the compiled kit is what holds the builder's two outputs together, and CI diffs both. | If a kit needs a field that is not a number, a name or a share code; or when kits can be chosen from the settings view, which needs the samples reloaded rather than just the file re-read. | diff --git a/firmware/src/app/app.cpp b/firmware/src/app/app.cpp index 236de76..8f4818c 100644 --- a/firmware/src/app/app.cpp +++ b/firmware/src/app/app.cpp @@ -11,6 +11,7 @@ #include "engine/kits/lofi.h" #include "engine/share.h" #include "hal/hal.h" +#include "io/kit.h" #include "io/share.h" #include "ui/color.h" #include "ui/draw.h" @@ -46,14 +47,17 @@ constexpr int kLineCapacity = 320; constexpr int kFooterCapacity = 32; const char* const kTapMarker = "tap"; // the top row while tap tempo waits (§8.2, D-102) -const engine::Kit& kit = engine::kits::kLofi; +// The card may replace it at init; the scheduler and controller hold its address, +// so it is filled in place rather than swapped. +engine::Kit the_kit = engine::kits::kLofi; +sound::SampleBank the_samples; // The engine (207 KB) and the model (85 KB) go to the platform's bulk memory; the // scheduler's 30 KB event list and the queues stay with the ordinary statics. HAL_BULK_MEMORY sound::Engine sound_engine; -HAL_BULK_MEMORY Model the_model(kit); -Scheduler scheduler(kit); -Controller controller(kit); +HAL_BULK_MEMORY Model the_model(the_kit); +Scheduler scheduler(the_kit); +Controller controller(the_kit); AudioPath audio; FiredLog the_fired_log; uint64_t last_frame_us = 0; @@ -196,7 +200,7 @@ void draw_song(uint16_t* framebuffer) { // milliseconds, a frame is not. The code carries the loop's own id, not the id of // the loop it came from, which stays in the state for the footer (§10.2, D-105). void draw_share(uint16_t* framebuffer, int bottom) { - const engine::SectionCode code = io::shared_code(frame_state, kit); + const engine::SectionCode code = io::shared_code(frame_state, the_kit); if (std::strcmp(code.text, shown_code.text) != 0) { shown_code = code; ui::encode_share_qr(shown_code.text, qr); @@ -206,7 +210,7 @@ void draw_share(uint16_t* framebuffer, int bottom) { void draw_settings(uint16_t* framebuffer) { const io::Settings& settings = frame.settings; - const ui::SettingsModel model{frame_state.key, frame_state.swing, kit.id, settings.brightness, + const ui::SettingsModel model{frame_state.key, frame_state.swing, the_kit.id, settings.brightness, settings.sleep_minutes, settings.midi_clock_in, settings.midi_clock_out, settings.sync_in, settings.sync_out, kFirmwareVersion, frame.settings_cursor}; ui::draw_settings_view(framebuffer, model); @@ -259,7 +263,7 @@ void draw(uint64_t now_us) { draw_ring(framebuffer, position, bottom); break; case View::text: - ui::draw_text_view(framebuffer, frame_state, kit, bottom); + ui::draw_text_view(framebuffer, frame_state, the_kit, bottom); break; case View::song: draw_song(framebuffer); @@ -322,14 +326,16 @@ bool tutorial_done() { // Builds everything afresh, so the tests can start over as often as they like; on // the device it runs once. Placement new is construction in place, not heap // allocation, and it keeps an 85 KB model off the stack. -void init(const sound::SampleBank& samples) { +void init() { const bool first_run = !tutorial_done(); - read_card(kit); // both reads happen before the lock: a card takes milliseconds (D-104) + // Everything the card has to say, before the lock: a card takes milliseconds (D-104). + read_card(the_kit); + io::load_samples(the_kit, the_samples); hal::lock(); // a timer already ticking (the harness re-initialises) cannot see the app half made new (&sound_engine) sound::Engine(); - new (&the_model) Model(kit); - new (&scheduler) Scheduler(kit); - new (&controller) Controller(kit); + new (&the_model) Model(the_kit); + new (&scheduler) Scheduler(the_kit); + new (&controller) Controller(the_kit); audio.reset(); the_fired_log = FiredLog{}; last_frame_us = 0; @@ -339,11 +345,11 @@ void init(const sound::SampleBank& samples) { applied_brightness = -1; the_model.tutorial = Tutorial{first_run, 0, false}; apply_card(the_model); // the settings and the song the device was left on - audio.init(sound_engine, kit, samples); + audio.init(sound_engine, the_kit, the_samples); const uint32_t seed = static_cast(hal::now_us()); scheduler.set_seed(seed); controller.set_seed(seed); - audio.params.publish(params_of(the_model.sections[0].state(), kit, the_model.master_volume)); + audio.params.publish(params_of(the_model.sections[0].state(), the_kit, the_model.master_volume)); hal::unlock(); hal::start_audio(&render); hal::start_timer(kTimerPeriodUs, &on_timer); @@ -370,13 +376,14 @@ void tick() { draw(now_us); hal::present(); light_leds(frame_position, frame_state); - keep_card(now_us, the_model, kit); + keep_card(now_us, the_model, the_kit); if (frame.settings.brightness != applied_brightness) { applied_brightness = frame.settings.brightness; hal::set_brightness(applied_brightness); } } +const engine::Kit& kit() { return the_kit; } const Model& model() { return the_model; } const FiredLog& fired_log() { return the_fired_log; } int64_t audio_position() { diff --git a/firmware/src/app/app.h b/firmware/src/app/app.h index 54d31cc..136e6b9 100644 --- a/firmware/src/app/app.h +++ b/firmware/src/app/app.h @@ -12,11 +12,14 @@ // audio and timer callbacks are registered with the HAL by init. namespace app { -// `samples` holds the kit's WAVs, or empty samples: the platform entry point -// provides them (the host reads spec/kits/, the device waits for io/). Once at -// start-up on a platform; the test harness calls it again between cases, which -// is safe because its audio callback runs only when the test calls it. -void init(const sound::SampleBank& samples); +// Brings the app up on whatever the card holds: the kit the settings name and its +// samples, the songs, the settings themselves. Once at start-up on a platform; the +// test harness calls it again between cases, which is safe because its audio callback +// runs only when the test calls it. +void init(); + +// The kit being played, which is the card's or the one built in (D-109). +const engine::Kit& kit(); // Input, holds and timeouts, the fired log, a frame when one is due. void tick(); diff --git a/firmware/src/app/card.cpp b/firmware/src/app/card.cpp index 5961a2f..821aab4 100644 --- a/firmware/src/app/card.cpp +++ b/firmware/src/app/card.cpp @@ -4,6 +4,8 @@ #include "engine/state.h" #include "hal/hal.h" +#include "engine/kits/lofi.h" +#include "io/kit.h" #include "io/store.h" namespace app { @@ -186,8 +188,11 @@ bool due(bool differs, uint64_t now_us, bool& dirty, uint64_t& changed_us) { } // namespace -void read_card(const engine::Kit& kit) { +void read_card(engine::Kit& kit) { io::load_settings(boot_settings); + // The kit the settings name, or the one built in when the card has no such kit — + // a device with an unreadable kit folder still plays (D-109). + if (!io::load_kit(boot_settings.kit, kit)) kit = engine::kits::kLofi; io::LoadResult current = io::LoadResult::missing; for (int slot = io::kFirstSlot; slot <= io::kLastSlot; ++slot) { const bool is_current = slot == boot_settings.song; diff --git a/firmware/src/app/card.h b/firmware/src/app/card.h index 6b563ae..7a11f21 100644 --- a/firmware/src/app/card.h +++ b/firmware/src/app/card.h @@ -12,9 +12,10 @@ namespace app { // Boot, in two halves so that holds at start-up too: read_card takes the settings, -// the song they name and which slots hold a song off the card, and apply_card puts -// them into a model the caller has just built, under the lock. -void read_card(const engine::Kit& kit); +// the kit they name, the song they name and which slots hold a song off the card, and +// apply_card puts them into a model the caller has just built, under the lock. The +// kit is read before the songs, since a song's code is decoded against it. +void read_card(engine::Kit& kit); void apply_card(Model& model); // Every frame: the pick the song view made, the erase a factory reset asked for, diff --git a/firmware/src/engine/kit.cpp b/firmware/src/engine/kit.cpp new file mode 100644 index 0000000..eca8150 --- /dev/null +++ b/firmware/src/engine/kit.cpp @@ -0,0 +1,51 @@ +#include "engine/kit.h" + +#include + +namespace engine { + +namespace { + +bool same(const DegreeList& a, const DegreeList& b) { + return a.length == b.length && std::memcmp(a.degrees, b.degrees, a.length) == 0; +} + +bool same(const TapTemplate& a, const TapTemplate& b) { + if (a.step_count != b.step_count) return false; + for (int i = 0; i < a.step_count; ++i) { + if (a.steps[i] != b.steps[i]) return false; + } + return true; +} + +bool same(const KitPad& a, const KitPad& b) { + if (std::strcmp(a.name, b.name) != 0 || a.voice != b.voice || std::strcmp(a.source, b.source) != 0) return false; + if (a.pitch_semitones != b.pitch_semitones || a.start != b.start || a.decay != b.decay) return false; + if (a.octave != b.octave || a.send != b.send || a.template_count != b.template_count) return false; + for (int i = 0; i < a.template_count; ++i) { + if (!same(a.templates[i], b.templates[i])) return false; + } + return true; +} + +} // namespace + +bool operator==(const Kit& a, const Kit& b) { + if (std::strcmp(a.id, b.id) != 0) return false; + for (int i = 0; i < kTrackCount; ++i) { + if (!same(a.pads[i], b.pads[i])) return false; + } + for (int i = 0; i < kModeCount; ++i) { + if (!same(a.progressions[i], b.progressions[i])) return false; + } + if (!same(a.pluck_sequence, b.pluck_sequence)) return false; + if (a.dice_loop_count != b.dice_loop_count) return false; + for (int i = 0; i < a.dice_loop_count; ++i) { + if (std::strcmp(a.dice_loops[i], b.dice_loops[i]) != 0) return false; + } + return a.swing_hundredths == b.swing_hundredths && a.filter == b.filter && a.fx == b.fx && + a.sidechain.on == b.sidechain.on && a.sidechain.duck_db == b.sidechain.duck_db && + a.sidechain.release_ms == b.sidechain.release_ms; +} + +} // namespace engine diff --git a/firmware/src/engine/kit.h b/firmware/src/engine/kit.h index 5497f7d..b3d1bdb 100644 --- a/firmware/src/engine/kit.h +++ b/firmware/src/engine/kit.h @@ -62,6 +62,11 @@ struct Kit { Sidechain sidechain; }; +// A kit read off a card and the one compiled in are the same kit or they are not: +// the test that says so is what keeps tools/kit_builder.py's two outputs in step. +bool operator==(const Kit& a, const Kit& b); +inline bool operator!=(const Kit& a, const Kit& b) { return !(a == b); } + inline const KitPad& pad_of(const Kit& kit, Pad pad) { return kit.pads[index_of(pad)]; } // The scale degree a melodic step's position selects; a position past the end diff --git a/firmware/src/engine/share.cpp b/firmware/src/engine/share.cpp index 26a5625..ca7f569 100644 --- a/firmware/src/engine/share.cpp +++ b/firmware/src/engine/share.cpp @@ -244,13 +244,8 @@ bool read_track(Reader& reader, Track& track) { track.step_count = 0; while (!reader.at_any_of(kStepStops)) { if (track.step_count >= kMaxStepsPerTrack) return false; - const char c = reader.peek(); Step step{0, 0}; - if (c != kRestChar) { - const int value = base36_value(c); - if (value < 0 || value >= kMaxStepValue) return false; - step = Step{static_cast(value / kNotesPerHitRow + 1), static_cast(value % kNotesPerHitRow)}; - } + if (!read_step(reader.peek(), step)) return false; track.steps[track.step_count++] = step; reader.advance(); } @@ -332,6 +327,17 @@ bool operator==(const Song& a, const Song& b) { return true; } +bool read_step(char c, Step& step) { + if (c == kRestChar) { + step = Step{0, 0}; + return true; + } + const int value = base36_value(c); + if (value < 0 || value >= kMaxStepValue) return false; + step = Step{static_cast(value / kNotesPerHitRow + 1), static_cast(value % kNotesPerHitRow)}; + return true; +} + Decoded decode(const char* code, const Kit& kit) { Decoded result{}; if (!within_limit(code, kMaxSectionCodeInput)) return result; diff --git a/firmware/src/engine/share.h b/firmware/src/engine/share.h index d09cab6..7cf9b36 100644 --- a/firmware/src/engine/share.h +++ b/firmware/src/engine/share.h @@ -47,6 +47,11 @@ struct DecodedSong { Song song; }; +// The step spelling of share-format §2: `.` is a rest, otherwise base36 of +// (hits − 1) × 8 + note. False when the character is neither. io/ reads a kit's tap +// templates with this table too, so a step has one spelling in the repo (D-109). +bool read_step(char c, Step& step); + Decoded decode(const char* code, const Kit& kit); SectionCode encode(const State& state, const Kit& kit); diff --git a/firmware/src/io/kit.cpp b/firmware/src/io/kit.cpp index 0c80ff8..ed5d624 100644 --- a/firmware/src/io/kit.cpp +++ b/firmware/src/io/kit.cpp @@ -3,7 +3,9 @@ #include #include +#include "engine/share.h" #include "hal/hal.h" +#include "io/lines.h" #include "sound/limits.h" namespace io { @@ -11,6 +13,13 @@ namespace io { namespace { constexpr int kPathCapacity = 64; +// The file is four dice loops at a section code's full length, eight pads with their +// templates, and a line each for the rest. +constexpr uint32_t kKitFileCapacity = 4096; +constexpr int kMaxKitLines = 96; +constexpr int kMaxFields = 8; // a pad line, the longest +constexpr const char* kKitPrefix = "RTK1"; +char kit_file_[kKitFileCapacity]; constexpr uint32_t kRiffHeaderBytes = 12; constexpr uint32_t kChunkHeaderBytes = 8; constexpr uint32_t kFormatChunkBytes = 16; // the least a fmt chunk may hold @@ -82,6 +91,185 @@ bool find_pcm(const uint8_t* bytes, uint32_t size, const char* path, uint32_t& o } // namespace + +// ---- the kit itself ------------------------------------------------------------- + +namespace { + +// One line's `key=value`, split where the caller's grammar says. Returns how many +// fields the value held, or -1 when the line is not this key or has too many. +int fields_of(char* line, const char* key, char** fields, int capacity) { + const size_t length = std::strlen(key); + if (std::strncmp(line, key, length) != 0 || line[length] != '=') return -1; + char* at = line + length + 1; + int count = 0; + for (;;) { + if (count == capacity) return -1; + fields[count++] = at; + char* comma = std::strchr(at, ','); + if (comma == nullptr) return count; + *comma = '\0'; + at = comma + 1; + } +} + +// A whole number in `text`, 0 to `most`. False on anything else, so nothing a card +// holds becomes a value the rest of the firmware would not have produced. +bool number(const char* text, int most, int& out) { + int value = 0; + int digits = 0; + for (const char* c = text; *c != '\0'; ++c) { + if (*c < '0' || *c > '9') return false; + value = value * 10 + (*c - '0'); + if (++digits > 5 || value > most) return false; + } + out = value; + return digits > 0; +} + +bool copy_word(const char* from, char* into, int capacity) { + const size_t length = std::strlen(from); + if (length == 0 || length >= static_cast(capacity)) return false; + std::memcpy(into, from, length + 1); + return true; +} + +bool read_degrees(char** fields, int count, engine::DegreeList& list) { + if (count < 1 || count > engine::kMaxNoteSequenceLength) return false; + for (int i = 0; i < count; ++i) { + int degree = 0; + if (!number(fields[i], 255, degree)) return false; + list.degrees[i] = static_cast(degree); + } + list.length = static_cast(count); + return true; +} + +bool read_pad(char** fields, int count, engine::KitPad& pad) { + if (count != kMaxFields) return false; + int pitch = 0; + int start = 0; + int decay = 0; + int octave = 0; + int send = 0; + const bool sample = std::strcmp(fields[1], "sample") == 0; + if (!sample && std::strcmp(fields[1], "synth") != 0) return false; + if (!copy_word(fields[0], pad.name, sizeof pad.name)) return false; + if (!copy_word(fields[2], pad.source, sizeof pad.source)) return false; + if (!number(fields[3], 24, pitch) || !number(fields[4], 100, start) || !number(fields[5], 100, decay) || + !number(fields[6], 9, octave) || !number(fields[7], engine::kTenthsMax, send)) { + return false; + } + pad.voice = sample ? engine::Voice::sample : engine::Voice::synth; + pad.pitch_semitones = static_cast(pitch); + pad.start = static_cast(start) / 100.0f; // hundredths on the card, a fraction here (D-109) + pad.decay = static_cast(decay) / 100.0f; + pad.octave = static_cast(octave); + pad.send = static_cast(send); + pad.template_count = 0; + return true; +} + +bool read_template(const char* steps, engine::KitPad& pad) { + if (pad.template_count >= engine::kMaxTapTemplates) return false; + engine::TapTemplate& into = pad.templates[pad.template_count]; + int count = 0; + for (const char* c = steps; *c != '\0'; ++c) { + if (count >= engine::kMaxStepsPerTrack) return false; + if (!engine::read_step(*c, into.steps[count++])) return false; + } + if (count == 0) return false; + into.step_count = static_cast(count); + pad.template_count += 1; + return true; +} + +// Every line of the file, in the order kit_builder.py writes them: the progressions +// in mode order, the pads in the order share-format §2 fixes, and each pad's +// templates under it. +bool read_kit(char** lines, int count, engine::Kit& kit) { + if (count < 1 || std::strcmp(lines[0], kKitPrefix) != 0) return false; + int progressions = 0; + int pads = 0; + int dice = 0; + bool have_pluck = false; + bool have_id = false; + char* fields[kMaxFields]; + for (int i = 1; i < count; ++i) { + char* line = lines[i]; + int got = fields_of(line, "id", fields, 1); + if (got == 1) { + if (!copy_word(fields[0], kit.id, sizeof kit.id)) return false; + have_id = true; + continue; + } + int value = 0; + got = fields_of(line, "swing", fields, 1); + if (got == 1) { + if (!number(fields[0], 100, value)) return false; + kit.swing_hundredths = static_cast(value); + continue; + } + got = fields_of(line, "filter", fields, 1); + if (got == 1) { + if (!number(fields[0], engine::kTenthsMax, value)) return false; + kit.filter = static_cast(value); + continue; + } + got = fields_of(line, "fx", fields, 1); + if (got == 1) { + if (!number(fields[0], engine::kTenthsMax, value)) return false; + kit.fx = static_cast(value); + continue; + } + got = fields_of(line, "sidechain", fields, 3); + if (got == 3) { + int on = 0; + int duck = 0; + int release = 0; + if (!number(fields[0], 1, on) || !number(fields[1], 24, duck) || !number(fields[2], 5000, release)) return false; + kit.sidechain = engine::Sidechain{on == 1, static_cast(duck), static_cast(release)}; + continue; + } + got = fields_of(line, "progression", fields, engine::kMaxNoteSequenceLength); + if (got > 0) { + if (progressions >= engine::kModeCount || !read_degrees(fields, got, kit.progressions[progressions])) return false; + progressions += 1; + continue; + } + got = fields_of(line, "pluck", fields, engine::kMaxNoteSequenceLength); + if (got > 0) { + if (have_pluck || !read_degrees(fields, got, kit.pluck_sequence)) return false; + have_pluck = true; + continue; + } + got = fields_of(line, "dice", fields, 1); + if (got == 1) { + if (dice >= engine::kMaxDiceLoops || !copy_word(fields[0], kit.dice_loops[dice], engine::kSectionCodeCapacity)) { + return false; + } + dice += 1; + continue; + } + got = fields_of(line, "pad", fields, kMaxFields); + if (got > 0) { + if (pads >= engine::kTrackCount || !read_pad(fields, got, kit.pads[pads])) return false; + pads += 1; + continue; + } + got = fields_of(line, "template", fields, 1); + if (got == 1) { + if (pads == 0 || !read_template(fields[0], kit.pads[pads - 1])) return false; + continue; + } + return false; // a line this firmware does not know: a kit is not a place to guess + } + kit.dice_loop_count = static_cast(dice); + return have_id && have_pluck && progressions == engine::kModeCount && pads == engine::kTrackCount && dice > 0; +} + +} // namespace + bool load_samples(const engine::Kit& kit, sound::SampleBank& bank) { bank = sound::SampleBank{}; uint32_t capacity = 0; @@ -118,4 +306,24 @@ bool load_samples(const engine::Kit& kit, sound::SampleBank& bank) { return true; } +bool load_kit(const char* id, engine::Kit& kit) { + char path[kPathCapacity]; + std::snprintf(path, sizeof path, "kits/%s/kit.txt", id); + uint32_t size = 0; + if (hal::read_file(path, reinterpret_cast(kit_file_), kKitFileCapacity - 1, &size) != hal::FileRead::ok || + size == 0) { + refuse(path, "is not a kit this device can read"); + return false; + } + kit_file_[size] = '\0'; + char* lines[kMaxKitLines]; + const int count = split_lines(kit_file_, lines, kMaxKitLines); + kit = engine::Kit{}; + if (count < 0 || !read_kit(lines, count, kit)) { + refuse(path, "does not say what a kit is"); + return false; + } + return true; +} + } // namespace io diff --git a/firmware/src/io/kit.h b/firmware/src/io/kit.h index 055b626..f2fa583 100644 --- a/firmware/src/io/kit.h +++ b/firmware/src/io/kit.h @@ -3,11 +3,17 @@ #include "engine/kit.h" #include "sound/voice.h" -// A kit's samples, off the card (PRD §7.5, §12 rule 6). The kit itself is still the -// one compiled in; this is the half that makes it audible on the device, where the -// WAVs are `kits//` on the microSD. +// A kit, off the card (PRD §7.5, §12 rule 6, D-108, D-109). A kit lives in +// `kits//` on the microSD: `kit.txt` says what the pads are and how they behave, +// and the WAVs beside it are the sounds. tools/kit_builder.py writes both from the +// kit.json a kit is authored in. namespace io { +// Reads `kits//kit.txt` into `kit`. False when there is no such kit or the file +// does not say what a kit is, which is logged; `kit` is then unspecified and the +// caller keeps whatever it had. +bool load_kit(const char* id, engine::Kit& kit); + // Reads every sample pad's WAV into the memory hal::sample_memory() gives, and fills // `bank` with what was read. A pad whose file is missing or is not a sample this // firmware can play is left silent and logged, so one bad file costs one sound rather diff --git a/firmware/src/io/lines.cpp b/firmware/src/io/lines.cpp new file mode 100644 index 0000000..5add8b2 --- /dev/null +++ b/firmware/src/io/lines.cpp @@ -0,0 +1,21 @@ +#include "io/lines.h" + +namespace io { + +int split_lines(char* text, char** lines, int capacity) { + int count = 0; + char* start = text; + for (char* c = text;; ++c) { + if (*c != '\n' && *c != '\0') continue; + const bool end = *c == '\0'; + if (end && c == start) break; // the text ended with its last newline + if (count == capacity) return -1; + *c = '\0'; + lines[count++] = start; + start = c + 1; + if (end) break; + } + return count; +} + +} // namespace io diff --git a/firmware/src/io/lines.h b/firmware/src/io/lines.h new file mode 100644 index 0000000..bd1219f --- /dev/null +++ b/firmware/src/io/lines.h @@ -0,0 +1,12 @@ +#pragma once + +// Splitting a text file off the card into its lines. Both the song files and the kit +// files are line-oriented (D-104, D-109), so this lives here rather than twice. +namespace io { + +// NUL-terminates each line of `text` where its newline was, and fills `lines` with +// where each begins. Returns how many, or -1 when there are more than `capacity`. +// The empty piece after a trailing newline is not a line. +int split_lines(char* text, char** lines, int capacity); + +} // namespace io diff --git a/firmware/src/io/store.cpp b/firmware/src/io/store.cpp index 4f14ba6..413ad9a 100644 --- a/firmware/src/io/store.cpp +++ b/firmware/src/io/store.cpp @@ -3,7 +3,9 @@ #include #include +#include "engine/limits.h" #include "hal/hal.h" +#include "io/lines.h" namespace io { @@ -56,24 +58,6 @@ hal::FileRead read_file(const char* path, uint32_t capacity) { return hal::FileRead::ok; } -// Splits file_ into NUL-terminated lines in place. Returns how many, or -1 when -// there are more than `capacity`; the empty piece after a trailing newline is not a line. -int split_lines(char** lines, int capacity) { - int count = 0; - char* start = file_; - for (char* c = file_;; ++c) { - if (*c != kNewline && *c != '\0') continue; - const bool end = *c == '\0'; - if (end && c == start) break; // the file ended with its last newline - if (count == capacity) return -1; - *c = '\0'; - lines[count++] = start; - start = c + 1; - if (end) break; - } - return count; -} - // `AABABBCD`, then an optional `~` and six base36 characters, as a song code ends // (share-format §5). An empty line is a song with no arrangement yet. bool read_arrangement(const char* line, engine::Song& song) { @@ -126,6 +110,11 @@ void read_setting(char* line, Settings& settings) { *separator = '\0'; const char* key = line; const char* text = separator + 1; + if (std::strcmp(key, "kit") == 0) { // a name, not a number, and the only one here + const size_t length = std::strlen(text); + if (length > 0 && length <= engine::kKitIdLength) std::strcpy(settings.kit, text); + return; + } int value = 0; int digits = 0; for (const char* c = text; is_digit(*c); ++c) { @@ -148,7 +137,7 @@ void read_setting(char* line, Settings& settings) { } // namespace bool operator==(const Settings& a, const Settings& b) { - return a.song == b.song && a.brightness == b.brightness && a.sleep_minutes == b.sleep_minutes && + return std::strcmp(a.kit, b.kit) == 0 && a.song == b.song && a.brightness == b.brightness && a.sleep_minutes == b.sleep_minutes && a.midi_clock_in == b.midi_clock_in && a.midi_clock_out == b.midi_clock_out && a.sync_in == b.sync_in && a.sync_out == b.sync_out; } @@ -166,7 +155,7 @@ LoadResult load_song(int slot, const engine::Kit& kit, engine::Song& song) { break; } char* lines[kSongLines]; - if (split_lines(lines, kSongLines) != kSongLines) { + if (split_lines(file_, lines, kSongLines) != kSongLines) { refuse(path, "is not four sections and an arrangement"); return LoadResult::invalid; } @@ -207,17 +196,18 @@ bool load_settings(Settings& settings) { settings = kDefaultSettings; if (read_file("settings.txt", kSettingsFileCapacity) != hal::FileRead::ok) return false; char* lines[kSettingsFileCapacity / 4]; - const int count = split_lines(lines, static_cast(kSettingsFileCapacity / 4)); + const int count = split_lines(file_, lines, static_cast(kSettingsFileCapacity / 4)); if (count < 0) return false; for (int i = 0; i < count; ++i) read_setting(lines[i], settings); return true; } bool save_settings(const Settings& settings) { - const int length = std::snprintf(file_, kSettingsFileCapacity, - "song=%d\nbrightness=%d\nsleep=%d\nmidi-in=%d\nmidi-out=%d\nsync-in=%d\nsync-out=%d\n", - settings.song, settings.brightness, settings.sleep_minutes, settings.midi_clock_in, - settings.midi_clock_out, settings.sync_in, settings.sync_out); + const int length = + std::snprintf(file_, kSettingsFileCapacity, + "kit=%s\nsong=%d\nbrightness=%d\nsleep=%d\nmidi-in=%d\nmidi-out=%d\nsync-in=%d\nsync-out=%d\n", + settings.kit, settings.song, settings.brightness, settings.sleep_minutes, settings.midi_clock_in, + settings.midi_clock_out, settings.sync_in, settings.sync_out); if (length <= 0) return false; return hal::write_file("settings.txt", reinterpret_cast(file_), static_cast(length)); } diff --git a/firmware/src/io/store.h b/firmware/src/io/store.h index 246df32..bef825a 100644 --- a/firmware/src/io/store.h +++ b/firmware/src/io/store.h @@ -19,6 +19,7 @@ namespace io { // swing belong to a section and travel with the song; the master volume starts at // −6 dB every boot (D-087); the settings cursor is where the view was, not a setting. struct Settings { + char kit[engine::kKitIdLength + 1]; // the folder under kits/ the device plays int song; // 1–8, the slot the device comes back to int brightness; // percent int sleep_minutes; // 0 = never @@ -28,7 +29,7 @@ struct Settings { bool sync_out; }; -constexpr Settings kDefaultSettings{1, 100, 10, true, true, true, true}; // §7.7: sleep after 10 minutes +constexpr Settings kDefaultSettings{"lofi", 1, 100, 10, true, true, true, true}; // §7.7: sleep after 10 minutes // What the §9.4 rows accept, here rather than in the input grammar because the card // is the other way into them and both have to agree (D-096, D-104). diff --git a/firmware/src/main.cpp b/firmware/src/main.cpp index 33d7d6d..3c12a3d 100644 --- a/firmware/src/main.cpp +++ b/firmware/src/main.cpp @@ -3,19 +3,14 @@ #include #include "app/app.h" -#include "engine/kits/lofi.h" #include "hal/hal.h" -#include "io/kit.h" -#include "sound/voice.h" void setup() { hal::init(); - // The kit's WAVs come off the card into PSRAM. Whatever is missing — a card, the - // PSRAM, one file — costs those pads their sound and nothing else: io/ says what it - // could not read over the serial log and the synth pads play either way. - sound::SampleBank samples; - io::load_samples(engine::kits::kLofi, samples); - app::init(samples); + // The kit the card names, its WAVs, the songs and the settings: app::init reads them + // all. Whatever is missing — a card, the PSRAM, one file — costs only what depends on + // it, so the instrument comes up either way. + app::init(); } void loop() { diff --git a/host/main.cpp b/host/main.cpp index 1e9278c..453ea88 100644 --- a/host/main.cpp +++ b/host/main.cpp @@ -1,20 +1,15 @@ // Host simulator entry point (PRD §12). The loop is the same as firmware/src/main.cpp; // only the HAL underneath differs. Deliberately includes no SDL header: hal/sdl/ owns SDL. -// The kit's samples come off the simulator's "SD card" through io/, exactly as they do -// on the device: host/CMakeLists.txt seeds that card with the kits the repo ships. +// The kit and its samples come off the simulator's "SD card" through io/, exactly as +// they do on the device: host/CMakeLists.txt seeds that card with the kits the repo ships. #include #include "app/app.h" -#include "engine/kits/lofi.h" #include "hal/hal.h" -#include "io/kit.h" -#include "sound/voice.h" int main() { hal::init(); - sound::SampleBank samples; - io::load_samples(engine::kits::kLofi, samples); - app::init(samples); + app::init(); std::puts("simulator: 1-8 pads; s w k z d e space = split swap skip undo dice show play; a b c shift+d sections;"); std::puts("simulator: up/down or the wheel turn the selected knob, left/right pick it, - = volume; Escape quits"); std::fflush(stdout); diff --git a/spec/kits/lofi/kit.txt b/spec/kits/lofi/kit.txt new file mode 100644 index 0000000..5d807ad --- /dev/null +++ b/spec/kits/lofi/kit.txt @@ -0,0 +1,37 @@ +RTK1 +id=lofi +swing=15 +filter=10 +fx=2 +sidechain=1,5,120 +progression=0,5,2,6 +progression=0,4,5,3 +progression=0,3,6,3 +progression=0,4,1,2 +progression=0,4,1,3 +pluck=0,2,4,7,9,7,4,2 +dice=RT2:lofi:100:10:2:0:15:cm:e10000-e1.0.0-e10000-e1-e100-e10123-e1-e1 +dice=RT2:lofi:100:10:2:0:15:cm:e10-e1.0-e100000-e1.0-e10-e101-e10123-e1 +dice=RT2:lofi:100:10:2:0:15:cm:e1008-e1.0.0-e1000000-b1.0.0-e10000-e10123-e101-e10. +pad=kick,sample,kick.wav,0,0,100,0,1 +template=0 +template=00 +template=000 +template=0000 +pad=snare,sample,snare.wav,0,0,100,0,1 +template=.0 +template=.0.0 +pad=hat,sample,hat.wav,0,0,100,0,1 +template=00 +template=0000 +pad=clap,sample,clap.wav,0,0,100,0,1 +template=.0 +template=.0.0 +pad=bass,synth,sub-saw,0,0,0,2,0 +template=0 +pad=chord,synth,warm-poly,0,0,0,4,4 +template=0 +pad=pluck,synth,keys,0,0,0,5,3 +template=0 +pad=rim,sample,rim.wav,0,0,100,0,1 +template=0 diff --git a/spec/scenarios.md b/spec/scenarios.md index f3bafb7..626a9fd 100644 --- a/spec/scenarios.md +++ b/spec/scenarios.md @@ -1,6 +1,6 @@ # Acceptance scenarios -Every behaviour in the PRD has one row here, and every engine test names the ID it covers (PRD §12 rule 2). T-01–T-24 are PRD §13 unchanged. T-25 onward were added on 2026-09-02 for §6 behaviours that had no scenario, T-85 onward on 2026-09-03 for the §9 views and the lights, T-95 and T-96 the same day for the last two §8.2 gestures, T-97–T-99 for what the card keeps and T-100 for the kit's samples; the PRD section each comes from is in brackets. +Every behaviour in the PRD has one row here, and every engine test names the ID it covers (PRD §12 rule 2). T-01–T-24 are PRD §13 unchanged. T-25 onward were added on 2026-09-02 for §6 behaviours that had no scenario, T-85 onward on 2026-09-03 for the §9 views and the lights, T-95 and T-96 the same day for the last two §8.2 gestures, T-97–T-99 for what the card keeps and T-100 and T-101 for the kit; the PRD section each comes from is in brackets. Conventions: fractions are of one cycle; the default kit (lofi), C minor and 100 bpm are assumed unless stated; "tap ×n" means n taps on that pad starting from empty, following the kit's smart defaults (§6.6); per-track modifier strings are the share-code form from `spec/share-format.md` §3. IDs are never reused: retire a scenario by striking it through, not by deleting it. @@ -106,6 +106,7 @@ Conventions: fractions are of one cycle; the default kit (lofi), C minor and 100 | T-98 | Write the settings, read them back; then a file with an unknown key, a value outside its range, a line that is not `key=value` and a missing row; then no file at all (§9.4, §7.5, D-104) | Every row and the open song come back as they were. The unknown key, the junk line and the out-of-range value are ignored and the rows they name keep what they had; a missing row keeps its default. A value the settings view itself could never set is not one the card gets to introduce: brightness outside 10–100 and a sleep that is not one of 0, 5, 10, 20, 30 or 60 keep their defaults. No file at all is the defaults: song 1, brightness 100, sleep 10, MIDI and sync on. | | T-99 | Kick ×1, then a second of frames, on a card that refuses writes, then a card that accepts (§7.5, §9.6, D-104) | Nothing is saved by hand: the card takes the song a second after the last change, so one tap costs one write, a refused write is tried again a second later and not on every frame, and the loop plays on either way. Once the card takes it, the next boot comes back to that loop. A pick the card cannot carry out is refused rather than half done: a card that will not take the song being left says `song 1 did not save` and the player stays on it with the edit still in hand, and a slot whose file did not parse says `hold to replace song 2` and is left alone until that hold comes (D-107). A boot writes nothing at all until the player plays something: an absent file and an empty song say the same thing. | | T-100 | Boot with the kit's WAVs on the card; then with one missing, one that is not 16-bit 48 kHz mono, one longer than the two seconds a sample may be, and one that is not a WAVE at all; then with no card, and on a board with no PSRAM fitted (§7.5, §12 rule 6, D-081, D-108) | Each sample pad plays the samples in its own file, packed one after another into the PSRAM and none of them overlapping; a pad whose file is missing or is not a sample this firmware can play is silent and the log names the file and what was wrong with it, while every other pad still sounds. With no card every sample pad is silent and the synth pads play on; with no PSRAM there is nowhere to put a sample at all, which is said once rather than per pad. A pad whose sample came off the card is heard: the same tap is loud with it and inaudible without. | +| T-101 | Read `kits/lofi/kit.txt` as tools/kit_builder.py wrote it; then a file with no `RTK1`, one with a line this firmware does not know, one a pad short, one whose filter is 11, and one whose template holds a character no step can be; then boot with the settings naming a kit on the card, and naming one that is not there (§7.5, §12 rule 6, D-109) | The kit read off the card is equal, field for field, to the one compiled into the firmware — which is what keeps the builder's two outputs saying the same thing. Every malformed file is refused whole and logged: a kit is not a place to guess, so an unknown line fails rather than being skipped, unlike the settings. Booting with `kit=` naming a kit on the card plays that kit, and a fresh loop takes its swing, filter and fx from it; naming a kit that is not there logs the path and plays the one built in, so the device always comes up. | ## Watch in testing diff --git a/tests/app_support.h b/tests/app_support.h index bfd3d1c..7b78c06 100644 --- a/tests/app_support.h +++ b/tests/app_support.h @@ -43,21 +43,20 @@ struct World { uint64_t timer_allocations = 0; float last_peak = 0.0f; // the loudest sample of the last block rendered - // The tutorial has run unless a test asks for a first boot (§8.5, T-22). - explicit World(bool first_run = false) { - const sound::SampleBank silent{}; - start(first_run, silent); - } - - // A world whose sample pads have sounds, for the tests that need to hear one. The - // bank is read off the card before the world starts; hal_fake::reset() clears the - // card's files but not the memory the samples were read into, so it stays valid. - explicit World(const sound::SampleBank& samples) { start(false, samples); } - - void start(bool first_run, const sound::SampleBank& samples) { - hal_fake::reset(); + // The tutorial has run unless a test asks for a first boot (§8.5, T-22). The card + // starts empty, so the app comes up on the kit compiled in with silent sample pads; + // a test that wants sounds puts them on the card and starts a world with `keep_card`. + explicit World(bool first_run = false) { start(first_run, true); } + + // A world that keeps whatever is already on the fake card, for the tests that put a + // kit or its samples there first (T-100, T-101). + struct OnThisCard {}; + explicit World(OnThisCard) { start(false, false); } + + void start(bool first_run, bool clear_card) { + if (clear_card) hal_fake::reset(); if (!first_run) hal::write_file(app::kTutorialDoneFile, &app::kTutorialRan, 1); - app::init(samples); + app::init(); timer_frames = static_cast(hal_fake::timer_period_us()) * sound::kSampleRate / 1000000; REQUIRE(timer_frames > 0); // a period under 21 us would never advance the world REQUIRE(hal_fake::audio_callback() != nullptr); @@ -67,8 +66,7 @@ struct World { // A power cycle: the app starts again on the same card, which is where anything // that outlives one has to be (T-56, T-92, T-99). void reboot() { - const sound::SampleBank silent{}; - app::init(silent); + app::init(); fired.clear(); seen = 0; origin = 0; diff --git a/tests/io_test.cpp b/tests/io_test.cpp index 7a3e497..7c69b10 100644 --- a/tests/io_test.cpp +++ b/tests/io_test.cpp @@ -1,7 +1,9 @@ -// io/ (spec/scenarios.md T-56, T-59, T-89, T-97, T-98, T-99, T-100): the song and settings +// io/ (spec/scenarios.md T-56, T-59, T-89, T-97, T-98, T-99, T-100, T-101): the song and settings // files the card holds, the app keeping them as the player plays, and the id a // shared loop carries. #include +#include +#include #include #include @@ -33,6 +35,14 @@ void put(const char* path, const std::string& text) { REQUIRE(hal::write_file(path, reinterpret_cast(text.data()), static_cast(text.size()))); } +// A kit's own files as tools/kit_builder.py wrote them, for the tests that compare +// what the tool writes with what the firmware reads. ROTA_KITS_DIR is where the host +// build says spec/kits/ is. +std::string kit_file(const std::string& relative) { + std::ifstream file(std::string(ROTA_KITS_DIR) + "/" + relative, std::ios::binary); + return std::string((std::istreambuf_iterator(file)), std::istreambuf_iterator()); +} + std::vector lines_of(const std::string& text) { std::vector lines; std::string line; @@ -135,14 +145,15 @@ TEST_CASE("T-97 A song's own lineage survives the model, not only the file") { TEST_CASE("T-98 The settings file keeps the rows and the open song, and ignores what it cannot read") { hal_fake::reset(); - const io::Settings written{5, 40, 0, false, true, false, true}; + const io::Settings written{"jazz", 5, 40, 0, false, true, false, true}; REQUIRE(io::save_settings(written)); io::Settings back{}; REQUIRE(io::load_settings(back)); CHECK(back == written); - put("settings.txt", "song=9\nbrightness=40\nnonsense\ncolour=blue\nsleep=30\nmidi-in=7\n"); + put("settings.txt", "kit=jazz\nsong=9\nbrightness=40\nnonsense\ncolour=blue\nsleep=30\nmidi-in=7\n"); REQUIRE(io::load_settings(back)); + CHECK(std::string(back.kit) == "jazz"); // a name, not a number CHECK(back.song == io::kDefaultSettings.song); // 9 is not a slot CHECK(back.brightness == 40); CHECK(back.sleep_minutes == 30); @@ -627,11 +638,7 @@ TEST_CASE("T-100 A sample read off the card is what the pad plays") { hal_fake::reset(); put("kits/lofi/kick.wav", loud_wave(4800)); // a tenth of a second of it - sound::SampleBank bank; - REQUIRE(io::load_samples(kit(), bank)); - REQUIRE(bank.samples[0].frames != nullptr); - - World w(bank); + World w{World::OnThisCard{}}; // the app reads the card itself, as the device does w.tap(Pad::kick); w.run_for(kSecond / 10); CHECK(w.last_peak > 0.05f); // the card's own samples reached the output @@ -641,3 +648,66 @@ TEST_CASE("T-100 A sample read off the card is what the pad plays") { silent.run_for(kSecond / 10); CHECK(silent.last_peak < 1e-6f); // not exactly zero: the effects chain has its own tail } + +TEST_CASE("T-101 The kit on the card is the kit compiled in, and a file that is not a kit does not load") { + hal_fake::reset(); + const std::string text = kit_file("lofi/kit.txt"); + REQUIRE_FALSE(text.empty()); + put("kits/lofi/kit.txt", text); + + engine::Kit kit{}; + REQUIRE(io::load_kit("lofi", kit)); + CHECK(kit == engine::kits::kLofi); // what kit_builder.py writes twice says the same thing twice + + CHECK_FALSE(io::load_kit("jazz", kit)); // no such kit on the card + + const std::vector lines = lines_of(text); + SUBCASE("a file that does not say it is a kit") { + put("kits/lofi/kit.txt", text.substr(text.find('\n') + 1)); + CHECK_FALSE(io::load_kit("lofi", kit)); + CHECK(logged("kits/lofi/kit.txt")); + } + SUBCASE("a line this firmware does not know") { + put("kits/lofi/kit.txt", text + "colour=blue\n"); + CHECK_FALSE(io::load_kit("lofi", kit)); // a kit is not a place to guess + } + SUBCASE("a pad missing") { + std::string without; + for (const std::string& line : lines) { + if (line.rfind("pad=rim", 0) != 0) without += line + "\n"; + } + put("kits/lofi/kit.txt", without); + CHECK_FALSE(io::load_kit("lofi", kit)); + } + SUBCASE("a value outside its range") { + std::string bad; + for (const std::string& line : lines) bad += (line == "filter=10" ? "filter=11" : line) + "\n"; + put("kits/lofi/kit.txt", bad); + CHECK_FALSE(io::load_kit("lofi", kit)); + } + SUBCASE("a step character no share code could hold") { + std::string bad; + for (const std::string& line : lines) bad += (line == "template=.0.0" ? "template=.!.0" : line) + "\n"; + put("kits/lofi/kit.txt", bad); + CHECK_FALSE(io::load_kit("lofi", kit)); + } +} + +TEST_CASE("T-101 The device plays the kit its card names, and the one built in when it cannot") { + hal_fake::reset(); + std::string text = kit_file("lofi/kit.txt"); + const std::string quieter = "swing=40"; + text.replace(text.find("swing=15"), 8, quieter); // a kit that is plainly not the built-in one + put("kits/jazz/kit.txt", text); + put("settings.txt", "kit=jazz\n"); + + World w{World::OnThisCard{}}; + CHECK(app::kit().swing_hundredths == 40); // the card's kit, not the compiled one + CHECK(w.state(0).swing == 40); // and a fresh loop takes its swing from it + + hal_fake::reset(); + put("settings.txt", "kit=nosuch\n"); + World fallback{World::OnThisCard{}}; + CHECK(app::kit() == engine::kits::kLofi); // no such kit on the card: the device still plays + CHECK(logged("kits/nosuch/kit.txt")); +} diff --git a/tests/ui_test.cpp b/tests/ui_test.cpp index 16e1ce1..a21baf6 100644 --- a/tests/ui_test.cpp +++ b/tests/ui_test.cpp @@ -829,8 +829,7 @@ TEST_CASE("T-22 Play skips the tutorial, the next boot does not run it, and step w.frame(); CHECK_FALSE(has_text(screen(), "tap the kick")); - const sound::SampleBank silent{}; - app::init(silent); // the next boot reads the flag off the card + app::init(); // the next boot reads the flag off the card CHECK_FALSE(app::model().tutorial.active); } diff --git a/tools/kit_builder.py b/tools/kit_builder.py index 56425f2..dba8ded 100755 --- a/tools/kit_builder.py +++ b/tools/kit_builder.py @@ -154,6 +154,29 @@ def cpp_float(value): return f"{float(value)!r}f" # always carries a decimal point: 0.0f, not 0f +BASE36 = "0123456789abcdefghijklmnopqrstuvwxyz" + + +def step_character(step): + """A step in the spelling share codes use (spec/share-format.md §2): `.` for a + rest, otherwise base36 of (hits − 1) × 8 + note. The device reads a kit's + templates with the same table, so there is one spelling for a step in this repo.""" + hits, note = step + return "." if hits == 0 else BASE36[(hits - 1) * 8 + note] + + +def hundredths(value, what): + """A fraction of a sample, 0.00–1.00. Whole hundredths only, so the kit file + carries an integer and the device needs no float parser (D-109).""" + number = finite_number(value, what) + if not 0.0 <= number <= 1.0: + raise KitError(f"{what} is 0.0 to 1.0, got {number}") + scaled = round(number * 100) + if abs(number * 100 - scaled) > 1e-9: + raise KitError(f"{what} must be a whole hundredth, got {number}") + return scaled + + def cpp_degree_list(values): return "{%d, {%s}}" % (len(values), ", ".join(str(v) for v in values)) @@ -255,6 +278,55 @@ def build_header(kit, source_path, output_path): """ +def build_kit_text(kit, kit_dir): + """The kit as the device reads it off the card (D-109): one line a field, the + pads in the order the share format fixes, each pad's templates under it. Text, + because §7.6 shows the card over USB and §12 rule 6 wants a format anyone can + write a kit in.""" + lines = [KIT_FILE_PREFIX, f"id={kit['id']}"] + lines.append(f"swing={hundredths(kit['swing'], 'swing')}") + lines.append(f"filter={tenths(kit['filter'], 'filter')}") + lines.append(f"fx={tenths(kit['fx'], 'fx')}") + sidechain = kit["sidechain"] + lines.append( + "sidechain=%d,%d,%d" + % ( + 1 if exact_bool(sidechain["on"], "sidechain on") else 0, + exact_int(sidechain["duck_db"], "sidechain duck_db"), + exact_int(sidechain["release_ms"], "sidechain release_ms"), + ) + ) + for mode in MODE_ORDER: + degrees = degree_list(kit["progressions"][mode], f"{mode} progression") + lines.append("progression=" + ",".join(str(d) for d in degrees)) + lines.append("pluck=" + ",".join(str(d) for d in degree_list(kit["pluck_sequence"], "pluck sequence"))) + for code in kit.get("dice_loops", []): + lines.append(f"dice={code}") + for pad in kit["pads"]: + what = f"pad {pad.get('name')!r}" + sample = pad["voice"] == "sample" + lines.append( + "pad=%s,%s,%s,%d,%d,%d,%d,%d" + % ( + pad["name"], + pad["voice"], + pad["source"] if sample else pad["preset"], + exact_int(pad.get("pitch", 0), f"{what} pitch") if sample else 0, + hundredths(pad.get("start", 0), f"{what} start") if sample else 0, + hundredths(pad.get("decay", 1.0), f"{what} decay") if sample else 0, + 0 if sample else exact_int(pad["octave"], f"{what} octave"), + tenths(pad["send"], f"{what} send"), + ) + ) + for template in pad.get("templates", []): + lines.append("template=" + "".join(step_character(s) for s in template_steps(template, what))) + return "\n".join(lines) + "\n" + + +KIT_FILE_PREFIX = "RTK1" +KIT_FILE_NAME = "kit.txt" + + def main(argv): if len(argv) != 3: print("usage: kit_builder.py spec/kits//kit.json firmware/src/engine/kits/.h", file=sys.stderr) @@ -264,12 +336,18 @@ def main(argv): with open(source_path, encoding="utf-8") as source: kit = json.load(source) header = build_header(kit, source_path, output_path) + kit_text = build_kit_text(kit, os.path.dirname(source_path)) except (KitError, KeyError, ValueError, OSError) as error: print(f"kit_builder: {source_path}: {error}", file=sys.stderr) return EXIT_INVALID_KIT with open(output_path, "w", encoding="utf-8", newline="\n") as output: output.write(header) - print(f"kit_builder: wrote {output_path}") + # The card's copy goes beside the json it came from, so the folder a kit lives in + # is the folder the device reads. + kit_file = os.path.join(os.path.dirname(source_path), KIT_FILE_NAME) + with open(kit_file, "w", encoding="utf-8", newline="\n") as output: + output.write(kit_text) + print(f"kit_builder: wrote {output_path} and {kit_file}") return EXIT_OK From e4bfcd399d56918b6e4384e340d906db72765741 Mon Sep 17 00:00:00 2001 From: Deva Date: Thu, 3 Sep 2026 17:25:18 +0530 Subject: [PATCH 3/5] Answer the review: nothing a card holds becomes a path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Five findings, all real, and the security one is the one that mattered. A kit id and a pad's source were pasted straight into a file path. A card saying `kit=../../elsewhere`, or a pad naming `../kick.wav`, sent the firmware looking outside the kit's folder — and CLAUDE.md says never build a file path from strings. Both are checked against the grammar now: an id is share-format §2's own `[a-z0-9]{1,12}`, a source is a plain file name, and neither may begin with a dot. A kit could also call itself something other than the folder it was found in. Nothing noticed, but its samples are looked for by its own id, so the kit would have come from one folder and its sounds from another. Refused. My own fixture had exactly that shape — `id=lofi` inside `kits/jazz/` — so the test that was meant to prove a card kit plays was passing for the wrong reason, which is how the review found it. A file could also leave out swing, filter, fx or sidechain and still load, because those have no count to check and the zeroed Kit left a zero behind. Every field is required now, and a test drops each in turn — including the one that showed the test's own assumption was wrong, since lofi has three dice loops and dropping one leaves a kit that is still valid. Co-Authored-By: Claude Opus 5 --- DECISIONS.md | 2 +- firmware/src/io/kit.cpp | 51 ++++++++++++++++++++++++++++++++--- firmware/src/io/kit.h | 8 +++++- firmware/src/io/store.cpp | 4 +-- spec/scenarios.md | 2 +- tests/io_test.cpp | 56 +++++++++++++++++++++++++++++++++++++-- 6 files changed, 112 insertions(+), 11 deletions(-) diff --git a/DECISIONS.md b/DECISIONS.md index 947c95e..10cd90f 100644 --- a/DECISIONS.md +++ b/DECISIONS.md @@ -112,4 +112,4 @@ What was decided, why, and when to look again. One row per decision. IDs are seq | D-106 | The share-code size cap (T-62) lives in `engine::decode` and `decode_song`, not at each path the bytes arrive on: 512 characters for a section and 2048 for a song, NUL not counted, scanning at most one past the limit so a missing terminator costs a bounded read. | 2026-09-03. The engine's decoder is linear and allocates nothing, so the cap is about refusing a code rather than about safety; putting it where the knowledge is means the card, `tools/render`, and the SysEx, USB and paste-box paths still to come are all covered without each remembering to. Twice the canonical worst case (238 and 988) rather than the buffer sizes, because T-16 says a decoder skips the fields a later version adds and a cap at the buffer would leave almost no room for them. Rejected: a cap at each arriving path (three places to forget, and nothing to write today since none of those paths exists yet). | If a later version's fields need more than the canonical code's length again. | | D-107 | A song slot whose file will not parse (§9.6, T-97): a tap refuses it and says `hold to replace song 2`; a hold on that pad copies the song on screen over it and says `song 2 replaced`. The hold is live only on a slot the device already knows it cannot read, so no hold can destroy a song the player could still open, and a hold anywhere else in the song view still does nothing. | 2026-09-03. D-104 made a file that will not parse a slot rather than a gap, which stopped a pick from silently copying over somebody's song but left the slot unreachable: the only thing that replaced it was the player already being on it, so recovery meant a computer or a factory reset — a dead pad on an instrument that ships with no manual. Press-then-hold is already this device's idiom for a destructive thing, in `hold dice to clear` and `hold play to reset`, and §9.6 said the pads' hold gestures were inactive in the song view, so the gesture was free and consistent rather than new; PRD §9.6 gains the exception. Rejected: a second tap inside the arming timeout (a double tap is easy by accident, and every other confirmation here is a hold); quarantining the file at boot (needs a HAL rename or delete, and silently moves a player's file aside); a settings row to clear a slot (a sub-menu for a rare failure, against §9.6's one screen). | If usability round 1 shows nobody finds the hold, or if a fourth tile state would say it better than the status line. | | D-108 | A kit's samples come off the card, not the firmware image: `io::load_samples` reads `kits//` into the PSRAM `hal::sample_memory()` hands it — 1.5 MB, the eight pads of two seconds D-081 allows each — and fills a `sound::SampleBank` with what it found. Each WAV is read straight into the room left in that memory and parsed where it lands, then moved down over its own header, so nothing is ever staged twice. A file that is missing or is not 16-bit 48 kHz mono PCM costs its own pad its sound and no more. The simulator does the same thing with the same code: `host/CMakeLists.txt` seeds `out/sdcard/kits/` from `spec/kits/` at configure time. | 2026-09-03. §7.5 puts the samples in PSRAM and the kits on the card, and until now the device passed `app::init` an empty bank, so the drum pads were silent on hardware and the simulator used the render tool's host-only loader — two paths, one of them not the product's. Reading in place rather than through a staging buffer because a sample is up to 192 KB and the device has 63 KB of ordinary RAM free; there is nowhere to put a copy. `hal::sample_memory()` answers with nothing when no PSRAM is fitted, which is every board until bring-up, because writing to a section that is not backed would fault rather than fail. Rejected: samples in the firmware image (a kit could never be changed without a rebuild, which is what §12 rule 6 is against); streaming from the card at play time (file I/O in the audio path, forbidden by §12 rule 4). | At bring-up, when a real PSRAM chip says whether reads from it keep up with sixteen voices; or if a kit needs more than eight samples of two seconds. | -| D-109 | A kit is readable text on the card: `tools/kit_builder.py` writes `kits//kit.txt` beside the header it already generates, both from the one `kit.json`, and `io::load_kit` reads it. One line a field, the five progressions in mode order, the eight pads in the order share-format §2 fixes with each pad's tap templates under it, and a step spelled exactly as a share code spells one — `engine::read_step` is now the only implementation of that table. Fractions are whole hundredths, so the device needs no float parser. `engine::Kit` holds its strings in fixed arrays rather than pointers, so a card kit and a compiled kit are one type, and `settings.txt`'s `kit=` line says which folder to play, falling back to the built-in kit when the card has no such kit. | 2026-09-03. §12 rule 6 asks for an open format so the community can make kits, and a kit that only exists as a C++ header can only be made by rebuilding the firmware. Text because §7.6 shows the card over USB and because the songs and settings are already text (D-104): one habit, not three. Rejected: parsing `kit.json` on the device (a JSON parser is a few hundred lines of firmware at a boundary that reads whatever a card holds, against the rule about reaching for the standard library first) and a packed binary (smaller and faster, but then only the tool can make a kit, which is the opposite of open). An unknown line fails the whole file rather than being skipped as an unknown settings row is: a setting the device ignores costs one row, a kit field it ignores would be an instrument quietly playing something other than what the kit says. The test that the card kit equals the compiled kit is what holds the builder's two outputs together, and CI diffs both. | If a kit needs a field that is not a number, a name or a share code; or when kits can be chosen from the settings view, which needs the samples reloaded rather than just the file re-read. | +| D-109 | A kit is readable text on the card: `tools/kit_builder.py` writes `kits//kit.txt` beside the header it already generates, both from the one `kit.json`, and `io::load_kit` reads it. One line a field, the five progressions in mode order, the eight pads in the order share-format §2 fixes with each pad's tap templates under it, and a step spelled exactly as a share code spells one — `engine::read_step` is now the only implementation of that table. Fractions are whole hundredths, so the device needs no float parser. `engine::Kit` holds its strings in fixed arrays rather than pointers, so a card kit and a compiled kit are one type, and `settings.txt`'s `kit=` line says which folder to play, falling back to the built-in kit when the card has no such kit. | 2026-09-03. §12 rule 6 asks for an open format so the community can make kits, and a kit that only exists as a C++ header can only be made by rebuilding the firmware. Text because §7.6 shows the card over USB and because the songs and settings are already text (D-104): one habit, not three. Rejected: parsing `kit.json` on the device (a JSON parser is a few hundred lines of firmware at a boundary that reads whatever a card holds, against the rule about reaching for the standard library first) and a packed binary (smaller and faster, but then only the tool can make a kit, which is the opposite of open). An unknown line fails the whole file rather than being skipped as an unknown settings row is: a setting the device ignores costs one row, a kit field it ignores would be an instrument quietly playing something other than what the kit says, and a field left out entirely fails for the same reason — the zero it would leave behind is not this kit. Every card string that reaches a path is checked against the grammar rather than trusted: a kit id is share-format §2's `[a-z0-9]{1,12}`, a pad's source is a plain file name, and a kit whose id is not the folder it was found in is refused, because its samples are looked for by its own id and would be hunted for somewhere else. The test that the card kit equals the compiled kit is what holds the builder's two outputs together, and CI diffs both. | If a kit needs a field that is not a number, a name or a share code; or when kits can be chosen from the settings view, which needs the samples reloaded rather than just the file re-read. | diff --git a/firmware/src/io/kit.cpp b/firmware/src/io/kit.cpp index ed5d624..59d1af8 100644 --- a/firmware/src/io/kit.cpp +++ b/firmware/src/io/kit.cpp @@ -19,6 +19,7 @@ constexpr uint32_t kKitFileCapacity = 4096; constexpr int kMaxKitLines = 96; constexpr int kMaxFields = 8; // a pad line, the longest constexpr const char* kKitPrefix = "RTK1"; +constexpr const char* kKitFileName = "kit.txt"; char kit_file_[kKitFileCapacity]; constexpr uint32_t kRiffHeaderBytes = 12; constexpr uint32_t kChunkHeaderBytes = 8; @@ -127,6 +128,17 @@ bool number(const char* text, int most, int& out) { return digits > 0; } +// A sample's file name, and nothing that could name a file outside the kit's folder: +// letters, digits, and the three punctuation marks a file name needs. +bool is_file_name(const char* text) { + if (*text == '\0' || *text == '.') return false; // no hidden files, and no `..` + for (const char* c = text; *c != '\0'; ++c) { + const bool ordinary = (*c >= 'a' && *c <= 'z') || (*c >= '0' && *c <= '9') || *c == '.' || *c == '-' || *c == '_'; + if (!ordinary) return false; + } + return true; +} + bool copy_word(const char* from, char* into, int capacity) { const size_t length = std::strlen(from); if (length == 0 || length >= static_cast(capacity)) return false; @@ -155,7 +167,8 @@ bool read_pad(char** fields, int count, engine::KitPad& pad) { const bool sample = std::strcmp(fields[1], "sample") == 0; if (!sample && std::strcmp(fields[1], "synth") != 0) return false; if (!copy_word(fields[0], pad.name, sizeof pad.name)) return false; - if (!copy_word(fields[2], pad.source, sizeof pad.source)) return false; + // The source becomes half of a path, so it is a file name or it is nothing. + if (!is_file_name(fields[2]) || !copy_word(fields[2], pad.source, sizeof pad.source)) return false; if (!number(fields[3], 24, pitch) || !number(fields[4], 100, start) || !number(fields[5], 100, decay) || !number(fields[6], 9, octave) || !number(fields[7], engine::kTenthsMax, send)) { return false; @@ -194,12 +207,16 @@ bool read_kit(char** lines, int count, engine::Kit& kit) { int dice = 0; bool have_pluck = false; bool have_id = false; + bool have_swing = false; + bool have_filter = false; + bool have_fx = false; + bool have_sidechain = false; char* fields[kMaxFields]; for (int i = 1; i < count; ++i) { char* line = lines[i]; int got = fields_of(line, "id", fields, 1); if (got == 1) { - if (!copy_word(fields[0], kit.id, sizeof kit.id)) return false; + if (!is_kit_id(fields[0]) || !copy_word(fields[0], kit.id, sizeof kit.id)) return false; have_id = true; continue; } @@ -208,18 +225,21 @@ bool read_kit(char** lines, int count, engine::Kit& kit) { if (got == 1) { if (!number(fields[0], 100, value)) return false; kit.swing_hundredths = static_cast(value); + have_swing = true; continue; } got = fields_of(line, "filter", fields, 1); if (got == 1) { if (!number(fields[0], engine::kTenthsMax, value)) return false; kit.filter = static_cast(value); + have_filter = true; continue; } got = fields_of(line, "fx", fields, 1); if (got == 1) { if (!number(fields[0], engine::kTenthsMax, value)) return false; kit.fx = static_cast(value); + have_fx = true; continue; } got = fields_of(line, "sidechain", fields, 3); @@ -229,6 +249,7 @@ bool read_kit(char** lines, int count, engine::Kit& kit) { int release = 0; if (!number(fields[0], 1, on) || !number(fields[1], 24, duck) || !number(fields[2], 5000, release)) return false; kit.sidechain = engine::Sidechain{on == 1, static_cast(duck), static_cast(release)}; + have_sidechain = true; continue; } got = fields_of(line, "progression", fields, engine::kMaxNoteSequenceLength); @@ -265,7 +286,10 @@ bool read_kit(char** lines, int count, engine::Kit& kit) { return false; // a line this firmware does not know: a kit is not a place to guess } kit.dice_loop_count = static_cast(dice); - return have_id && have_pluck && progressions == engine::kModeCount && pads == engine::kTrackCount && dice > 0; + // Every field, not just the ones with a count: an absent line would otherwise leave + // the zero `engine::Kit{}` put there, and a kit with no swing is not this kit. + return have_id && have_pluck && have_swing && have_filter && have_fx && have_sidechain && + progressions == engine::kModeCount && pads == engine::kTrackCount && dice > 0; } } // namespace @@ -306,9 +330,22 @@ bool load_samples(const engine::Kit& kit, sound::SampleBank& bank) { return true; } +bool is_kit_id(const char* text) { + const size_t length = std::strlen(text); + if (length == 0 || length > engine::kKitIdLength) return false; + for (const char* c = text; *c != '\0'; ++c) { + if (!((*c >= 'a' && *c <= 'z') || (*c >= '0' && *c <= '9'))) return false; + } + return true; +} + bool load_kit(const char* id, engine::Kit& kit) { + if (!is_kit_id(id)) { + hal::log("io: that is not a kit id, so no kit was looked for"); + return false; + } char path[kPathCapacity]; - std::snprintf(path, sizeof path, "kits/%s/kit.txt", id); + std::snprintf(path, sizeof path, "kits/%s/%s", id, kKitFileName); uint32_t size = 0; if (hal::read_file(path, reinterpret_cast(kit_file_), kKitFileCapacity - 1, &size) != hal::FileRead::ok || size == 0) { @@ -323,6 +360,12 @@ bool load_kit(const char* id, engine::Kit& kit) { refuse(path, "does not say what a kit is"); return false; } + // Its samples are looked for by its own id, so a kit that calls itself something + // else than the folder it sits in would send us hunting in another folder. + if (std::strcmp(kit.id, id) != 0) { + refuse(path, "calls itself a different kit than the folder it is in"); + return false; + } return true; } diff --git a/firmware/src/io/kit.h b/firmware/src/io/kit.h index f2fa583..6865cd6 100644 --- a/firmware/src/io/kit.h +++ b/firmware/src/io/kit.h @@ -9,9 +9,15 @@ // kit.json a kit is authored in. namespace io { +// A kit id as share-format §2 spells one: 1–12 of `a`–`z` and `0`–`9`. Everything that +// reaches a path comes off a card, so nothing that could climb out of `kits/` is a name +// this firmware will use (D-109). +bool is_kit_id(const char* text); + // Reads `kits//kit.txt` into `kit`. False when there is no such kit or the file // does not say what a kit is, which is logged; `kit` is then unspecified and the -// caller keeps whatever it had. +// caller keeps whatever it had. The file's own `id` must be the folder it was found +// in, or the kit and its samples would be looked for in two different places. bool load_kit(const char* id, engine::Kit& kit); // Reads every sample pad's WAV into the memory hal::sample_memory() gives, and fills diff --git a/firmware/src/io/store.cpp b/firmware/src/io/store.cpp index 413ad9a..5f68c86 100644 --- a/firmware/src/io/store.cpp +++ b/firmware/src/io/store.cpp @@ -5,6 +5,7 @@ #include "engine/limits.h" #include "hal/hal.h" +#include "io/kit.h" #include "io/lines.h" namespace io { @@ -111,8 +112,7 @@ void read_setting(char* line, Settings& settings) { const char* key = line; const char* text = separator + 1; if (std::strcmp(key, "kit") == 0) { // a name, not a number, and the only one here - const size_t length = std::strlen(text); - if (length > 0 && length <= engine::kKitIdLength) std::strcpy(settings.kit, text); + if (is_kit_id(text)) std::strcpy(settings.kit, text); // and a name that cannot be a path return; } int value = 0; diff --git a/spec/scenarios.md b/spec/scenarios.md index 626a9fd..6a03876 100644 --- a/spec/scenarios.md +++ b/spec/scenarios.md @@ -106,7 +106,7 @@ Conventions: fractions are of one cycle; the default kit (lofi), C minor and 100 | T-98 | Write the settings, read them back; then a file with an unknown key, a value outside its range, a line that is not `key=value` and a missing row; then no file at all (§9.4, §7.5, D-104) | Every row and the open song come back as they were. The unknown key, the junk line and the out-of-range value are ignored and the rows they name keep what they had; a missing row keeps its default. A value the settings view itself could never set is not one the card gets to introduce: brightness outside 10–100 and a sleep that is not one of 0, 5, 10, 20, 30 or 60 keep their defaults. No file at all is the defaults: song 1, brightness 100, sleep 10, MIDI and sync on. | | T-99 | Kick ×1, then a second of frames, on a card that refuses writes, then a card that accepts (§7.5, §9.6, D-104) | Nothing is saved by hand: the card takes the song a second after the last change, so one tap costs one write, a refused write is tried again a second later and not on every frame, and the loop plays on either way. Once the card takes it, the next boot comes back to that loop. A pick the card cannot carry out is refused rather than half done: a card that will not take the song being left says `song 1 did not save` and the player stays on it with the edit still in hand, and a slot whose file did not parse says `hold to replace song 2` and is left alone until that hold comes (D-107). A boot writes nothing at all until the player plays something: an absent file and an empty song say the same thing. | | T-100 | Boot with the kit's WAVs on the card; then with one missing, one that is not 16-bit 48 kHz mono, one longer than the two seconds a sample may be, and one that is not a WAVE at all; then with no card, and on a board with no PSRAM fitted (§7.5, §12 rule 6, D-081, D-108) | Each sample pad plays the samples in its own file, packed one after another into the PSRAM and none of them overlapping; a pad whose file is missing or is not a sample this firmware can play is silent and the log names the file and what was wrong with it, while every other pad still sounds. With no card every sample pad is silent and the synth pads play on; with no PSRAM there is nowhere to put a sample at all, which is said once rather than per pad. A pad whose sample came off the card is heard: the same tap is loud with it and inaudible without. | -| T-101 | Read `kits/lofi/kit.txt` as tools/kit_builder.py wrote it; then a file with no `RTK1`, one with a line this firmware does not know, one a pad short, one whose filter is 11, and one whose template holds a character no step can be; then boot with the settings naming a kit on the card, and naming one that is not there (§7.5, §12 rule 6, D-109) | The kit read off the card is equal, field for field, to the one compiled into the firmware — which is what keeps the builder's two outputs saying the same thing. Every malformed file is refused whole and logged: a kit is not a place to guess, so an unknown line fails rather than being skipped, unlike the settings. Booting with `kit=` naming a kit on the card plays that kit, and a fresh loop takes its swing, filter and fx from it; naming a kit that is not there logs the path and plays the one built in, so the device always comes up. | +| T-101 | Read `kits/lofi/kit.txt` as tools/kit_builder.py wrote it; then a file with no `RTK1`, one with a line this firmware does not know, one a pad short, one whose filter is 11, and one whose template holds a character no step can be; then a kit whose id is not the folder it sits in, one whose pad names a source outside its folder, and a settings file whose `kit=` could climb out of `kits/`; then boot with the settings naming a kit on the card, and naming one that is not there (§7.5, §12 rule 6, D-109) | The kit read off the card is equal, field for field, to the one compiled into the firmware — which is what keeps the builder's two outputs saying the same thing. Every malformed file is refused whole and logged: a kit is not a place to guess, so an unknown line fails rather than being skipped, unlike the settings, and a file missing any field it should have — including the ones with no count, whose absence would otherwise leave a zero behind — does not load. Nothing a card holds becomes a path: a kit id is 1–12 of `a`–`z` and `0`–`9` as share-format §2 spells one, a pad's source is a plain file name, and a kit whose id is not the folder it was found in is refused, since its samples would be hunted for somewhere else. Booting with `kit=` naming a kit on the card plays that kit, and a fresh loop takes its swing, filter and fx from it; naming a kit that is not there logs the path and plays the one built in, so the device always comes up. | ## Watch in testing diff --git a/tests/io_test.cpp b/tests/io_test.cpp index 7c69b10..3f9eaaf 100644 --- a/tests/io_test.cpp +++ b/tests/io_test.cpp @@ -696,12 +696,13 @@ TEST_CASE("T-101 The kit on the card is the kit compiled in, and a file that is TEST_CASE("T-101 The device plays the kit its card names, and the one built in when it cannot") { hal_fake::reset(); std::string text = kit_file("lofi/kit.txt"); - const std::string quieter = "swing=40"; - text.replace(text.find("swing=15"), 8, quieter); // a kit that is plainly not the built-in one + text.replace(text.find("id=lofi"), 7, "id=jazz"); // a kit is named by the folder it lives in + text.replace(text.find("swing=15"), 8, "swing=40"); // and this one is plainly not the built-in kit put("kits/jazz/kit.txt", text); put("settings.txt", "kit=jazz\n"); World w{World::OnThisCard{}}; + CHECK(std::string(app::kit().id) == "jazz"); CHECK(app::kit().swing_hundredths == 40); // the card's kit, not the compiled one CHECK(w.state(0).swing == 40); // and a fresh loop takes its swing from it @@ -711,3 +712,54 @@ TEST_CASE("T-101 The device plays the kit its card names, and the one built in w CHECK(app::kit() == engine::kits::kLofi); // no such kit on the card: the device still plays CHECK(logged("kits/nosuch/kit.txt")); } + +TEST_CASE("T-101 Nothing a card holds becomes a path out of the kit it names") { + hal_fake::reset(); + engine::Kit kit{}; + for (const char* name : {"../../etc", "lofi/../..", "LOFI", "", "waytoolongakitid"}) { + CAPTURE(name); + CHECK_FALSE(io::load_kit(name, kit)); // not a kit id, so no path is built from it at all + } + + // The settings keep the kit they had rather than take one that could climb out. + put("settings.txt", "kit=../../elsewhere\n"); + io::Settings settings{}; + REQUIRE(io::load_settings(settings)); + CHECK(std::string(settings.kit) == std::string(io::kDefaultSettings.kit)); + + // A pad's source is the other half of a path, so it is a file name or the kit is refused. + std::string text = kit_file("lofi/kit.txt"); + text.replace(text.find("kick.wav"), 8, "../kick.wav"); + put("kits/lofi/kit.txt", text); + CHECK_FALSE(io::load_kit("lofi", kit)); + + // And a kit that calls itself something other than its folder is refused, since its + // samples would then be looked for somewhere else entirely. + text = kit_file("lofi/kit.txt"); + text.replace(text.find("id=lofi"), 7, "id=jazz"); + put("kits/lofi/kit.txt", text); + CHECK_FALSE(io::load_kit("lofi", kit)); + CHECK(logged("calls itself a different kit")); +} + +TEST_CASE("T-101 A kit file missing any one of its fields does not load") { + hal_fake::reset(); + engine::Kit kit{}; + const std::string text = kit_file("lofi/kit.txt"); + for (const char* field : {"id=", "swing=", "filter=", "fx=", "sidechain=", "pluck=", "dice=", "progression="}) { + const std::string missing = field; + CAPTURE(missing); + std::string without; + bool dropped = false; + for (const std::string& line : lines_of(text)) { + if (line.rfind(field, 0) == 0) { // every line of that kind: lofi has three dice loops + dropped = true; + continue; + } + without += line + "\n"; + } + REQUIRE(dropped); + put("kits/lofi/kit.txt", without); + CHECK_FALSE(io::load_kit("lofi", kit)); // a zero left where a field should be is not this kit + } +} From aed0c6a1ff3510284c3a956b04773a6828ce32a7 Mon Sep 17 00:00:00 2001 From: Deva Date: Thu, 3 Sep 2026 17:32:48 +0530 Subject: [PATCH 4/5] Answer the second round: a field said twice is a file that says two things MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit One of the two taken. A repeated `id`, `swing`, `filter`, `fx` or `sidechain` line overwrote the first quietly, so a file saying two contradictory things loaded as whichever came last. Which one the kit meant is not a question this firmware gets to answer; it refuses the file. Not taken: requiring every pad to have a tap template. engine::next_template returns -1 when a pad has none and engine::tap then appends a plain hit, so a pad without smart defaults is a pad that works — and tools/kit_builder.py allows a kit.json with no templates. Requiring them here would make the reader stricter than the writer, so a valid kit.json could produce a kit.txt this firmware refuses, which is the one thing the round-trip test exists to prevent. Co-Authored-By: Claude Opus 5 --- DECISIONS.md | 2 +- firmware/src/io/kit.cpp | 15 ++++++++++----- spec/scenarios.md | 2 +- tests/io_test.cpp | 15 +++++++++++++++ 4 files changed, 27 insertions(+), 7 deletions(-) diff --git a/DECISIONS.md b/DECISIONS.md index 10cd90f..055591c 100644 --- a/DECISIONS.md +++ b/DECISIONS.md @@ -112,4 +112,4 @@ What was decided, why, and when to look again. One row per decision. IDs are seq | D-106 | The share-code size cap (T-62) lives in `engine::decode` and `decode_song`, not at each path the bytes arrive on: 512 characters for a section and 2048 for a song, NUL not counted, scanning at most one past the limit so a missing terminator costs a bounded read. | 2026-09-03. The engine's decoder is linear and allocates nothing, so the cap is about refusing a code rather than about safety; putting it where the knowledge is means the card, `tools/render`, and the SysEx, USB and paste-box paths still to come are all covered without each remembering to. Twice the canonical worst case (238 and 988) rather than the buffer sizes, because T-16 says a decoder skips the fields a later version adds and a cap at the buffer would leave almost no room for them. Rejected: a cap at each arriving path (three places to forget, and nothing to write today since none of those paths exists yet). | If a later version's fields need more than the canonical code's length again. | | D-107 | A song slot whose file will not parse (§9.6, T-97): a tap refuses it and says `hold to replace song 2`; a hold on that pad copies the song on screen over it and says `song 2 replaced`. The hold is live only on a slot the device already knows it cannot read, so no hold can destroy a song the player could still open, and a hold anywhere else in the song view still does nothing. | 2026-09-03. D-104 made a file that will not parse a slot rather than a gap, which stopped a pick from silently copying over somebody's song but left the slot unreachable: the only thing that replaced it was the player already being on it, so recovery meant a computer or a factory reset — a dead pad on an instrument that ships with no manual. Press-then-hold is already this device's idiom for a destructive thing, in `hold dice to clear` and `hold play to reset`, and §9.6 said the pads' hold gestures were inactive in the song view, so the gesture was free and consistent rather than new; PRD §9.6 gains the exception. Rejected: a second tap inside the arming timeout (a double tap is easy by accident, and every other confirmation here is a hold); quarantining the file at boot (needs a HAL rename or delete, and silently moves a player's file aside); a settings row to clear a slot (a sub-menu for a rare failure, against §9.6's one screen). | If usability round 1 shows nobody finds the hold, or if a fourth tile state would say it better than the status line. | | D-108 | A kit's samples come off the card, not the firmware image: `io::load_samples` reads `kits//` into the PSRAM `hal::sample_memory()` hands it — 1.5 MB, the eight pads of two seconds D-081 allows each — and fills a `sound::SampleBank` with what it found. Each WAV is read straight into the room left in that memory and parsed where it lands, then moved down over its own header, so nothing is ever staged twice. A file that is missing or is not 16-bit 48 kHz mono PCM costs its own pad its sound and no more. The simulator does the same thing with the same code: `host/CMakeLists.txt` seeds `out/sdcard/kits/` from `spec/kits/` at configure time. | 2026-09-03. §7.5 puts the samples in PSRAM and the kits on the card, and until now the device passed `app::init` an empty bank, so the drum pads were silent on hardware and the simulator used the render tool's host-only loader — two paths, one of them not the product's. Reading in place rather than through a staging buffer because a sample is up to 192 KB and the device has 63 KB of ordinary RAM free; there is nowhere to put a copy. `hal::sample_memory()` answers with nothing when no PSRAM is fitted, which is every board until bring-up, because writing to a section that is not backed would fault rather than fail. Rejected: samples in the firmware image (a kit could never be changed without a rebuild, which is what §12 rule 6 is against); streaming from the card at play time (file I/O in the audio path, forbidden by §12 rule 4). | At bring-up, when a real PSRAM chip says whether reads from it keep up with sixteen voices; or if a kit needs more than eight samples of two seconds. | -| D-109 | A kit is readable text on the card: `tools/kit_builder.py` writes `kits//kit.txt` beside the header it already generates, both from the one `kit.json`, and `io::load_kit` reads it. One line a field, the five progressions in mode order, the eight pads in the order share-format §2 fixes with each pad's tap templates under it, and a step spelled exactly as a share code spells one — `engine::read_step` is now the only implementation of that table. Fractions are whole hundredths, so the device needs no float parser. `engine::Kit` holds its strings in fixed arrays rather than pointers, so a card kit and a compiled kit are one type, and `settings.txt`'s `kit=` line says which folder to play, falling back to the built-in kit when the card has no such kit. | 2026-09-03. §12 rule 6 asks for an open format so the community can make kits, and a kit that only exists as a C++ header can only be made by rebuilding the firmware. Text because §7.6 shows the card over USB and because the songs and settings are already text (D-104): one habit, not three. Rejected: parsing `kit.json` on the device (a JSON parser is a few hundred lines of firmware at a boundary that reads whatever a card holds, against the rule about reaching for the standard library first) and a packed binary (smaller and faster, but then only the tool can make a kit, which is the opposite of open). An unknown line fails the whole file rather than being skipped as an unknown settings row is: a setting the device ignores costs one row, a kit field it ignores would be an instrument quietly playing something other than what the kit says, and a field left out entirely fails for the same reason — the zero it would leave behind is not this kit. Every card string that reaches a path is checked against the grammar rather than trusted: a kit id is share-format §2's `[a-z0-9]{1,12}`, a pad's source is a plain file name, and a kit whose id is not the folder it was found in is refused, because its samples are looked for by its own id and would be hunted for somewhere else. The test that the card kit equals the compiled kit is what holds the builder's two outputs together, and CI diffs both. | If a kit needs a field that is not a number, a name or a share code; or when kits can be chosen from the settings view, which needs the samples reloaded rather than just the file re-read. | +| D-109 | A kit is readable text on the card: `tools/kit_builder.py` writes `kits//kit.txt` beside the header it already generates, both from the one `kit.json`, and `io::load_kit` reads it. One line a field, the five progressions in mode order, the eight pads in the order share-format §2 fixes with each pad's tap templates under it, and a step spelled exactly as a share code spells one — `engine::read_step` is now the only implementation of that table. Fractions are whole hundredths, so the device needs no float parser. `engine::Kit` holds its strings in fixed arrays rather than pointers, so a card kit and a compiled kit are one type, and `settings.txt`'s `kit=` line says which folder to play, falling back to the built-in kit when the card has no such kit. | 2026-09-03. §12 rule 6 asks for an open format so the community can make kits, and a kit that only exists as a C++ header can only be made by rebuilding the firmware. Text because §7.6 shows the card over USB and because the songs and settings are already text (D-104): one habit, not three. Rejected: parsing `kit.json` on the device (a JSON parser is a few hundred lines of firmware at a boundary that reads whatever a card holds, against the rule about reaching for the standard library first) and a packed binary (smaller and faster, but then only the tool can make a kit, which is the opposite of open). An unknown line fails the whole file rather than being skipped as an unknown settings row is: a setting the device ignores costs one row, a kit field it ignores would be an instrument quietly playing something other than what the kit says, and a field left out entirely fails for the same reason — the zero it would leave behind is not this kit — as does a single-valued field said twice, since which of the two the kit meant is not a question this firmware gets to answer. Every card string that reaches a path is checked against the grammar rather than trusted: a kit id is share-format §2's `[a-z0-9]{1,12}`, a pad's source is a plain file name, and a kit whose id is not the folder it was found in is refused, because its samples are looked for by its own id and would be hunted for somewhere else. The test that the card kit equals the compiled kit is what holds the builder's two outputs together, and CI diffs both. | If a kit needs a field that is not a number, a name or a share code; or when kits can be chosen from the settings view, which needs the samples reloaded rather than just the file re-read. | diff --git a/firmware/src/io/kit.cpp b/firmware/src/io/kit.cpp index 59d1af8..a0ff677 100644 --- a/firmware/src/io/kit.cpp +++ b/firmware/src/io/kit.cpp @@ -211,33 +211,35 @@ bool read_kit(char** lines, int count, engine::Kit& kit) { bool have_filter = false; bool have_fx = false; bool have_sidechain = false; + // A field said twice is a file that says two things: which one the kit meant is not + // a question this firmware gets to answer, so it refuses the file instead. char* fields[kMaxFields]; for (int i = 1; i < count; ++i) { char* line = lines[i]; int got = fields_of(line, "id", fields, 1); if (got == 1) { - if (!is_kit_id(fields[0]) || !copy_word(fields[0], kit.id, sizeof kit.id)) return false; + if (have_id || !is_kit_id(fields[0]) || !copy_word(fields[0], kit.id, sizeof kit.id)) return false; have_id = true; continue; } int value = 0; got = fields_of(line, "swing", fields, 1); if (got == 1) { - if (!number(fields[0], 100, value)) return false; + if (have_swing || !number(fields[0], 100, value)) return false; kit.swing_hundredths = static_cast(value); have_swing = true; continue; } got = fields_of(line, "filter", fields, 1); if (got == 1) { - if (!number(fields[0], engine::kTenthsMax, value)) return false; + if (have_filter || !number(fields[0], engine::kTenthsMax, value)) return false; kit.filter = static_cast(value); have_filter = true; continue; } got = fields_of(line, "fx", fields, 1); if (got == 1) { - if (!number(fields[0], engine::kTenthsMax, value)) return false; + if (have_fx || !number(fields[0], engine::kTenthsMax, value)) return false; kit.fx = static_cast(value); have_fx = true; continue; @@ -247,7 +249,10 @@ bool read_kit(char** lines, int count, engine::Kit& kit) { int on = 0; int duck = 0; int release = 0; - if (!number(fields[0], 1, on) || !number(fields[1], 24, duck) || !number(fields[2], 5000, release)) return false; + if (have_sidechain || !number(fields[0], 1, on) || !number(fields[1], 24, duck) || + !number(fields[2], 5000, release)) { + return false; + } kit.sidechain = engine::Sidechain{on == 1, static_cast(duck), static_cast(release)}; have_sidechain = true; continue; diff --git a/spec/scenarios.md b/spec/scenarios.md index 6a03876..6cceed7 100644 --- a/spec/scenarios.md +++ b/spec/scenarios.md @@ -106,7 +106,7 @@ Conventions: fractions are of one cycle; the default kit (lofi), C minor and 100 | T-98 | Write the settings, read them back; then a file with an unknown key, a value outside its range, a line that is not `key=value` and a missing row; then no file at all (§9.4, §7.5, D-104) | Every row and the open song come back as they were. The unknown key, the junk line and the out-of-range value are ignored and the rows they name keep what they had; a missing row keeps its default. A value the settings view itself could never set is not one the card gets to introduce: brightness outside 10–100 and a sleep that is not one of 0, 5, 10, 20, 30 or 60 keep their defaults. No file at all is the defaults: song 1, brightness 100, sleep 10, MIDI and sync on. | | T-99 | Kick ×1, then a second of frames, on a card that refuses writes, then a card that accepts (§7.5, §9.6, D-104) | Nothing is saved by hand: the card takes the song a second after the last change, so one tap costs one write, a refused write is tried again a second later and not on every frame, and the loop plays on either way. Once the card takes it, the next boot comes back to that loop. A pick the card cannot carry out is refused rather than half done: a card that will not take the song being left says `song 1 did not save` and the player stays on it with the edit still in hand, and a slot whose file did not parse says `hold to replace song 2` and is left alone until that hold comes (D-107). A boot writes nothing at all until the player plays something: an absent file and an empty song say the same thing. | | T-100 | Boot with the kit's WAVs on the card; then with one missing, one that is not 16-bit 48 kHz mono, one longer than the two seconds a sample may be, and one that is not a WAVE at all; then with no card, and on a board with no PSRAM fitted (§7.5, §12 rule 6, D-081, D-108) | Each sample pad plays the samples in its own file, packed one after another into the PSRAM and none of them overlapping; a pad whose file is missing or is not a sample this firmware can play is silent and the log names the file and what was wrong with it, while every other pad still sounds. With no card every sample pad is silent and the synth pads play on; with no PSRAM there is nowhere to put a sample at all, which is said once rather than per pad. A pad whose sample came off the card is heard: the same tap is loud with it and inaudible without. | -| T-101 | Read `kits/lofi/kit.txt` as tools/kit_builder.py wrote it; then a file with no `RTK1`, one with a line this firmware does not know, one a pad short, one whose filter is 11, and one whose template holds a character no step can be; then a kit whose id is not the folder it sits in, one whose pad names a source outside its folder, and a settings file whose `kit=` could climb out of `kits/`; then boot with the settings naming a kit on the card, and naming one that is not there (§7.5, §12 rule 6, D-109) | The kit read off the card is equal, field for field, to the one compiled into the firmware — which is what keeps the builder's two outputs saying the same thing. Every malformed file is refused whole and logged: a kit is not a place to guess, so an unknown line fails rather than being skipped, unlike the settings, and a file missing any field it should have — including the ones with no count, whose absence would otherwise leave a zero behind — does not load. Nothing a card holds becomes a path: a kit id is 1–12 of `a`–`z` and `0`–`9` as share-format §2 spells one, a pad's source is a plain file name, and a kit whose id is not the folder it was found in is refused, since its samples would be hunted for somewhere else. Booting with `kit=` naming a kit on the card plays that kit, and a fresh loop takes its swing, filter and fx from it; naming a kit that is not there logs the path and plays the one built in, so the device always comes up. | +| T-101 | Read `kits/lofi/kit.txt` as tools/kit_builder.py wrote it; then a file with no `RTK1`, one with a line this firmware does not know, one a pad short, one whose filter is 11, and one whose template holds a character no step can be; then a kit whose id is not the folder it sits in, one whose pad names a source outside its folder, and a settings file whose `kit=` could climb out of `kits/`; then boot with the settings naming a kit on the card, and naming one that is not there (§7.5, §12 rule 6, D-109) | The kit read off the card is equal, field for field, to the one compiled into the firmware — which is what keeps the builder's two outputs saying the same thing. Every malformed file is refused whole and logged: a kit is not a place to guess, so an unknown line fails rather than being skipped, unlike the settings, and a file missing any field it should have — including the ones with no count, whose absence would otherwise leave a zero behind — does not load, nor does one that says a single-valued field twice. Nothing a card holds becomes a path: a kit id is 1–12 of `a`–`z` and `0`–`9` as share-format §2 spells one, a pad's source is a plain file name, and a kit whose id is not the folder it was found in is refused, since its samples would be hunted for somewhere else. Booting with `kit=` naming a kit on the card plays that kit, and a fresh loop takes its swing, filter and fx from it; naming a kit that is not there logs the path and plays the one built in, so the device always comes up. | ## Watch in testing diff --git a/tests/io_test.cpp b/tests/io_test.cpp index 3f9eaaf..6139ce1 100644 --- a/tests/io_test.cpp +++ b/tests/io_test.cpp @@ -763,3 +763,18 @@ TEST_CASE("T-101 A kit file missing any one of its fields does not load") { CHECK_FALSE(io::load_kit("lofi", kit)); // a zero left where a field should be is not this kit } } + +TEST_CASE("T-101 A kit file that says a thing twice does not load") { + hal_fake::reset(); + engine::Kit kit{}; + const std::string text = kit_file("lofi/kit.txt"); + for (const char* said : {"id=lofi", "swing=15", "filter=10", "fx=2", "sidechain=1,5,120", + "pluck=0,2,4,7,9,7,4,2"}) { + const std::string twice = said; + CAPTURE(twice); + put("kits/lofi/kit.txt", text + twice + "\n"); + CHECK_FALSE(io::load_kit("lofi", kit)); // which of the two it meant is not ours to guess + } + put("kits/lofi/kit.txt", text); // and the file it was made from still loads + CHECK(io::load_kit("lofi", kit)); +} From a4634c6c8e1ee5a78f013b926c30dfaa8ee6335e Mon Sep 17 00:00:00 2001 From: Deva Date: Thu, 3 Sep 2026 17:38:35 +0530 Subject: [PATCH 5/5] Answer the third round: say what the duplicate-field test depends on MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The case appended a repeated field to the file's text and trusted the builder to have ended it with a newline. It does — but if it ever stopped, the duplicate would glue itself to the last line and the case would pass because that line was malformed rather than because the field was said twice. The test says what it depends on now, and fails loudly if it stops being true. Co-Authored-By: Claude Opus 5 --- tests/io_test.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/io_test.cpp b/tests/io_test.cpp index 6139ce1..6d7a81a 100644 --- a/tests/io_test.cpp +++ b/tests/io_test.cpp @@ -768,6 +768,9 @@ TEST_CASE("T-101 A kit file that says a thing twice does not load") { hal_fake::reset(); engine::Kit kit{}; const std::string text = kit_file("lofi/kit.txt"); + // The repeated field has to be its own line, or the case would pass because the line + // it glued itself onto is malformed rather than because it is a repeat. + REQUIRE(text.back() == '\n'); for (const char* said : {"id=lofi", "swing=15", "filter=10", "fx=2", "sidechain=1,5,120", "pluck=0,2,4,7,9,7,4,2"}) { const std::string twice = said;