From a706838b57294cf767880459351d40ba6bb62810 Mon Sep 17 00:00:00 2001 From: "Adrian A. Baumann" Date: Thu, 21 May 2026 21:51:27 +0200 Subject: [PATCH] docs: add Gaugecontroller v2.0 implementation plan --- .../plans/2026-05-21-gaugecontroller-v2.md | 474 ++++++++++++++++++ 1 file changed, 474 insertions(+) create mode 100644 docs/superpowers/plans/2026-05-21-gaugecontroller-v2.md diff --git a/docs/superpowers/plans/2026-05-21-gaugecontroller-v2.md b/docs/superpowers/plans/2026-05-21-gaugecontroller-v2.md new file mode 100644 index 0000000..12d88c1 --- /dev/null +++ b/docs/superpowers/plans/2026-05-21-gaugecontroller-v2.md @@ -0,0 +1,474 @@ +# Gaugecontroller v2.0 Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Centralise all gauge configuration in `gauge_config.h` and make the three inconsistently-written parsers use `sscanf` like the rest. + +**Architecture:** A new header file holds one `constexpr GaugeConfig` table — pin assignments and motion defaults merged — and derives `GAUGE_COUNT` from its length. The sketch includes this header, removes the old `GaugePins` struct and all hardcoded defaults, and initialises `Gauge` runtime state from the table in `setup()`. Three parse functions are then rewritten from manual string splitting to `sscanf`. + +**Tech Stack:** Arduino (AVR/Mega), `arduino-cli`, C++11 `constexpr`. + +> **Note on testing:** This is a bare-metal Arduino sketch with no unit-test framework. Each task's verification step is a clean compile with `arduino-cli`. Functional testing requires the physical hardware; the plan notes what to check over serial when hardware is available. + +--- + +## File Map + +| File | Action | Responsibility | +|---|---|---| +| `Gaugecontroller/gauge_config.h` | **Create** | All pin assignments and motion defaults; `GAUGE_COUNT` | +| `Gaugecontroller/Gaugecontroller.ino` | **Modify** | Remove `GaugePins`, add include, strip `Gauge` defaults, update all references, rewrite 3 parsers | + +--- + +## Task 1: Create `gauge_config.h` + +**Files:** +- Create: `Gaugecontroller/gauge_config.h` + +- [ ] **Step 1: Create the file** + + Create `Gaugecontroller/gauge_config.h` with the following content: + + ```cpp + #pragma once + #include + + struct GaugeConfig { + // Hardware + uint8_t dirPin; + uint8_t stepPin; + int8_t enablePin; // -1 = no enable pin + bool dirInverted; + bool stepActiveHigh; + bool enableActiveLow; + + // Motion defaults (integers; cast to float in setup()) + long minPos; + long maxPos; + long homingBackoffSteps; + int maxSpeed; // steps/s + int accel; // steps/s² + int homingSpeed; // steps/s + }; + + constexpr GaugeConfig gaugeConfigs[] = { + // dir step en dirInv stepHi enLow min max backoff speed accel homeSpd + { 48, 49, -1, false, true, true, 0, 3780, 3800, 4000, 6000, 500 }, + { 8, 9, -1, true, true, true, 0, 3780, 3800, 4000, 6000, 500 }, + { 52, 53, -1, false, true, true, 0, 3780, 3800, 4000, 6000, 500 }, + { 50, 51, -1, false, true, true, 0, 3780, 3800, 4000, 6000, 500 }, + }; + + static const uint8_t GAUGE_COUNT = + sizeof(gaugeConfigs) / sizeof(gaugeConfigs[0]); + ``` + + To add a fifth gauge later: append one row to `gaugeConfigs[]`. Nothing else changes. + +--- + +## Task 2: Wire `gauge_config.h` into `Gaugecontroller.ino` + +**Files:** +- Modify: `Gaugecontroller/Gaugecontroller.ino` + +This task makes six targeted edits in order. Each edit is shown as old → new. Do them top-to-bottom so line numbers don't shift unexpectedly. + +- [ ] **Step 1: Add the include** + + After the existing three `#include` lines at the top, add: + + ```cpp + #include "gauge_config.h" + ``` + + The top of the file should now read: + + ```cpp + #include + #include + #include + #include "gauge_config.h" + ``` + +- [ ] **Step 2: Remove `GAUGE_COUNT` and `GaugePins`** + + Delete these two blocks entirely (they are now in `gauge_config.h`): + + ```cpp + static const uint8_t GAUGE_COUNT = 4; + ``` + + ```cpp + struct GaugePins { + uint8_t dirPin; + uint8_t stepPin; + int8_t enablePin; // -1 means there is no enable pin + bool dirInverted; + bool stepActiveHigh; + bool enableActiveLow; + }; + + constexpr GaugePins gaugePins[GAUGE_COUNT] = { + // dir, step, en, dirInv, stepHigh, enActiveLow + {48, 49, -1, false, true, true}, // Gauge 0 + {8, 9, -1, true, true, true}, // Gauge 1 + {52, 53, -1, false, true, true}, // Gauge 2 + {50, 51, -1, false, true, true}, // Gauge 3 + }; + ``` + +- [ ] **Step 3: Strip hardcoded defaults from `struct Gauge`** + + In the `Gauge` struct definition, remove the numeric defaults from the six motion fields. Change: + + ```cpp + long minPos = 0; + long maxPos = 3780; + long homingBackoffSteps = 3800; // Deliberately a touch past full reverse travel. + + float velocity = 0.0f; + float maxSpeed = 4000.0f; + float accel = 6000.0f; + float homingSpeed = 500.0f; + ``` + + To: + + ```cpp + long minPos = 0; + long maxPos = 0; + long homingBackoffSteps = 0; + + float velocity = 0.0f; + float maxSpeed = 0.0f; + float accel = 0.0f; + float homingSpeed = 0.0f; + ``` + + These will be populated from `gaugeConfigs[i]` in `setup()` (Step 5). + +- [ ] **Step 4: Update `gaugePins` → `gaugeConfigs` references outside `setup()`** + + Three functions reference `gaugePins`. Update each one: + + **`writeDirectionPin` (~line 106):** + ```cpp + // Before + bool level = gaugePins[id].dirInverted ? !forward : forward; + + // After + bool level = gaugeConfigs[id].dirInverted ? !forward : forward; + ``` + + **`writeStepPin` (~line 111):** + ```cpp + // Before + bool level = gaugePins[id].stepActiveHigh ? active : !active; + + // After + bool level = gaugeConfigs[id].stepActiveHigh ? active : !active; + ``` + + **`configureStepperHardware` (~line 152):** + ```cpp + // Before + stepperHardware[id].stepPort = portOutputRegister(digitalPinToPort(gaugePins[id].stepPin)); + stepperHardware[id].stepMask = digitalPinToBitMask(gaugePins[id].stepPin); + stepperHardware[id].dirPort = portOutputRegister(digitalPinToPort(gaugePins[id].dirPin)); + stepperHardware[id].dirMask = digitalPinToBitMask(gaugePins[id].dirPin); + + // After + stepperHardware[id].stepPort = portOutputRegister(digitalPinToPort(gaugeConfigs[id].stepPin)); + stepperHardware[id].stepMask = digitalPinToBitMask(gaugeConfigs[id].stepPin); + stepperHardware[id].dirPort = portOutputRegister(digitalPinToPort(gaugeConfigs[id].dirPin)); + stepperHardware[id].dirMask = digitalPinToBitMask(gaugeConfigs[id].dirPin); + ``` + + **`setEnable` (~line 291):** + ```cpp + // Before + int8_t pin = gaugePins[id].enablePin; + if (pin < 0) return; + bool level = gaugePins[id].enableActiveLow ? !en : en; + + // After + int8_t pin = gaugeConfigs[id].enablePin; + if (pin < 0) return; + bool level = gaugeConfigs[id].enableActiveLow ? !en : en; + ``` + +- [ ] **Step 5: Update `setup()` — pin references and add motion default init** + + In `setup()`, the `for` loop currently reads: + + ```cpp + for (uint8_t i = 0; i < GAUGE_COUNT; i++) { + pinMode(gaugePins[i].dirPin, OUTPUT); + pinMode(gaugePins[i].stepPin, OUTPUT); + configureStepperHardware(i); + + digitalWrite(gaugePins[i].dirPin, LOW); + digitalWrite(gaugePins[i].stepPin, gaugePins[i].stepActiveHigh ? LOW : HIGH); + + if (gaugePins[i].enablePin >= 0) { + pinMode(gaugePins[i].enablePin, OUTPUT); + setEnable(i, true); + } + + gauges[i].lastUpdateMicros = micros(); + } + ``` + + Replace it with: + + ```cpp + for (uint8_t i = 0; i < GAUGE_COUNT; i++) { + pinMode(gaugeConfigs[i].dirPin, OUTPUT); + pinMode(gaugeConfigs[i].stepPin, OUTPUT); + configureStepperHardware(i); + + digitalWrite(gaugeConfigs[i].dirPin, LOW); + digitalWrite(gaugeConfigs[i].stepPin, gaugeConfigs[i].stepActiveHigh ? LOW : HIGH); + + if (gaugeConfigs[i].enablePin >= 0) { + pinMode(gaugeConfigs[i].enablePin, OUTPUT); + setEnable(i, true); + } + + gauges[i].minPos = gaugeConfigs[i].minPos; + gauges[i].maxPos = gaugeConfigs[i].maxPos; + gauges[i].homingBackoffSteps = gaugeConfigs[i].homingBackoffSteps; + gauges[i].maxSpeed = (float)gaugeConfigs[i].maxSpeed; + gauges[i].accel = (float)gaugeConfigs[i].accel; + gauges[i].homingSpeed = (float)gaugeConfigs[i].homingSpeed; + + gauges[i].lastUpdateMicros = micros(); + } + ``` + +- [ ] **Step 6: Compile and verify clean** + + ```bash + arduino-cli compile --fqbn arduino:avr:mega Gaugecontroller + ``` + + Expected: zero errors, zero warnings about `gaugePins` or `GAUGE_COUNT`. If the compiler reports "use of undeclared identifier 'gaugePins'", grep for any remaining reference: + + ```bash + grep -n "gaugePins" Gaugecontroller/Gaugecontroller.ino + ``` + + Should return nothing. + +- [ ] **Step 7: Commit** + + ```bash + git add Gaugecontroller/gauge_config.h Gaugecontroller/Gaugecontroller.ino + git commit -m "refactor: centralise gauge config in gauge_config.h" + ``` + +--- + +## Task 3: Rewrite `parseSpeed`, `parseAccel`, `parseSweep` to use `sscanf` + +**Files:** +- Modify: `Gaugecontroller/Gaugecontroller.ino` + +- [ ] **Step 1: Replace `parseSpeed`** + + Find and replace the entire `parseSpeed` function: + + ```cpp + // Before (~15 lines) + bool parseSpeed(const String& line) { + int firstSpace = line.indexOf(' '); + int secondSpace = line.indexOf(' ', firstSpace + 1); + if (firstSpace < 0 || secondSpace < 0) return false; + if (line.substring(0, firstSpace) != "SPEED") return false; + + int id = line.substring(firstSpace + 1, secondSpace).toInt(); + float speed = line.substring(secondSpace + 1).toFloat(); + + if (id < 0 || id >= GAUGE_COUNT) { + sendReply("ERR BAD_ID"); + return true; + } + if (speed <= 0.0f) { + sendReply("ERR BAD_SPEED"); + return true; + } + + gauges[id].maxSpeed = speed; + sendReply("OK"); + return true; + } + ``` + + ```cpp + // After + bool parseSpeed(const String& line) { + int id; float speed; + if (sscanf(line.c_str(), "SPEED %d %f", &id, &speed) == 2) { + if (id < 0 || id >= GAUGE_COUNT) { sendReply("ERR BAD_ID"); return true; } + if (speed <= 0.0f) { sendReply("ERR BAD_SPEED"); return true; } + gauges[id].maxSpeed = speed; + sendReply("OK"); + return true; + } + return false; + } + ``` + +- [ ] **Step 2: Replace `parseAccel`** + + ```cpp + // Before (~15 lines) + bool parseAccel(const String& line) { + int firstSpace = line.indexOf(' '); + int secondSpace = line.indexOf(' ', firstSpace + 1); + if (firstSpace < 0 || secondSpace < 0) return false; + if (line.substring(0, firstSpace) != "ACCEL") return false; + + int id = line.substring(firstSpace + 1, secondSpace).toInt(); + float accel = line.substring(secondSpace + 1).toFloat(); + + if (id < 0 || id >= GAUGE_COUNT) { + sendReply("ERR BAD_ID"); + return true; + } + if (accel <= 0.0f) { + sendReply("ERR BAD_ACCEL"); + return true; + } + + gauges[id].accel = accel; + sendReply("OK"); + return true; + } + ``` + + ```cpp + // After + bool parseAccel(const String& line) { + int id; float accel; + if (sscanf(line.c_str(), "ACCEL %d %f", &id, &accel) == 2) { + if (id < 0 || id >= GAUGE_COUNT) { sendReply("ERR BAD_ID"); return true; } + if (accel <= 0.0f) { sendReply("ERR BAD_ACCEL"); return true; } + gauges[id].accel = accel; + sendReply("OK"); + return true; + } + return false; + } + ``` + +- [ ] **Step 3: Replace `parseSweep`** + + ```cpp + // Before (~20 lines) + bool parseSweep(const String& line) { + int firstSpace = line.indexOf(' '); + int secondSpace = line.indexOf(' ', firstSpace + 1); + int thirdSpace = line.indexOf(' ', secondSpace + 1); + + if (firstSpace < 0 || secondSpace < 0 || thirdSpace < 0) return false; + if (line.substring(0, firstSpace) != "SWEEP") return false; + + int id = line.substring(firstSpace + 1, secondSpace).toInt(); + float accel = line.substring(secondSpace + 1, thirdSpace).toFloat(); + float speed = line.substring(thirdSpace + 1).toFloat(); + + if (id < 0 || id >= GAUGE_COUNT) { + sendReply("ERR BAD_ID"); + return true; + } + + Gauge& g = gauges[id]; + + if (accel <= 0.0f || speed <= 0.0f) { + g.sweepEnabled = false; + g.velocity = 0.0f; + stopTimerStepping(id); + sendReply("OK"); + return true; + } + + g.accel = accel; + g.maxSpeed = speed; + g.sweepEnabled = true; + g.sweepTowardMax = true; + atomicWriteLong(g.targetPos, g.maxPos); + sendReply("OK"); + return true; + } + ``` + + ```cpp + // After + bool parseSweep(const String& line) { + int id; float accel, speed; + if (sscanf(line.c_str(), "SWEEP %d %f %f", &id, &accel, &speed) == 3) { + if (id < 0 || id >= GAUGE_COUNT) { sendReply("ERR BAD_ID"); return true; } + Gauge& g = gauges[id]; + if (accel <= 0.0f || speed <= 0.0f) { + g.sweepEnabled = false; + g.velocity = 0.0f; + stopTimerStepping(id); + sendReply("OK"); + return true; + } + g.accel = accel; + g.maxSpeed = speed; + g.sweepEnabled = true; + g.sweepTowardMax = true; + atomicWriteLong(g.targetPos, g.maxPos); + sendReply("OK"); + return true; + } + return false; + } + ``` + +- [ ] **Step 4: Verify no `indexOf`/`substring` remain in any `parse*` function** + + ```bash + grep -n "indexOf\|substring" Gaugecontroller/Gaugecontroller.ino + ``` + + Expected: no output. If any lines appear, check which function still uses the old pattern and redo that step. + +- [ ] **Step 5: Compile and verify clean** + + ```bash + arduino-cli compile --fqbn arduino:avr:mega Gaugecontroller + ``` + + Expected: zero errors. + +- [ ] **Step 6: Commit** + + ```bash + git add Gaugecontroller/Gaugecontroller.ino + git commit -m "refactor: uniform sscanf parsing in parseSpeed, parseAccel, parseSweep" + ``` + +--- + +## Optional hardware smoke-test (when board is available) + +After uploading, send these commands over serial and confirm expected replies: + +``` +PING → PONG +HOMEALL → OK (then each gauge homes; HOMED 0..3 appear on debug port) +POS? → POS 0 0 0 1 0 0 (×4, one per gauge) +SET 0 1000 → OK +SPEED 0 2000 → OK +ACCEL 0 8000 → OK +SWEEP 0 6000 4000 → OK +SWEEP 0 0 0 → OK (stops sweep) +``` + +No new error codes were introduced; all existing commands should behave identically to v1.