You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
The Meade LX200 command parser has been fully extracted into `src/core/meade/` as a pure, host-testable, allocation-light parser with no side effects. The split is:
66
+
67
+
| Layer | Location | Status |
68
+
|-------|----------|--------|
69
+
|**Parser** (pure) |`src/core/meade/MeadeParser.*` + 11 family-specific `.cpp` files | ✅ Done — 100% covered by 13 test files in `unit_tests/test_meade/`|
|**Executor/Adapter**|`src/MeadeCommandProcessor.hpp/cpp`| ✅ Done — implements `IMeadeHandlers`, delegates to `Mount`/`LcdMenu`|
73
+
74
+
The parser directory (`src/core/meade/`) contains 16 files covering all 12 command families (Get, Set, Quit, Distance, Init, SyncControl, Home, SlewRate, GPS, Focus, Movement, Extra). Each family has:
75
+
- A typed handler interface (e.g. `IMeadeGetHandlers`, `IMeadeMovementHandlers`)
76
+
- A `handleMeade*` entry point that parses suffixes, invokes handler callbacks, and serializes `MeadeResponse`
77
+
- Value types for coordinates (e.g. `RaCoordinate`, `DecCoordinate`)
78
+
79
+
The `MeadeCommandProcessor` adapter bridges the parser to the legacy `Mount` singleton, implementing all ~90 handler overrides. It is the **only** file in `src/` root that references the core parser.
80
+
81
+
### Test coverage
82
+
83
+
13 test files in `unit_tests/test_meade/` provide comprehensive wire-byte coverage for every parser family using fake handler stubs. Tests verify:
- Parser-level validation (the `parseMeadeCommand` classifier)
88
+
89
+
### What remains
90
+
91
+
The parser is pure and tested. The executor (`MeadeCommandProcessor`) is an adapter that still couples to `Mount*` and `LcdMenu*` directly. The `Mount` god-object still contains all domain logic (slewing, tracking, guiding, parking, homing, focus, coordinate math). No HAL, ports, or controllers exist yet beyond the meade parser.
92
+
93
+
---
94
+
61
95
## Phased Plan (each phase shippable & green in CI)
62
96
63
97
### Phase 0 — Safety net & tooling (no behavior change)
@@ -70,7 +104,7 @@ Cross-cutting:
70
104
- Run `pio test -e native -v`.
71
105
- Run coverage and fail if `core/` coverage drops below configured threshold (start at 0, ratchet upward).
72
106
5. Add a tiny **Arduino host shim** under `unit_tests/test_common/arduino_shim/` providing minimal stubs (`millis`, `String`, `pinMode`, `digitalWrite`, fake `EEPROM`, fake `Serial`) for files that include `<Arduino.h>` but whose logic we want to test on host. This shim will later be replaced by proper HAL fakes under `unit_tests/test_common/hal_fakes/` (Phase 3).
73
-
6. Establish folders: `src/core/`, `src/ports/`, `src/hal/`, `src/adapters/`, `src/app/` (empty + READMEs); leave existing files in place. Host-side HAL fakes will land under `unit_tests/test_common/hal_fakes/` when Phase 3 begins.
107
+
6. Establish folders: `src/ports/`, `src/hal/`, `src/adapters/`, `src/app/` (READMEs already exist); leave existing files in place. Host-side HAL fakes will land under `unit_tests/test_common/hal_fakes/` when Phase 3 begins.
74
108
75
109
**Verify:**`pio test -e native -v` green; coverage report artifact produced in CI; build for all existing boards still green via `matrix_build.py`.
76
110
@@ -98,7 +132,7 @@ Steps (parallel after Phase 0):
98
132
3. Create `core/CalendarMath` from `Mount::getLocalDate`.
99
133
4. Create `core/CoordinateFormatter` from RA/DEC string formatters.
100
134
5. Create `core/MountGeometry` value type holding steps-per-degree, calibration angles, hemisphere, backlash — replaces scattered Mount fields used by math.
101
-
6. Create `core/MeadeParser` by splitting `MeadeCommandProcessor`: pure tokenize/dispatch lookup tables in `core/`; execution stays in adapter for now.
135
+
6.~~Create `core/MeadeParser` by splitting `MeadeCommandProcessor`: pure tokenize/dispatch lookup tables in `core/`; execution stays in adapter for now.~~**✅ DONE** — Meade parser is already in `core/meade/` with 12 family-specific handler interfaces, 13 comprehensive test files, and `MeadeCommandProcessor` as the adapter implementing `IMeadeHandlers`.
102
136
103
137
**Verify:** Phase 1 tests still green unchanged; firmware binary for each board builds identically (size diff ≈ 0); new `core/` files all covered by host tests.
104
138
@@ -163,15 +197,16 @@ Each step: extract → add focused unit tests with FFF-faked ports → remove th
163
197
164
198
**Verify:**`core/` and `ports/` contain zero `#ifdef` for features (CI grep check); all 5 existing board matrix builds still pass; binary size delta within budget (set explicit per-board limit, e.g., +3% allowed).
165
199
166
-
### Phase 6 — Meade execution layer
167
-
*Finish what Phase 2 started: separate parser from executor.*
200
+
### Phase 6 — Meade execution layer cleanup
201
+
*The parser is already in `core/`. This phase finishes the Meade slice by refining the executor and transport layers.*
168
202
169
-
1.`core/MeadeExecutor` operating on controller interfaces (no `Mount*`).
170
-
2.`adapters/SerialTransport` + `adapters/WifiTransport` feed bytes to `MeadeParser`; parsed commands dispatch to `MeadeExecutor`.
171
-
3. Remove `Mount* _mount` from `MeadeCommandProcessor` and the legacy `Mount::delay()` blocking call.
172
-
4. Add a comprehensive Meade-protocol test suite using `scripts/MeadeCommandParser.py` traces as fixtures.
203
+
1.`MeadeCommandProcessor` (current adapter) is already in `src/` root implementing `IMeadeHandlers` → move it to `src/adapters/MeadeCommandAdapter` to match layer conventions.
204
+
2. Introduce `adapters/SerialTransport` + `adapters/WifiTransport` to feed bytes to `core/meade/MeadeParser`; parsed commands dispatch to the adapter.
205
+
3. Remove `Mount* _mount` raw pointer from `MeadeCommandProcessor` — replace with port-based interfaces from Phase 3/4 (e.g. `IMeadeHandlers` implemented over controller interfaces, not the god-object).
206
+
4. Remove the legacy `Mount::delay()` blocking call from GPS acquisition handler — replace with `IClock`-based non-blocking state machine.
207
+
5. The existing 13 test files in `unit_tests/test_meade/` already cover all parser families. Add integration tests wiring `SerialTransport` → `MeadeParser` → `MeadeCommandAdapter` with faked ports.
173
208
174
-
**Verify:** Meade test suite green; Stellarium/ASCOM round-trip smoke test (manual) recorded as a regression checklist; coverage of `core/` ≥ 80%.
209
+
**Verify:** Meade test suite green (existing 13 files + new integration tests); Stellarium/ASCOM round-trip smoke test (manual) recorded as a regression checklist; coverage of `core/meade/` ≥ 80% (already achieved).
175
210
176
211
### Phase 7 — Cleanup & documentation
177
212
1. Mount facade slimmed to a thin compat shim (or removed if no external dependents).
@@ -183,18 +218,30 @@ Each step: extract → add focused unit tests with FFF-faked ports → remove th
-[`unit_tests/test_meade/`](unit_tests/test_meade/) — 13 test files covering all parser families with fake handler stubs
226
+
227
+
### Phase 0–1 targets
228
+
-[`src/Mount.cpp`](src/Mount.cpp), [`src/Mount.hpp`](src/Mount.hpp) — the god-object being decomposed; `loop()`, `calculateRAandDECSteppers()`, `guidePulse()`, `startSlewing()`, `readPersistentData()` are the biggest extraction targets.
229
+
-[`src/Sidereal.cpp`](src/Sidereal.cpp), [`src/DayTime.cpp`](src/DayTime.cpp), [`src/Declination.cpp`](src/Declination.cpp), [`src/Latitude.cpp`](src/Latitude.cpp), [`src/Longitude.cpp`](src/Longitude.cpp) — already pure; move into `core/` in Phase 2.
230
+
-[`src/EPROMStore.cpp`](src/EPROMStore.cpp), [`src/EPROMStore.hpp`](src/EPROMStore.hpp) — already a good seam; becomes `IPersistentStore` + adapter.
-[`src/MeadeCommandProcessor.cpp`](src/MeadeCommandProcessor.cpp), [`src/MeadeCommandProcessor.hpp`](src/MeadeCommandProcessor.hpp) — adapter already bridges parser to `Mount`; move to `src/adapters/` and wire through ports.
238
+
-[`src/f_serial.hpp`](src/f_serial.hpp) — serial framing code that calls `MeadeCommandProcessor::instance()->processCommand()`; becomes `SerialTransport` adapter.
187
239
188
-
-[src/Mount.cpp](../src/Mount.cpp), [src/Mount.hpp](../src/Mount.hpp) — the god-object being decomposed; `loop()`, `calculateRAandDECSteppers()`, `guidePulse()`, `startSlewing()`, `readPersistentData()` are the biggest extraction targets.
1.**C++ standard.**`core/` benefits from at least C++17 (`std::optional`, `std::variant`, `if constexpr`). PlatformIO defaults vary by board (some AVR ports stuck on C++11/14). *Recommendation:* set `build_flags = -std=gnu++17` for `native_core`; verify each board env supports it (likely yes on current toolchains) — fall back to `-std=gnu++14` + tagged unions if AVR pinches.
230
277
2.**Binary size on AVR_MEGA2560.** Polymorphism + extra indirection costs flash on AVR. *Recommendation:* keep vtables small (≤ ~12 ports), mark adapters `final`, allow link-time devirtualization. If we still bust the budget, accept template-based static dispatch for the hot path (`SlewController<RaAxis, DecAxis>`) — adds complexity but keeps AVR shipping.
231
-
3.**Interrupt-driven stepping.**`InterruptAccelStepper` mutates state from ISR context. Ports for axes need explicit thread/ISR-safety contract documented; `core/` controllers must never assume single-threaded access to axis state. *Recommendation:* document this in `ports/IStepperAxis.h`; add a `Snapshot()` method returning a consistent state read.\n
278
+
3.**Interrupt-driven stepping.**`InterruptAccelStepper` mutates state from ISR context. Ports for axes need explicit thread/ISR-safety contract documented; `core/` controllers must never assume single-threaded access to axis state. *Recommendation:* document this in `ports/IStepperAxis.h`; add a `Snapshot()` method returning a consistent state read.
0 commit comments