From ce5db617a1befb72975b2bbc9f207111d0787516 Mon Sep 17 00:00:00 2001 From: Julian Appel Date: Fri, 24 Jul 2026 09:49:21 +0200 Subject: [PATCH] Harden firmware state and transfer handling --- AGENTS.md | 23 ++- README.md | 7 +- boards/versapad_nobl.json | 2 +- doc/00_architecture.md | 9 +- doc/01_matrix.md | 4 +- doc/02_encoder.md | 8 +- doc/03_action_engine.md | 32 ++-- doc/04_macro_system.md | 12 +- doc/05_led_system.md | 10 +- doc/06_nvm_config.md | 27 ++- doc/07_serial_protocol.md | 36 ++-- doc/08_development.md | 15 +- doc/09_known_limitations.md | 113 +++++------ src/CButton.cpp | 5 +- src/CEventQueue.cpp | 4 +- src/CEventQueue.h | 10 +- src/CMainController.cpp | 180 ++++++++++++++---- src/CMainController.h | 15 ++ src/config/action.h | 6 +- src/config/macro_config.cpp | 18 ++ src/config/macro_config.h | 7 + src/config/nvm_config.cpp | 74 ++++++- src/config/nvm_config.h | 11 +- src/hal/usb_hid.cpp | 137 +++++++++++-- src/hal/usb_hid.h | 10 +- src/hal/usb_serial.h | 8 +- .../gcc/flash_without_bootloader.ld | 6 +- 27 files changed, 558 insertions(+), 231 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 5511b73..2bcb0e6 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -43,8 +43,9 @@ festgehalten werden. Firmware-Migration/Versionswechsel, Anpassung der externen GUI und Aktualisierung von `doc/06_nvm_config.md` sowie `doc/07_serial_protocol.md`. -- Die Windows-GUI gehört nicht zu diesem Repository. Keine Kompatibilität mit - ihr behaupten, wenn nur die Firmware geändert oder geprüft wurde. +- Die Windows-GUI liegt im benachbarten, eigenständigen Repository + `../VersaGUI`. Bei gemeinsamen Verträgen beide Repositories ändern, prüfen + und jeweils bedarfsgerecht committen. `DelphiGUI` wird nicht gepflegt. - Die NVM-Adressen liegen am oberen Ende des 128-KiB-Flash. Vor Änderungen an Linker-Skripten, Boardgrößen oder NVM-Layouts immer `doc/09_known_limitations.md` lesen. @@ -52,8 +53,9 @@ festgehalten werden. großen Stackpuffer und keine Float-Arithmetik in Loop-/ISR-Pfaden einführen. - ISR-Code muss kurz und nicht blockierend bleiben. Niemals USB, NVM, WS2812-Ausgabe oder `delay()` aus einer ISR aufrufen. -- Das feste CDC-Protokoll besteht aus 8-Byte-Paketen. Es besitzt aktuell weder - Framing noch Sequenz-/Vollständigkeitsprüfung. +- Das feste CDC-Protokoll besteht aus 8-Byte-Paketen. Blob-Transfers prüfen + Chunkzahl, eindeutige Indizes und Vollständigkeit; paketweises Framing und + eine Makro-CRC gibt es weiterhin nicht. ## Änderungsleitfaden @@ -81,9 +83,16 @@ git diff --check ``` Für Hardware-, USB-, NVM- oder Timingänderungen zusätzlich einen passenden -Gerätetest beschreiben. Es gibt derzeit keine automatisierten Unit- oder -Integrationstests. Ein erfolgreicher Build beweist daher weder elektrische -Funktion noch GUI-Kompatibilität. +Gerätetest beschreiben. Für die Firmware gibt es derzeit keine automatisierten +Unit- oder Integrationstests. Ein erfolgreicher Build beweist daher weder +elektrische Funktion noch GUI-Kompatibilität. + +Bei Änderungen an Config-, Makro- oder CDC-Verträgen zusätzlich: + +```bash +dotnet build ../VersaGUI/src/VersaGUI.csproj --no-restore +dotnet run --project ../VersaGUI/tests/VersaGUI.ContractTests/VersaGUI.ContractTests.csproj +``` Vor Abschluss prüfen: diff --git a/README.md b/README.md index b63063a..b5f0b14 100644 --- a/README.md +++ b/README.md @@ -96,10 +96,9 @@ updateLEDs() | Makros | `0x1FB00..0x1FCFF` | 512 B | | Config | `0x1FD00..0x1FFFF` | 768 B, davon 740 B genutzt | -Wichtig: Das aktive Linker-Skript reserviert aktuell nur die letzten 512 Byte -explizit. Das derzeit kleine Firmware-Image überschneidet sich nicht mit den -NVM-Daten, zukünftiges Wachstum ist aber nicht vollständig abgesichert. Siehe -[bekannte Einschränkungen](doc/09_known_limitations.md). +Das aktive Linker-Skript und `boards/versapad_nobl.json` begrenzen das +Firmware-Image auf `0x00000..0x1FAFF`. Damit sind alle fünf NVM-Rows gegen +Firmwarewachstum geschützt. ## Werksreset diff --git a/boards/versapad_nobl.json b/boards/versapad_nobl.json index 143be47..d1d2429 100644 --- a/boards/versapad_nobl.json +++ b/boards/versapad_nobl.json @@ -20,7 +20,7 @@ "name": "VersaPad v2 (Atmel-ICE, no bootloader)", "upload": { "maximum_ram_size": 16384, - "maximum_size": 131072, + "maximum_size": 129792, "protocol": "atmel-ice", "require_upload_port": false, "use_1200bps_touch": false diff --git a/doc/00_architecture.md b/doc/00_architecture.md index 4ff29bd..ee2cf35 100644 --- a/doc/00_architecture.md +++ b/doc/00_architecture.md @@ -119,8 +119,7 @@ Der Werksreset ist keine PC-Funktion, sondern Teil der Firmware: ## Nebenläufigkeit Encoder-Callbacks laufen im EIC-Interrupt, Matrixcallbacks im Loop. Beide -schreiben in dieselbe `CEventQueue`; `processEvents()` liest im Loop. Die Queue -besitzt aktuell keinen Interruptschutz für den Matrix-Push und verwirft Events -bei Überlauf. Das ist eine bekannte Einschränkung, keine garantierte -Multi-Producer-Sicherheit; siehe -[09_known_limitations.md](09_known_limitations.md). +schreiben in dieselbe `CEventQueue`; `processEvents()` liest im Loop. Der +Matrixcallback maskiert Interrupts während seines Queue-Pushs, sodass Loop und +ISR den Tail-Index nicht gleichzeitig ändern. Bei Überlauf werden Events +weiterhin verworfen. diff --git a/doc/01_matrix.md b/doc/01_matrix.md index 43fc5ed..49f2ffa 100644 --- a/doc/01_matrix.md +++ b/doc/01_matrix.md @@ -44,5 +44,5 @@ key_id = col * MATRIX_ROWS + row - Encoder-SW-Tasten gehen durch denselben Matrix-Pfad (`COL_0`) - `matrix_scan()` wird einmal pro `loop()` aufgerufen - Der Callback schreibt in dieselbe Queue wie die Encoder-ISRs. Ein - Encoderinterrupt kann den Matrix-Push unterbrechen; die aktuelle Queue - schützt diesen Multi-Producer-Fall nicht ausdrücklich. + kurzer `noInterrupts()`/`interrupts()`-Abschnitt schützt den Matrix-Push + davor, von einem Encoderinterrupt unterbrochen zu werden. diff --git a/doc/02_encoder.md b/doc/02_encoder.md index c54476d..d5e6a36 100644 --- a/doc/02_encoder.md +++ b/doc/02_encoder.md @@ -45,11 +45,9 @@ static void isr_enc0_b() { handle_encoder(0); } - ISR-Wrapper führen nur Dekodierung und Queue-Push aus; sie rufen kein USB, NVM, LED-Rendering oder `delay()` auf -Wichtig: Die Queue wird zusätzlich vom Matrixcallback im Loop beschrieben. -Ein Encoderinterrupt kann diesen Push unterbrechen. Die Implementierung ist -damit nicht streng Single-Producer und besitzt für diesen Fall aktuell keinen -Interruptschutz. Details: -[09_known_limitations.md](09_known_limitations.md). +Die Queue wird zusätzlich vom Matrixcallback im Loop beschrieben. Dieser +Loop-Push läuft in einer kurzen Critical Section; gleichpriorisierte +Encoder-ISRs unterbrechen sich auf dem Cortex-M0+ nicht gegenseitig. ## Initialisierung diff --git a/doc/03_action_engine.md b/doc/03_action_engine.md index d4ddbf2..299f270 100644 --- a/doc/03_action_engine.md +++ b/doc/03_action_engine.md @@ -25,9 +25,9 @@ Das `packed` ist zwingend, weil Config v3 bytegenau zwischen Firmware und GUI ue | `NONE` | keine Aktion | - | | `HID_KEY` | Tastaturtaste ueber USB HID | low byte = keycode, high byte = modifier | | `HID_CONSUMER` | Media/Consumer-HID | usage id | -| `HOST_COMMAND` | Event an die GUI | aktuell nicht ausgewertet | +| `HOST_COMMAND` | Event an die GUI | 16-Bit-Command-ID | | `MACRO` | Makro aus `SMacroTable` | slot 0..31 | -| `PROFILE_SWITCH` | Profilwechsel | 0..2 oder `0xFF` fuer naechstes Profil | +| `PROFILE_SWITCH` | Profilwechsel | 0..2, `0x00FF` oder `0xFFFF` für nächstes Profil | ## Verhalten bei `KEY_DOWN` @@ -35,7 +35,7 @@ Das `packed` ist zwingend, weil Config v3 bytegenau zwischen Firmware und GUI ue |---|---| | `HID_KEY` | `usb_hid_send_key()` | | `HID_CONSUMER` | `usb_hid_send_consumer()` | -| `HOST_COMMAND` | `USB_EVT_KEY_DOWN (0x81)` mit `key_id` senden | +| `HOST_COMMAND` | `USB_EVT_KEY_DOWN (0x81)` mit `key_id` und Command-ID senden | | `MACRO` | komplette Sequenz sofort abspielen | | `PROFILE_SWITCH` | Config aus NVM laden, Profil aendern, CRC neu berechnen, speichern, Buttons neu initialisieren | | `NONE` | nichts | @@ -46,7 +46,7 @@ Das `packed` ist zwingend, weil Config v3 bytegenau zwischen Firmware und GUI ue |---|---| | `HID_KEY` | `usb_hid_release_key()` | | `HID_CONSUMER` | `usb_hid_release_consumer()` | -| `HOST_COMMAND` | keine Ausgabe; `USB_EVT_KEY_UP` ist nur definiert | +| `HOST_COMMAND` | `USB_EVT_KEY_UP (0x82)` mit `key_id` und Command-ID senden | | `MACRO` | nichts | | `PROFILE_SWITCH` | nichts | | `NONE` | nichts | @@ -62,25 +62,29 @@ down -> delay(10 ms) -> up - Makros laufen komplett synchron in der Firmware. -Das Hold-Modell verwaltet keine Menge gleichzeitig gedrückter Tasten. Ein -neuer Keyboard-Down ersetzt den vorherigen Report und jeder Keyboard-Up leert -den gesamten Report. Consumer-HID hat dieselbe Einschränkung mit genau einem -Usage-Wert. +Keyboard-Keys und Modifier werden im HID-HAL referenzgezählt. Bis zu sechs +unterschiedliche Keyboard-Usages können der Report gleichzeitig abbilden; +beim Loslassen einer Action bleiben die übrigen Holds aktiv. + +Der Consumer-Descriptor kann jeweils nur ein Usage darstellen. Der HAL +verwaltet mehrere Holds und zeigt das zuletzt gedrückte aktive Usage; nach +dessen Release wird das zuvor aktive Usage wiederhergestellt. ## Host-Commands Der aktuelle Code sendet für jede `HOST_COMMAND`-Action nur: ```text -Byte 0 = USB_EVT_KEY_DOWN (0x81) +Byte 0 = USB_EVT_KEY_DOWN (0x81) oder USB_EVT_KEY_UP (0x82) Byte 1 = Matrix-Key-ID oder Encoder-ID +Byte 2 = Command-ID Low-Byte +Byte 3 = Command-ID High-Byte ``` -`SAction.data` wird dabei nicht übertragen. Auch Encoder-Actions verwenden -aktuell `0x81`; die definierten Events `ENC_CW (0x83)` und `ENC_CCW (0x84)` -werden nicht emittiert. Ein Release erzeugt kein `KEY_UP`-Paket. Das ist -aktuelles Verhalten und als Einschränkung in -[09_known_limitations.md](09_known_limitations.md) festgehalten. +Encoder-Host-Actions senden genau ein Richtungsereignis: +`ENC_CW (0x83)` beziehungsweise `ENC_CCW (0x84)`, ebenfalls mit Encoder-ID und +Command-ID. Die GUI erhält damit die konfigurierte Action direkt aus dem +Event und muss den aktiven Profilstand nicht rekonstruieren. ## Makro-Ausfuehrung diff --git a/doc/04_macro_system.md b/doc/04_macro_system.md index bd59797..22b703f 100644 --- a/doc/04_macro_system.md +++ b/doc/04_macro_system.md @@ -52,7 +52,8 @@ sie nicht: `SAction.data` wird direkt als Slotindex `0..31` verwendet. - kopiert 512 Byte aus NVM in `SMacroTable` - erkennt komplett geloeschten Flash (`0xFF`) als "noch nie beschrieben" -- setzt dann eine leere Tabelle +- prüft alle belegten Steps gegen den HID-Descriptor (`keycode <= 0x65`) +- setzt bei gelöschtem oder ungültigem Inhalt eine leere Tabelle Eine leere Tabelle ist also ein gueltiger Default-Zustand. @@ -60,15 +61,18 @@ Eine leere Tabelle ist also ein gueltiger Default-Zustand. `macro_config_save()`: -1. Tabelle in einen 4-Byte-aligned Puffer kopieren -2. beide Rows loeschen -3. 8 Pages zu je 64 Byte schreiben +1. HID-Keycodes validieren +2. Tabelle in einen 4-Byte-aligned Puffer kopieren +3. beide Rows loeschen +4. 8 Pages zu je 64 Byte schreiben Rueckgabewert: - `true` bei Erfolg - `false` bei NVM-Timeout +`static_assert` schützt die erwarteten Größen 2 und 512 Byte beim Build. + ## Ausfuehrung Beim Triggern eines Makros: diff --git a/doc/05_led_system.md b/doc/05_led_system.md index fbd2c31..819e00b 100644 --- a/doc/05_led_system.md +++ b/doc/05_led_system.md @@ -73,12 +73,12 @@ Die Firmware verwendet eine eigene ganzzahlige Hue-Umrechnung und ruft `Adafruit_NeoPixel::ColorHSV()` nicht auf. Für `PULSE` muss `period_ms >= 2` gelten, da der Code durch die halbe Periode -teilt. Eingehende Configs validieren diesen Grenzwert aktuell nicht. +teilt. Configvalidierung lehnt kleinere Werte ab; `set_anim()` klemmt direkte +interne Aufrufe zusätzlich auf mindestens 2 ms. -`COLOR_FADE` benötigt `set_color_fade(to, period_ms)`, weil dort Start- und -Zielfarbe gesetzt werden. Ein bloßes `set_anim(COLOR_FADE, ...)`, wie es beim -direkten Laden dieses Enum-Werts aus der Config geschieht, initialisiert diese -Farben nicht aus den Configdaten. +`COLOR_FADE` benötigt `set_color_fade(to, period_ms)`. Beim Laden aus der +Config interpretiert der Controller die gespeicherte Base-Farbe als Ziel und +startet einen einmaligen Fade von Schwarz zu dieser Farbe. ### Phasenversatz (Regenbogen-Wellen) diff --git a/doc/06_nvm_config.md b/doc/06_nvm_config.md index a7f2076..672e101 100644 --- a/doc/06_nvm_config.md +++ b/doc/06_nvm_config.md @@ -17,12 +17,10 @@ Dateien: Makros und Config sind komplett getrennt. -Diese Tabelle beschreibt die Adressen, auf die der Laufzeitcode zugreift. Das -aktive Linker-Skript reserviert davon aktuell nur `0x1FE00..0x1FFFF` -ausdrücklich; Makrobereich und erste Config-Row liegen noch im zulässigen -Firmware-ROM. Das aktuelle Binary ist klein genug, aber die Bereiche sind -nicht vollständig gegen Firmwarewachstum geschützt. Siehe -[09_known_limitations.md](09_known_limitations.md). +Das aktive Linker-Skript reserviert `0x1FB00..0x1FFFF` als eigenen +NVM-Memory-Bereich. Die Boarddefinition meldet entsprechend höchstens +129.792 Byte Firmware-Flash. Damit kann ein erfolgreich gelinktes Image die +fünf NVM-Rows nicht überdecken. ## `SDeviceConfig` @@ -101,18 +99,17 @@ Praktisch sichtbares Ergebnis: `nvm_config_load()`: 1. 740 Byte aus NVM kopieren -2. Magic pruefen -3. Version pruefen -4. CRC pruefen -5. bei Fehlern Defaults laden und `false` zurueckgeben +2. Magic, Version und CRC prüfen +3. Profilindex, Actiontypen/-daten und LED-Enums prüfen +4. `PULSE`-Perioden auf mindestens 2 ms prüfen +5. bei Fehlern Defaults laden und `false` zurückgeben Die Firmware faellt also immer auf einen gueltigen Zustand zurueck. Die Defaults werden bei diesem Fallback nur in das übergebene RAM-Struct geschrieben und nicht automatisch in Flash persistiert. -Nach erfolgreicher CRC-Prüfung sichert `load()` einen zu großen -`active_profile` im RAM auf Profil 0 ab. Andere Felder und Enum-Werte werden -nicht auf gültige Bereiche geprüft. +Dieselbe Validierung wird vor einem Config-Commit aus dem CDC-Protokoll +verwendet. ## Speichern @@ -134,8 +131,8 @@ Wichtig: - der Schreibpuffer muss 4-Byte-aligned sein - `packed` allein reicht dafuer nicht -- `nvm_config_save()` berechnet die CRC nicht selbst; der Aufrufer muss - `cfg.crc = nvm_config_crc(cfg)` vorher setzen +- `nvm_config_save()` berechnet die CRC in einer lokalen Kopie immer neu +- `static_assert` schützt die erwarteten Größen 3, 236 und 740 Byte beim Build ## Zusammenhang mit Werksreset diff --git a/doc/07_serial_protocol.md b/doc/07_serial_protocol.md index 2dad363..d593641 100644 --- a/doc/07_serial_protocol.md +++ b/doc/07_serial_protocol.md @@ -50,10 +50,10 @@ Es gibt kein Framing, keinen Längenheader und keine Prüfsumme auf Paketebene. | ID | Name | Zweck | |---|---|---| -| `0x81` | `KEY_DOWN` | wird für jede aktuelle Host-Action gesendet | -| `0x82` | `KEY_UP` | definiert, aktuell nicht gesendet | -| `0x83` | `ENC_CW` | definiert, aktuell nicht gesendet | -| `0x84` | `ENC_CCW` | definiert, aktuell nicht gesendet | +| `0x81` | `KEY_DOWN` | Host-Button gedrückt | +| `0x82` | `KEY_UP` | Host-Button losgelassen | +| `0x83` | `ENC_CW` | Encoder-Host-Action im Uhrzeigersinn | +| `0x84` | `ENC_CCW` | Encoder-Host-Action gegen Uhrzeigersinn | | `0x85` | `PONG` | Antwort auf Ping | | `0x90` | `CONFIG_ACK` | Config erfolgreich gespeichert | | `0x91` | `CONFIG_NACK` | Config ungueltig oder NVM-Timeout | @@ -66,9 +66,10 @@ Es gibt kein Framing, keinen Längenheader und keine Prüfsumme auf Paketebene. | `0x98` | `MACRO_END` | Makro-Dump fertig | | `0x99` | `MACRO_NACK` | Makro-Speichern fehlgeschlagen | -Bei `ActionType::HOST_COMMAND` enthält Byte 1 die Matrix-Key-ID oder die -Encoder-ID. `SAction.data` wird nicht übertragen. Auch CW- und CCW-Actions -senden aktuell `0x81`; die Drehrichtung ist im Paket nicht enthalten. +Bei `ActionType::HOST_COMMAND` enthält Byte 1 die Matrix-Key-ID oder +Encoder-ID. Die 16-Bit-Command-ID aus `SAction.data` steht little-endian in +Byte 2 und 3. Encoder-Actions verwenden die richtungsspezifischen IDs +`0x83/0x84`. ## Chunk-Zahlen @@ -120,16 +121,15 @@ Nur bei erfolgreicher Pruefung wird in NVM geschrieben. `MACRO_COMMIT` schreibt ohne CRC direkt nach NVM und signalisiert nur Erfolg oder Fehler. -Die in `BEGIN` angekündigte Chunkzahl wird zwar gespeichert, aber beim Commit -nicht ausgewertet. Die Firmware verfolgt nicht, welche Chunkindizes tatsächlich -eingetroffen sind. Doppelte, fehlende und ungeordnete Chunks werden deshalb -nicht als solche erkannt. Bei Configdaten schlägt ein unvollständiger Transfer -typischerweise an der CRC fehl; bei Makrodaten kann ein unvollständiger -Null-gefüllter Puffer gespeichert werden. +Beide Empfangspfade erwarten exakt die berechnete Chunkzahl, markieren jeden +Index einmalig und akzeptieren `COMMIT` nur nach einem vollständigen Transfer. +Doppelte oder außerhalb des Bereichs liegende Chunks machen den Transfer +ungültig und führen beim Commit zu NACK. -Die Configvalidierung prüft keine Feldwerte oder Enum-Bereiche. Insbesondere -müssen Hostimplementierungen gültige Profilindizes, Actiontypen, LED-Enums und -Animationsperioden liefern. +Config-Commit prüft zusätzlich Magic, Version, CRC, Profilindex, +Actiontypen/-daten, LED-Enums und kritische Animationsperioden. +Makro-Commit prüft die Vollständigkeit und alle HID-Keycodes. Die Makrotabelle +besitzt weiterhin keine eigene persistente CRC. ## Praktische Hinweise fuer die GUI @@ -139,7 +139,9 @@ Animationsperioden liefern. - `DtrEnable` muss aktiv sein, sonst verwirft das Board CDC-Ausgaben - ausschließlich vollständige 8-Byte-Pakete schreiben; schon ein verlorenes Byte verschiebt die Paketgrenzen für alle folgenden Daten -- Encoder-Host-Actions derzeit nicht anhand von `0x83/0x84` erwarten +- Host-Command-ID little-endian aus Byte 2/3 lesen +- Config- und Makro-Dumps ebenfalls auf Chunkzahl, eindeutige Indizes und + Vollständigkeit prüfen ## Implementierungsdetails diff --git a/doc/08_development.md b/doc/08_development.md index 524ff92..9e50f00 100644 --- a/doc/08_development.md +++ b/doc/08_development.md @@ -9,7 +9,7 @@ LLM-basierten Coding-Agent zusätzlich die Anweisungen in - PlatformIO Core oder PlatformIO IDE - USB-Kabel für Laufzeittests - Atmel-ICE beziehungsweise kompatibler CMSIS-DAP-Adapter für den Upload -- Zugriff auf die separat gepflegte Windows-GUI, wenn das CDC-Protokoll oder +- das benachbarte Repository `../VersaGUI`, wenn das CDC-Protokoll oder persistente Formate geändert werden PlatformIO lädt den Arduino-SAMD-Core, OpenOCD und `Adafruit NeoPixel` über @@ -66,7 +66,8 @@ und NVM-Schreibvorgängen. Es gibt keinen Scheduler und keine Threads. ## Verifikation -Es gibt derzeit keine automatisierten Tests. Der minimale lokale Check ist: +Für die Firmware gibt es derzeit keine automatisierten Tests. Der minimale +lokale Check ist: ```bash pio run -e versapad @@ -82,9 +83,12 @@ Je nach Änderung folgen Hardwaretests: - NVM: Power-Cycle, ungültige CRC und Werksreset - LEDs: alle Animationen, Helligkeit und temporäre Overrides -Die GUI ist ein externer Vertrag. Änderungen an `SDeviceConfig`, -`SMacroTable`, Action-Werten oder USB-IDs sind erst vollständig verifiziert, -wenn Firmware und GUI dieselben Bytes senden und interpretieren. +Die GUI ist ein separates Git-Repository im selben Workspace. Änderungen an +`SDeviceConfig`, `SMacroTable`, Action-Werten oder USB-IDs sind erst +vollständig verifiziert, wenn Firmware und `../VersaGUI` dieselben Bytes +senden und interpretieren. `DelphiGUI` gehört nicht zum gepflegten Scope. +Die automatisierten GUI-Vertragstests liegen unter +`../VersaGUI/tests/VersaGUI.ContractTests/`. ## Dokumentation mitpflegen @@ -92,4 +96,3 @@ Bei jedem Change die betroffene Fachdokumentation aktualisieren. Zahlen wie Structgrößen, Offsets, Chunk-Anzahlen und Flashgrenzen immer aus dem neuen Code neu ableiten. Offene oder absichtlich nicht behobene Punkte gehören nach [`09_known_limitations.md`](09_known_limitations.md). - diff --git a/doc/09_known_limitations.md b/doc/09_known_limitations.md index 9e2117c..be5152d 100644 --- a/doc/09_known_limitations.md +++ b/doc/09_known_limitations.md @@ -1,93 +1,78 @@ # Bekannte Einschränkungen und Risiken -Diese Liste beschreibt den aktuellen Implementierungsstand. Sie ist keine -Liste bereits umgesetzter Features. +Diese Liste beschreibt den aktuellen Implementierungsstand nach den +Robustheitskorrekturen. Sie ist keine Liste bereits umgesetzter Features. -## Flash-Reservierung stimmt nicht vollständig mit dem NVM-Zugriff überein +## Bootloader-Ziel bleibt nicht unterstützt -Die Firmware liest und schreibt: - -```text -0x1FB00..0x1FCFF Makros (512 Byte) -0x1FD00..0x1FFFF Config (768 Byte, davon 740 Byte genutzt) -``` - -Das aktive Linker-Skript `flash_without_bootloader.ld` erlaubt Firmware jedoch -bis einschließlich `0x1FDFF` und reserviert nur `0x1FE00..0x1FFFF`. Damit sind -`0x1FB00..0x1FDFF` nicht gegen ein zukünftig wachsendes Firmware-Image -geschützt. Das aktuelle Image liegt deutlich darunter, aber der Build prüft -diese NVM-Grenze nicht. - -Das Bootloader-Linker-Skript reserviert aktuell gar keinen separaten -NVM-Bereich. Das auskommentierte USB-Bootloader-Environment ist daher kein -unterstütztes Ziel. +Das aktive Ziel `versapad_nobl` reserviert den kompletten Bereich +`0x1FB00..0x1FFFF` für Makros und Config. Das auskommentierte +USB-Bootloader-Environment verwendet dagegen weiterhin eine historische +Board-/Linker-Konfiguration und ist nicht als Produktionsziel verifiziert. Die aktive Boarddatei benennt die MCU als `samd21g17d`, setzt für den Arduino-Core aber weiterhin das Kompatibilitätsmakro `__SAMD21G18A__`. Der -PlatformIO-Build meldet korrekt 128 KiB Flash und 16 KiB RAM; vor -device-spezifischen Änderungen sollte diese historische Makro-Abweichung -trotzdem geprüft werden. +PlatformIO-Build meldet korrekt 128 KiB physischen Flash, 16 KiB RAM und +129.792 Byte nutzbaren Firmwarebereich. Vor device-spezifischen +Core-Änderungen sollte die historische Makro-Abweichung trotzdem geprüft +werden. -## Event-Queue hat gemischte Producer +## Event-Queue hat eine feste Kapazität -Matrixevents werden im Loop erzeugt, Encoderevents in EIC-Interrupts. Beide -rufen `CEventQueue::push()` auf und verändern denselben Tail-Index ohne -Interruptschutz. Ein Encoderinterrupt kann einen Matrix-Push unterbrechen. -Die bisherige Annahme eines reinen Single-Producer/Single-Consumer-Ringbuffers -ist deshalb nicht vollständig erfüllt; seltene verlorene oder überschriebene -Events sind theoretisch möglich. +Matrix- und Encoder-Producer verändern den Tail-Index nicht mehr gleichzeitig: +Der Matrixcallback maskiert Interrupts während seines Queue-Pushs. -Zusätzlich werden Events bei voller Queue still verworfen. +Die Queue besitzt aber weiterhin nur 16 nutzbare Slots. Bei Überlauf wird ein +neues Event ohne Hostmeldung verworfen. Das kann vor allem während +blockierender Makro-, NVM- oder Feedbackpfade auftreten. -## `HOST_COMMAND` nutzt seine `data` nicht +## Host-Command-Ausführung liegt in der Desktop-App -`SAction.data` ist für `ActionType::HOST_COMMAND` vorhanden, wird in -`execute_action_down()` aber nicht übertragen. Gesendet wird nur -`USB_EVT_KEY_DOWN (0x81)` mit der Matrix-Key-ID beziehungsweise Encoder-ID. +Die Firmware überträgt Command-ID, Key-/Encoder-ID und Eventrichtung +vollständig. VersaGUI validiert und empfängt diese Pakete, führt eine +Command-ID aber noch nicht als Prozess-, URL- oder frei konfigurierbare +Hostaktion aus. Eine spätere Implementierung benötigt ein explizites, +sicheres Mapping; beliebige Command-Strings sollten nicht direkt an eine Shell +weitergegeben werden. -`USB_EVT_KEY_UP (0x82)`, `USB_EVT_ENC_CW (0x83)` und -`USB_EVT_ENC_CCW (0x84)` sind definiert, werden vom aktuellen Controller aber -nicht gesendet. Bei Encoder-Host-Actions geht dadurch die Richtung im -gesendeten Paket verloren, sofern die Host-Anwendung sie nicht anderweitig aus -der konfigurierten Action ableitet. +## Grenzen des HID-Reports -## HID-Holds sind global +Keyboard-Holds und Modifier werden referenzgezählt. Der USB-Descriptor kann +maximal sechs unterschiedliche Keyboard-Usages gleichzeitig darstellen. +Weitere Holds bleiben intern aktiv und rücken nach, sobald ein Report-Slot +frei wird. -Der Keyboard-Report enthält zwar sechs Keycode-Felder, die Implementierung -setzt aber nur das erste. Jeder neue `HID_KEY`-Down ersetzt den vorherigen -Report; jedes Release sendet einen komplett leeren Report. Analog existiert -nur ein globaler Consumer-Usage-Wert. Gleichzeitige unabhängige Holds werden -daher nicht korrekt verwaltet. +Der Consumer-Descriptor enthält genau ein Usage. Mehrere Consumer-Holds werden +intern verwaltet, sichtbar ist jeweils das zuletzt gedrückte aktive Usage. -## CDC-Transfers sind nur schwach validiert +## CDC bleibt ein festes, ungeframtes Paketprotokoll -- `BEGIN` speichert die angekündigte Chunkzahl, `COMMIT` vergleicht sie aber - nicht mit empfangenen Chunks. -- Doppelte, fehlende oder ungeordnete Chunks werden nicht verfolgt. -- Config-Commit prüft Magic, Version und CRC, aber keine Feldwerte oder - Enum-Bereiche. -- Makro-Commit hat weder CRC noch Vollständigkeitsprüfung. -- Das 8-Byte-Protokoll besitzt kein Framing. Ein verlorenes oder zusätzliches - Byte desynchronisiert alle folgenden Pakete. +Config- und Makrotransfers prüfen jetzt Chunkzahl, eindeutige Indizes und +Vollständigkeit. Config besitzt zusätzlich CRC und Feldvalidierung. -Insbesondere kann ein formal CRC-korrektes Profil ungültige Animationswerte -enthalten. `PULSE` benötigt in der aktuellen Arithmetik eine Periode von -mindestens 2 ms; dieser Mindestwert wird nicht validiert. +Weiterhin gilt: -## Farbanimationen umgehen Teile der Helligkeits-/Override-Logik +- Das Protokoll hat kein Byte-Framing. Ein verlorenes oder zusätzliches Byte + verschiebt die 8-Byte-Paketgrenzen bis zum Reconnect. +- Einzelpakete besitzen keine Sequenznummer oder Prüfsumme. +- Die Makrotabelle besitzt im NVM keine persistente CRC; beim Transfer werden + nur Vollständigkeit und HID-Keycode-Bereiche geprüft. + +## Farbanimationen und Helligkeit Globale und LED-spezifische Helligkeit werden beim Laden in die Base-Farbe eingerechnet. `COLOR_CYCLE` berechnet RGB dagegen direkt mit festen 40 % -Helligkeit und ignoriert Base-Farbe sowie Override. `COLOR_FADE` benötigt den -separaten Aufruf `set_color_fade()`; das reine Laden des Enum-Werts aus einer -Config setzt keine Start- und Zielfarbe. +Helligkeit und ignoriert Base-Farbe sowie Override. + +`COLOR_FADE` aus der Config wird als einmaliger Fade von Schwarz zur +gespeicherten Base-Farbe interpretiert. ## Reservierte beziehungsweise noch ungenutzte Hardware und Felder - Die drei Fader-Pins sind im Variant und in `config/pins.h` definiert, werden - von der Firmware aber nicht eingelesen. -- `enc_sensitivity[4]` wird gespeichert und mit Default `1` befüllt, beeinflusst - die Encoderdekodierung derzeit aber nicht. + absichtlich noch nicht von der Firmware eingelesen. +- `enc_sensitivity[4]` wird gespeichert und mit Default `1` befüllt, + beeinflusst die Encoderdekodierung derzeit aber nicht. - `SET_LED_BASE` verändert nur den RAM-Zustand und wird nicht in NVM persistiert. diff --git a/src/CButton.cpp b/src/CButton.cpp index cc87b06..030d952 100644 --- a/src/CButton.cpp +++ b/src/CButton.cpp @@ -101,7 +101,10 @@ void CButton::clear_override() void CButton::set_anim(LEDAnim anim, uint16_t period_ms, uint16_t phase_offset_ms) { m_anim = anim; - m_anim_period_ms = (period_ms > 0) ? period_ms : 1; // Division durch 0 vermeiden + // PULSE teilt intern durch die halbe Periode und braucht daher mindestens + // 2 ms. Für alle anderen Animationen genügt 1 ms als sicherer Mindestwert. + uint16_t minimum = (anim == LEDAnim::PULSE) ? 2 : 1; + m_anim_period_ms = (period_ms >= minimum) ? period_ms : minimum; // phase_offset_ms in die Vergangenheit zurücksetzen → verschobener Startpunkt m_anim_start_ms = millis() - phase_offset_ms; m_dirty = true; diff --git a/src/CEventQueue.cpp b/src/CEventQueue.cpp index 95c4760..0198970 100644 --- a/src/CEventQueue.cpp +++ b/src/CEventQueue.cpp @@ -9,8 +9,8 @@ // // Nebenläufigkeit: // Encoder-ISRs und der Matrixcallback im Loop können beide push() aufrufen. -// Dieser gemischte Producerfall ist aktuell nicht durch eine Critical -// Section geschützt; siehe doc/09_known_limitations.md. +// Der Matrixcallback schützt seinen push() mit einer kurzen Critical Section, +// sodass m_tail nie von Loop und ISR gleichzeitig verändert wird. #include "CEventQueue.h" diff --git a/src/CEventQueue.h b/src/CEventQueue.h index 5fd809b..e0339b6 100644 --- a/src/CEventQueue.h +++ b/src/CEventQueue.h @@ -9,8 +9,10 @@ // // Nebenläufigkeit: // push() wird aus Encoder-ISRs und aus dem Matrixcallback im Loop aufgerufen. -// pop() läuft ebenfalls im Loop. Es gibt aktuell keine Critical Section für -// den Fall, dass ein Encoderinterrupt einen Matrix-Push unterbricht. +// Der Matrixcallback maskiert Interrupts während push(); Encoder-ISRs können +// sich auf dem Single-Core-M0+ nicht gleichpriorisiert gegenseitig unterbrechen. +// pop() läuft im Loop; ein gleichzeitig eintreffender Push wird spätestens +// im nächsten processEvents()-Durchlauf sichtbar. #pragma once #include "SEvent.h" @@ -31,6 +33,6 @@ public: private: static const uint8_t QUEUE_SIZE = 17; // 16 nutzbare Slots SEvent m_buf[QUEUE_SIZE]; - uint8_t m_head = 0; // Nächster Lese-Index (Consumer: pop) - uint8_t m_tail = 0; // Nächster Schreib-Index (Producer: push) + volatile uint8_t m_head = 0; // Nächster Lese-Index (Consumer: pop) + volatile uint8_t m_tail = 0; // Nächster Schreib-Index (Producer: push) }; diff --git a/src/CMainController.cpp b/src/CMainController.cpp index b02996b..c695788 100644 --- a/src/CMainController.cpp +++ b/src/CMainController.cpp @@ -54,12 +54,17 @@ static void matrix_cb(uint8_t key, bool pressed) ev.type = pressed ? EventType::KEY_DOWN : EventType::KEY_UP; ev.key_id = key; ev.payload = 0; + + // Encoder-ISRs benutzen dieselbe Queue. Den Loop-Producer kurz gegen einen + // dazwischenlaufenden ISR-Push schützen; Encoder-Pushes selbst laufen + // bereits mit maskierten gleichpriorisierten Interrupts. + noInterrupts(); s_queue->push(ev); + interrupts(); } // Wird von handle_encoder() aufgerufen – läuft im ISR-Kontext (EIC-Interrupt). -// Die Queue vermeidet den Heap, schützt den gemischten Matrix-/ISR-Producerfall -// aber aktuell nicht mit einer Critical Section. +// Der Matrix-Producer maskiert Interrupts während seines Queue-Pushs. static void encoder_cb(uint8_t enc, int8_t dir) { if (!s_queue) return; @@ -75,8 +80,10 @@ static void encoder_cb(uint8_t enc, int8_t dir) CMainController::CMainController() : m_cfg_chunks_expected(0) , m_cfg_receiving(false) + , m_cfg_transfer_valid(false) , m_macro_chunks_expected(0) , m_macro_receiving(false) + , m_macro_transfer_valid(false) , m_factory_left_held(false) , m_factory_right_held(false) , m_factory_reset_armed(false) @@ -84,10 +91,21 @@ CMainController::CMainController() , m_factory_hold_started_ms(0) { memset(m_cfg_buf, 0, sizeof(m_cfg_buf)); + memset(m_cfg_received, 0, sizeof(m_cfg_received)); memset(m_macro_buf, 0, sizeof(m_macro_buf)); + memset(m_macro_received, 0, sizeof(m_macro_received)); memset(&m_macros, 0, sizeof(m_macros)); } +bool CMainController::all_chunks_received( + const uint8_t* received, uint8_t count) +{ + for (uint8_t i = 0; i < count; i++) { + if (received[i] == 0) return false; + } + return true; +} + void CMainController::setup() { macro_config_load(m_macros); // Makro-Tabelle aus NVM laden (oder leere Tabelle) @@ -148,6 +166,11 @@ void CMainController::init_buttons() // Phase gleichmäßig verteilen → stehender Regenbogen dreht sich uint16_t phase = (uint16_t)((uint32_t)mx_idx * period / 20); m_buttons[key].set_anim(LEDAnim::COLOR_CYCLE, period, phase); + } else if (anim == LEDAnim::COLOR_FADE) { + // Config enthält nur eine Ziel-/Base-Farbe. COLOR_FADE wird beim + // Laden daher eindeutig als einmaliges Schwarz→Base interpretiert. + m_buttons[key].set_base(RGB(0, 0, 0)); + m_buttons[key].set_color_fade(base, period); } else { m_buttons[key].set_anim(anim, period); } @@ -214,17 +237,30 @@ void CMainController::poll_vendor() // Neuen Empfang starten – bisherige Daten verwerfen m_cfg_chunks_expected = pkt.key_id(); m_cfg_receiving = true; + m_cfg_transfer_valid = (m_cfg_chunks_expected == CONFIG_CHUNKS); memset(m_cfg_buf, 0, sizeof(m_cfg_buf)); + memset(m_cfg_received, 0, sizeof(m_cfg_received)); break; case USB_CMD_CONFIG_DATA: if (m_cfg_receiving) { // 6 Nutzbytes ab Puffer-Offset (chunk_index × 6) eintragen - uint16_t offset = (uint16_t)pkt.key_id() * 6; - if (offset < sizeof(m_cfg_buf)) { + uint8_t chunk = pkt.key_id(); + uint16_t offset = (uint16_t)chunk * SERIAL_PAYLOAD_BYTES; + if (m_cfg_transfer_valid && + chunk < CONFIG_CHUNKS && + m_cfg_received[chunk] == 0 && + offset < sizeof(m_cfg_buf)) + { uint16_t remaining = (uint16_t)(sizeof(m_cfg_buf) - offset); - uint8_t count = (uint8_t)(remaining > 6 ? 6 : remaining); + uint8_t count = (uint8_t)( + remaining > SERIAL_PAYLOAD_BYTES + ? SERIAL_PAYLOAD_BYTES + : remaining); memcpy(m_cfg_buf + offset, &pkt.data[2], count); + m_cfg_received[chunk] = 1; + } else { + m_cfg_transfer_valid = false; } } break; @@ -237,8 +273,8 @@ void CMainController::poll_vendor() nvm_config_load(cfg); // ungültige NVM → Defaults const uint8_t* raw = reinterpret_cast(&cfg); const uint16_t sz = sizeof(SDeviceConfig); // 740 - const uint8_t payload = 6; - uint8_t chunks = (uint8_t)((sz + payload - 1) / payload); // 124 + const uint8_t payload = SERIAL_PAYLOAD_BYTES; + const uint8_t chunks = CONFIG_CHUNKS; usb_serial_send(USB_EVT_CONFIG_BEGIN, chunks); @@ -258,14 +294,17 @@ void CMainController::poll_vendor() } case USB_CMD_CONFIG_COMMIT: - if (m_cfg_receiving) { - m_cfg_receiving = false; + { + bool complete = + m_cfg_receiving && + m_cfg_transfer_valid && + all_chunks_received(m_cfg_received, CONFIG_CHUNKS); + m_cfg_receiving = false; + + if (complete) { SDeviceConfig cfg; memcpy(&cfg, m_cfg_buf, sizeof(cfg)); - if (cfg.magic == NVM_CONFIG_MAGIC && - cfg.version == NVM_CONFIG_VERSION && - cfg.crc == nvm_config_crc(cfg)) - { + if (nvm_config_validate(cfg)) { if (nvm_config_save(cfg)) { init_buttons(); usb_serial_send(USB_EVT_CONFIG_ACK, 0); // Erfolg melden @@ -277,46 +316,76 @@ void CMainController::poll_vendor() { usb_serial_send(USB_EVT_CONFIG_NACK, 0); // CRC/Magic-Fehler } + } else { + usb_serial_send(USB_EVT_CONFIG_NACK, 0); } break; + } // ── Makro-Übertragung: BEGIN → n×DATA → COMMIT ────────────────── case USB_CMD_MACRO_BEGIN: m_macro_chunks_expected = pkt.key_id(); m_macro_receiving = true; + m_macro_transfer_valid = (m_macro_chunks_expected == MACRO_CHUNKS); memset(m_macro_buf, 0, sizeof(m_macro_buf)); + memset(m_macro_received, 0, sizeof(m_macro_received)); break; case USB_CMD_MACRO_DATA: if (m_macro_receiving) { - uint16_t offset = (uint16_t)pkt.key_id() * 6; - if (offset < sizeof(m_macro_buf)) { + uint8_t chunk = pkt.key_id(); + uint16_t offset = + (uint16_t)chunk * SERIAL_PAYLOAD_BYTES; + if (m_macro_transfer_valid && + chunk < MACRO_CHUNKS && + m_macro_received[chunk] == 0 && + offset < sizeof(m_macro_buf)) + { uint16_t remaining = (uint16_t)(sizeof(m_macro_buf) - offset); - uint8_t count = (uint8_t)(remaining > 6 ? 6 : remaining); + uint8_t count = (uint8_t)( + remaining > SERIAL_PAYLOAD_BYTES + ? SERIAL_PAYLOAD_BYTES + : remaining); memcpy(m_macro_buf + offset, &pkt.data[2], count); + m_macro_received[chunk] = 1; + } else { + m_macro_transfer_valid = false; } } break; case USB_CMD_MACRO_COMMIT: - if (m_macro_receiving) { - m_macro_receiving = false; - memcpy(&m_macros, m_macro_buf, sizeof(m_macros)); - if (macro_config_save(m_macros)) { - usb_serial_send(USB_EVT_MACRO_ACK, 0); - } else { - usb_serial_send(USB_EVT_MACRO_NACK, 0); // NVM-Timeout - } + { + bool complete = + m_macro_receiving && + m_macro_transfer_valid && + all_chunks_received(m_macro_received, MACRO_CHUNKS); + m_macro_receiving = false; + + SMacroTable incoming; + if (complete) { + memcpy(&incoming, m_macro_buf, sizeof(incoming)); + } + + if (complete && + macro_config_validate(incoming) && + macro_config_save(incoming)) + { + m_macros = incoming; + usb_serial_send(USB_EVT_MACRO_ACK, 0); + } else { + usb_serial_send(USB_EVT_MACRO_NACK, 0); } break; + } // ── Makro-Dump anfordern ───────────────────────────────────────── case USB_CMD_MACRO_READ: { const uint8_t* raw = reinterpret_cast(&m_macros); const uint16_t sz = sizeof(SMacroTable); // 512 - const uint8_t payload = 6; - uint8_t chunks = (uint8_t)((sz + payload - 1) / payload); // 86 + const uint8_t payload = SERIAL_PAYLOAD_BYTES; + const uint8_t chunks = MACRO_CHUNKS; usb_serial_send(USB_EVT_MACRO_BEGIN, chunks); @@ -348,7 +417,7 @@ void CMainController::poll_vendor() // // KEY_DOWN: execute_action_down() – HID-Taste wird gedrückt, bleibt aktiv bis KEY_UP. // KEY_UP: execute_action_up() – HID-Taste wird losgelassen. -// Encoder CW/CCW: execute_action_down() + execute_action_up() für atomare TAP-Sequenz. +// Encoder CW/CCW: Host-Event mit Richtung oder HID-Tap-Sequenz. void CMainController::processEvents() { @@ -378,17 +447,15 @@ void CMainController::processEvents() case EventType::ENC_CW: if (ev.key_id < 4) { - execute_action_down(m_enc_cw[ev.key_id], ev.key_id); - delay(10); - execute_action_up(m_enc_cw[ev.key_id], ev.key_id); + execute_encoder_action( + m_enc_cw[ev.key_id], ev.key_id, USB_EVT_ENC_CW); } break; case EventType::ENC_CCW: if (ev.key_id < 4) { - execute_action_down(m_enc_ccw[ev.key_id], ev.key_id); - delay(10); - execute_action_up(m_enc_ccw[ev.key_id], ev.key_id); + execute_encoder_action( + m_enc_ccw[ev.key_id], ev.key_id, USB_EVT_ENC_CCW); } break; @@ -483,8 +550,8 @@ void CMainController::perform_factory_reset() // Laufzeit-Zustand immer an die Defaults angleichen – selbst wenn NVM gerade // nicht geschrieben werden konnte, sieht das Gerät sofort wieder "frisch" aus. m_macros = macros; - usb_hid_release_key(); - usb_hid_release_consumer(); + usb_hid_release_all_keys(); + usb_hid_release_all_consumers(); init_buttons(); show_factory_reset_feedback(); @@ -526,7 +593,7 @@ void CMainController::show_factory_reset_feedback() // execute_action_up(): Taste wird losgelassen (Hold-Ende). // HID_KEY: sendet Key-Up. // HID_CONSUMER: sendet Consumer-Up. -// HOST_COMMAND: aktuell keine Ausgabe auf Release. +// HOST_COMMAND: sendet KEY_UP mit Command-ID. // MACRO/NONE: keine Aktion. void CMainController::execute_action_down(SAction action, uint8_t key_id) @@ -551,8 +618,12 @@ void CMainController::execute_action_down(SAction action, uint8_t key_id) } case ActionType::HOST_COMMAND: - // Windows-App übernimmt Ausführung; KEY_DOWN-Event senden - usb_serial_send(USB_EVT_KEY_DOWN, key_id); + // Command-ID little-endian in Byte 2/3 übertragen. + usb_serial_send( + USB_EVT_KEY_DOWN, + key_id, + static_cast(action.data & 0xFF), + static_cast(action.data >> 8)); break; case ActionType::MACRO: @@ -566,7 +637,7 @@ void CMainController::execute_action_down(SAction action, uint8_t key_id) if (s.keycode == 0) break; usb_hid_send_key(s.keycode, s.modifier); delay(10); - usb_hid_release_key(); + usb_hid_release_key(s.keycode, s.modifier); delay(20); // Kurze Pause zwischen Steps damit der Host mitkommt } break; @@ -599,15 +670,23 @@ void CMainController::execute_action_up(SAction action, uint8_t key_id) switch (action.type) { case ActionType::HID_KEY: - usb_hid_release_key(); + { + uint8_t keycode = static_cast(action.data & 0xFF); + uint8_t modifier = static_cast(action.data >> 8); + usb_hid_release_key(keycode, modifier); break; + } case ActionType::HID_CONSUMER: - usb_hid_release_consumer(); + usb_hid_release_consumer(action.data); break; case ActionType::HOST_COMMAND: - // USB_EVT_KEY_UP ist definiert, wird aktuell aber nicht gesendet. + usb_serial_send( + USB_EVT_KEY_UP, + key_id, + static_cast(action.data & 0xFF), + static_cast(action.data >> 8)); break; case ActionType::MACRO: @@ -619,6 +698,23 @@ void CMainController::execute_action_up(SAction action, uint8_t key_id) } } +void CMainController::execute_encoder_action( + SAction action, uint8_t enc_id, uint8_t host_event) +{ + if (action.type == ActionType::HOST_COMMAND) { + usb_serial_send( + host_event, + enc_id, + static_cast(action.data & 0xFF), + static_cast(action.data >> 8)); + return; + } + + execute_action_down(action, enc_id); + delay(10); + execute_action_up(action, enc_id); +} + // ─── LED-Rendering ──────────────────────────────────────────────────────────── // // Fragt alle CButton-Instanzen ab. Jede Instanz mit dirty-Flag schreibt diff --git a/src/CMainController.h b/src/CMainController.h index 6c84236..542c6b6 100644 --- a/src/CMainController.h +++ b/src/CMainController.h @@ -41,17 +41,32 @@ private: void processEvents(); // Queue leeren, Aktionen ausführen void execute_action_down(SAction action, uint8_t key_id); // Taste drücken (Hold-Start) void execute_action_up(SAction action, uint8_t key_id); // Taste losgelassen (Hold-Ende) + void execute_encoder_action(SAction action, uint8_t enc_id, uint8_t host_event); void updateLEDs(); // Dirty-LEDs in WS2812-Buffer schreiben + enum : uint8_t { + SERIAL_PAYLOAD_BYTES = 6, + CONFIG_CHUNKS = (sizeof(SDeviceConfig) + SERIAL_PAYLOAD_BYTES - 1) / + SERIAL_PAYLOAD_BYTES, + MACRO_CHUNKS = (sizeof(SMacroTable) + SERIAL_PAYLOAD_BYTES - 1) / + SERIAL_PAYLOAD_BYTES, + }; + // ── Config-Empfangspuffer ───────────────────────────────────────────────── uint8_t m_cfg_buf[sizeof(SDeviceConfig)]; // 740 Bytes + uint8_t m_cfg_received[CONFIG_CHUNKS]; uint8_t m_cfg_chunks_expected; bool m_cfg_receiving; + bool m_cfg_transfer_valid; // ── Makro-Empfangspuffer ────────────────────────────────────────────────── uint8_t m_macro_buf[sizeof(SMacroTable)]; // 512 Bytes + uint8_t m_macro_received[MACRO_CHUNKS]; uint8_t m_macro_chunks_expected; bool m_macro_receiving; + bool m_macro_transfer_valid; + + static bool all_chunks_received(const uint8_t* received, uint8_t count); // Geladene Makro-Tabelle (im RAM – wird beim Start aus NVM geladen) SMacroTable m_macros; diff --git a/src/config/action.h b/src/config/action.h index dd880aa..975d9da 100644 --- a/src/config/action.h +++ b/src/config/action.h @@ -6,15 +6,15 @@ enum class ActionType : uint8_t NONE, // Keine Aktion HID_KEY, // Standard-Keyboard-Keycode (direkt in Firmware gesendet) HID_CONSUMER, // Consumer-Control-Keycode (Volume, Media, …) - HOST_COMMAND, // Host-Event; data wird vom aktuellen Controller nicht übertragen + HOST_COMMAND, // Host-Event; data = Command-ID für die Desktop-App MACRO, // Makro-Slot (data = Slot-Index 0–31) → bis zu 8 HID-Keys sequenziell - PROFILE_SWITCH, // Profil 0–2 oder 0xFF = nächstes Profil; speichert in NVM + PROFILE_SWITCH, // Profil 0–2 oder 0x00FF/0xFFFF = nächstes Profil; speichert in NVM }; struct __attribute__((packed)) SAction { ActionType type; - uint16_t data; // Typabhängige Nutzdaten; für HOST_COMMAND aktuell ungenutzt + uint16_t data; // Typabhängige Nutzdaten; HOST_COMMAND = Command-ID // packed: 1B type + 2B data = 3B (kein Alignment-Padding) // Muss packed sein, damit sizeof(SDeviceConfig)==740 und die // hostseitige Serialisierung bytegenau übereinstimmen. diff --git a/src/config/macro_config.cpp b/src/config/macro_config.cpp index 2d83f83..19e4c19 100644 --- a/src/config/macro_config.cpp +++ b/src/config/macro_config.cpp @@ -42,6 +42,18 @@ static bool nvm_write_page(uint32_t addr, const uint8_t* data) return nvm_exec(NVMCTRL_CTRLA_CMD_WP); } +bool macro_config_validate(const SMacroTable& tbl) +{ + for (uint8_t slot = 0; slot < MACRO_SLOTS; slot++) { + for (uint8_t step = 0; step < MACRO_MAX_STEPS; step++) { + uint8_t keycode = tbl.steps[slot][step].keycode; + if (keycode > 0x65) + return false; + } + } + return true; +} + bool macro_config_load(SMacroTable& tbl) { memcpy(&tbl, reinterpret_cast(k_macro_addr), sizeof(tbl)); @@ -56,11 +68,17 @@ bool macro_config_load(SMacroTable& tbl) memset(&tbl, 0, sizeof(tbl)); // Leere Tabelle als Default return false; } + if (!macro_config_validate(tbl)) { + memset(&tbl, 0, sizeof(tbl)); + return false; + } return true; } bool macro_config_save(const SMacroTable& tbl) { + if (!macro_config_validate(tbl)) return false; + // Auf 4-Byte-ausgerichteten Puffer kopieren bevor nvm_write_page ihn als uint32_t* liest. // SMacroTable ist __attribute__((packed)) und könnte unaligned liegen → // direkter uint32_t*-Cast würde auf Cortex-M0+ einen HardFault auslösen. diff --git a/src/config/macro_config.h b/src/config/macro_config.h index 884d4b8..af2d630 100644 --- a/src/config/macro_config.h +++ b/src/config/macro_config.h @@ -28,6 +28,13 @@ struct __attribute__((packed)) SMacroTable SMacroStep steps[MACRO_SLOTS][MACRO_MAX_STEPS]; }; +static_assert(sizeof(SMacroStep) == 2, "SMacroStep binary layout changed"); +static_assert(sizeof(SMacroTable) == 512, "SMacroTable binary layout changed"); + +// Prüft, dass alle belegten Steps in den vom HID-Descriptor unterstützten +// Keyboard-Usage-Bereich fallen. +bool macro_config_validate(const SMacroTable& tbl); + // Makro-Tabelle aus NVM lesen (Row 0+1: 0x1FB00). // Gibt false zurück wenn der Flash-Bereich noch gelöscht (0xFF) war → leere Tabelle geladen. bool macro_config_load(SMacroTable& tbl); diff --git a/src/config/nvm_config.cpp b/src/config/nvm_config.cpp index c92fa13..41baee0 100644 --- a/src/config/nvm_config.cpp +++ b/src/config/nvm_config.cpp @@ -2,6 +2,7 @@ // NVM-Zugriff für SDeviceConfig (3 Rows ab 0x1FD00, 768B gesamt, 740B genutzt). #include "nvm_config.h" +#include "macro_config.h" #include #include @@ -65,6 +66,63 @@ uint16_t nvm_config_crc(const SDeviceConfig& cfg) return crc; } +static bool action_valid(const SAction& action) +{ + switch (action.type) { + case ActionType::NONE: + return true; + + case ActionType::HID_KEY: + return static_cast(action.data & 0xFF) <= 0x65; + + case ActionType::HID_CONSUMER: + return action.data <= 0x03FF; + + case ActionType::HOST_COMMAND: + return true; + + case ActionType::MACRO: + return action.data < MACRO_SLOTS; + + case ActionType::PROFILE_SWITCH: + return action.data <= 2 || + action.data == 0x00FF || + action.data == 0xFFFF; + + default: + return false; + } +} + +bool nvm_config_validate(const SDeviceConfig& cfg) +{ + if (cfg.magic != NVM_CONFIG_MAGIC) return false; + if (cfg.version != NVM_CONFIG_VERSION) return false; + if (cfg.crc != nvm_config_crc(cfg)) return false; + if (cfg.active_profile >= 3) return false; + + for (uint8_t p = 0; p < 3; p++) { + const SDeviceProfile& prof = cfg.profiles[p]; + + for (uint8_t i = 0; i < 20; i++) { + if (!action_valid(prof.mx_actions[i])) return false; + + uint8_t anim = prof.led_anim[i]; + if (anim > 6) return false; // LEDAnim::COLOR_FADE + if (anim == 2 && prof.led_period_ms[i] < 2) return false; + } + + for (uint8_t enc = 0; enc < 4; enc++) { + for (uint8_t action = 0; action < 3; action++) { + if (!action_valid(prof.enc_actions[enc][action])) + return false; + } + } + } + + return true; +} + // ── Defaults ───────────────────────────────────────────────────────────────── void nvm_config_defaults(SDeviceConfig& cfg) @@ -112,12 +170,10 @@ bool nvm_config_load(SDeviceConfig& cfg) { memcpy(&cfg, reinterpret_cast(k_config_addr), sizeof(cfg)); - if (cfg.magic != NVM_CONFIG_MAGIC) { nvm_config_defaults(cfg); return false; } - if (cfg.version != NVM_CONFIG_VERSION) { nvm_config_defaults(cfg); return false; } - if (cfg.crc != nvm_config_crc(cfg)) { nvm_config_defaults(cfg); return false; } - - // Profil-Index absichern - if (cfg.active_profile >= 3) cfg.active_profile = 0; + if (!nvm_config_validate(cfg)) { + nvm_config_defaults(cfg); + return false; + } return true; } @@ -126,11 +182,15 @@ bool nvm_config_load(SDeviceConfig& cfg) bool nvm_config_save(const SDeviceConfig& cfg) { + SDeviceConfig stored = cfg; + stored.crc = nvm_config_crc(stored); + if (!nvm_config_validate(stored)) return false; + // Config (740B) in 768B-Puffer kopieren (3 Rows), Rest mit 0xFF füllen. // __attribute__((aligned(4))) ist zwingend: nvm_write_page castet zu uint32_t*. uint8_t row[768] __attribute__((aligned(4))); memset(row, 0xFF, sizeof(row)); - memcpy(row, &cfg, sizeof(cfg)); + memcpy(row, &stored, sizeof(stored)); NVMCTRL->CTRLB.bit.MANW = 1; diff --git a/src/config/nvm_config.h b/src/config/nvm_config.h index d8a2cd0..fb67fe4 100644 --- a/src/config/nvm_config.h +++ b/src/config/nvm_config.h @@ -61,13 +61,22 @@ struct __attribute__((packed)) SDeviceConfig // Gesamt: 32 + 708 = 740B }; +static_assert(sizeof(SAction) == 3, "SAction binary layout changed"); +static_assert(sizeof(SDeviceProfile) == 236, "SDeviceProfile binary layout changed"); +static_assert(sizeof(SDeviceConfig) == 740, "SDeviceConfig binary layout changed"); + // Standardwerte wenn keine gültige Config im NVM void nvm_config_defaults(SDeviceConfig& cfg); +// Vollständige Prüfung des persistenten/seriellen Binärvertrags inklusive CRC, +// Enum-Bereichen, Action-Nutzdaten und animationsspezifischen Mindestwerten. +bool nvm_config_validate(const SDeviceConfig& cfg); + // Config aus NVM lesen. Gibt false zurück wenn Magic/CRC/Version ungültig → Defaults geladen. bool nvm_config_load(SDeviceConfig& cfg); -// Config in NVM schreiben (löscht 3 Rows, schreibt 12 Pages). +// Config in NVM schreiben (CRC wird intern neu berechnet; löscht 3 Rows, +// schreibt 12 Pages). // Gibt false zurück wenn eine NVM-Operation nicht rechtzeitig fertig wird (Board hängt nicht). bool nvm_config_save(const SDeviceConfig& cfg); diff --git a/src/hal/usb_hid.cpp b/src/hal/usb_hid.cpp index 9dd2f6c..941346e 100644 --- a/src/hal/usb_hid.cpp +++ b/src/hal/usb_hid.cpp @@ -1,6 +1,7 @@ #include "usb_hid.h" #include #include +#include // ── HID Report Descriptor: Keyboard + Consumer Control ─────────────────────── // Host-Kommunikation außerhalb von HID läuft separat über USB CDC (SerialUSB). @@ -67,30 +68,140 @@ struct ConsumerReport { uint16_t usage; }; -void usb_hid_init() {} +static uint8_t s_key_refcount[256] = {}; +static uint8_t s_modifier_refcount[8] = {}; -void usb_hid_send_key(uint8_t keycode, uint8_t modifier) +struct ConsumerState { + uint16_t usage; + uint8_t refcount; + uint32_t order; +}; + +static constexpr uint8_t CONSUMER_STATE_SLOTS = 8; +static ConsumerState s_consumer_state[CONSUMER_STATE_SLOTS] = {}; +static uint32_t s_consumer_order = 0; + +static void send_keyboard_state() { KeyboardReport report = {}; - report.modifier = modifier; - report.keycodes[0] = keycode; + + for (uint8_t bit = 0; bit < 8; bit++) { + if (s_modifier_refcount[bit] > 0) + report.modifier |= static_cast(1u << bit); + } + + uint8_t out = 0; + for (uint16_t key = 1; key < 256 && out < 6; key++) { + if (s_key_refcount[key] > 0) + report.keycodes[out++] = static_cast(key); + } + HID().SendReport(HID_REPORT_ID_KEYBOARD, &report, sizeof(report)); } -void usb_hid_release_key() +static void send_consumer_state() { - KeyboardReport report = {}; - HID().SendReport(HID_REPORT_ID_KEYBOARD, &report, sizeof(report)); -} + uint16_t usage = 0; + uint32_t newest = 0; + + for (uint8_t i = 0; i < CONSUMER_STATE_SLOTS; i++) { + if (s_consumer_state[i].refcount > 0 && + s_consumer_state[i].order >= newest) + { + newest = s_consumer_state[i].order; + usage = s_consumer_state[i].usage; + } + } -void usb_hid_send_consumer(uint16_t usage) -{ ConsumerReport report = { usage }; HID().SendReport(HID_REPORT_ID_CONSUMER, &report, sizeof(report)); } -void usb_hid_release_consumer() +void usb_hid_init() { - ConsumerReport report = { 0 }; - HID().SendReport(HID_REPORT_ID_CONSUMER, &report, sizeof(report)); + memset(s_key_refcount, 0, sizeof(s_key_refcount)); + memset(s_modifier_refcount, 0, sizeof(s_modifier_refcount)); + memset(s_consumer_state, 0, sizeof(s_consumer_state)); + s_consumer_order = 0; +} + +void usb_hid_send_key(uint8_t keycode, uint8_t modifier) +{ + if (keycode != 0 && s_key_refcount[keycode] < 0xFF) + s_key_refcount[keycode]++; + + for (uint8_t bit = 0; bit < 8; bit++) { + if ((modifier & (1u << bit)) != 0 && s_modifier_refcount[bit] < 0xFF) + s_modifier_refcount[bit]++; + } + + send_keyboard_state(); +} + +void usb_hid_release_key(uint8_t keycode, uint8_t modifier) +{ + if (keycode != 0 && s_key_refcount[keycode] > 0) + s_key_refcount[keycode]--; + + for (uint8_t bit = 0; bit < 8; bit++) { + if ((modifier & (1u << bit)) != 0 && s_modifier_refcount[bit] > 0) + s_modifier_refcount[bit]--; + } + + send_keyboard_state(); +} + +void usb_hid_release_all_keys() +{ + memset(s_key_refcount, 0, sizeof(s_key_refcount)); + memset(s_modifier_refcount, 0, sizeof(s_modifier_refcount)); + send_keyboard_state(); +} + +void usb_hid_send_consumer(uint16_t usage) +{ + ConsumerState* free_slot = nullptr; + + for (uint8_t i = 0; i < CONSUMER_STATE_SLOTS; i++) { + ConsumerState& state = s_consumer_state[i]; + if (state.refcount > 0 && state.usage == usage) { + if (state.refcount < 0xFF) state.refcount++; + state.order = ++s_consumer_order; + send_consumer_state(); + return; + } + if (state.refcount == 0 && free_slot == nullptr) + free_slot = &state; + } + + if (free_slot != nullptr) { + free_slot->usage = usage; + free_slot->refcount = 1; + free_slot->order = ++s_consumer_order; + } + + send_consumer_state(); +} + +void usb_hid_release_consumer(uint16_t usage) +{ + for (uint8_t i = 0; i < CONSUMER_STATE_SLOTS; i++) { + ConsumerState& state = s_consumer_state[i]; + if (state.refcount > 0 && state.usage == usage) { + state.refcount--; + if (state.refcount == 0) { + state.usage = 0; + state.order = 0; + } + break; + } + } + + send_consumer_state(); +} + +void usb_hid_release_all_consumers() +{ + memset(s_consumer_state, 0, sizeof(s_consumer_state)); + send_consumer_state(); } diff --git a/src/hal/usb_hid.h b/src/hal/usb_hid.h index 3b77c3a..2e582ca 100644 --- a/src/hal/usb_hid.h +++ b/src/hal/usb_hid.h @@ -28,8 +28,14 @@ void usb_hid_init(); +// Keyboard-Zustand wird referenzgezählt. Dadurch bleiben andere gehaltene +// Tasten/Modifier aktiv, wenn genau eine Action losgelassen wird. void usb_hid_send_key(uint8_t keycode, uint8_t modifier = 0); -void usb_hid_release_key(); +void usb_hid_release_key(uint8_t keycode, uint8_t modifier = 0); +void usb_hid_release_all_keys(); +// Der Consumer-Descriptor kann jeweils ein Usage übertragen. Mehrere Holds +// werden intern verwaltet; sichtbar bleibt das zuletzt gedrückte aktive Usage. void usb_hid_send_consumer(uint16_t usage); -void usb_hid_release_consumer(); +void usb_hid_release_consumer(uint16_t usage); +void usb_hid_release_all_consumers(); diff --git a/src/hal/usb_serial.h b/src/hal/usb_serial.h index 310fcdf..e54c2ee 100644 --- a/src/hal/usb_serial.h +++ b/src/hal/usb_serial.h @@ -41,10 +41,10 @@ #define USB_CMD_MACRO_READ 0x23 // Board sendet aktuelle Makro-Tabelle zurück // ── Events: Board → PC ──────────────────────────────────────────────────────── -#define USB_EVT_KEY_DOWN 0x81 // key_id → HOST_COMMAND-Button gedrückt -#define USB_EVT_KEY_UP 0x82 // Reserviert; vom Controller aktuell nicht gesendet -#define USB_EVT_ENC_CW 0x83 // Reserviert; vom Controller aktuell nicht gesendet -#define USB_EVT_ENC_CCW 0x84 // Reserviert; vom Controller aktuell nicht gesendet +#define USB_EVT_KEY_DOWN 0x81 // key_id + Command-ID in Data[2..3] +#define USB_EVT_KEY_UP 0x82 // key_id + Command-ID in Data[2..3] +#define USB_EVT_ENC_CW 0x83 // enc_id + Command-ID in Data[2..3] +#define USB_EVT_ENC_CCW 0x84 // enc_id + Command-ID in Data[2..3] #define USB_EVT_PONG 0x85 // Antwort auf USB_CMD_PING #define USB_EVT_CONFIG_ACK 0x90 // Config erfolgreich in NVM geschrieben #define USB_EVT_CONFIG_NACK 0x91 // Config CRC/Magic ungültig – nicht geschrieben diff --git a/variants/versapad/linker_scripts/gcc/flash_without_bootloader.ld b/variants/versapad/linker_scripts/gcc/flash_without_bootloader.ld index ccef9a5..cd243b3 100644 --- a/variants/versapad/linker_scripts/gcc/flash_without_bootloader.ld +++ b/variants/versapad/linker_scripts/gcc/flash_without_bootloader.ld @@ -11,9 +11,9 @@ SEARCH_DIR(.) MEMORY { - rom (rx) : ORIGIN = 0x00000000, LENGTH = 0x0001FE00 /* 127.5K – Firmware */ - config (rx) : ORIGIN = 0x0001FE00, LENGTH = 0x00000200 /* 512B – NVM Config (2 Rows) */ - ram (rwx) : ORIGIN = 0x20000000, LENGTH = 0x00004000 /* 16K */ + rom (rx) : ORIGIN = 0x00000000, LENGTH = 0x0001FB00 /* 126.75K – Firmware */ + nvm (rx) : ORIGIN = 0x0001FB00, LENGTH = 0x00000500 /* 1.25K – Makros + Config */ + ram (rwx) : ORIGIN = 0x20000000, LENGTH = 0x00004000 /* 16K */ } /* Initial stack pointer = top of RAM */