diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md new file mode 100644 index 00000000..c9e78a07 --- /dev/null +++ b/ARCHITECTURE.md @@ -0,0 +1,318 @@ +# Architecture + +Goal architecture for OpenAstroTracker-Firmware: a hardware-independent domain core behind +explicit ports, adapters that bind those ports to the Arduino/HAL layer, and one composition +root per board. + +See [PLAN.md](PLAN.md) for the migration plan. + +--- + +## 1. Overview + +The firmware drives an equatorial mount over a Meade LX200 serial protocol and runs on several +boards with different stepper drivers, displays and accessories. + +The design separates four concerns: + +| Concern | Owner | +|---------|-------| +| What the mount should do | `core/` — pure domain logic | +| What the domain needs from outside itself | `ports/` — domain interfaces | +| What the hardware can do | `hal/` — hardware capabilities and platform backends | +| Which combination runs on this board | `app/` — composition root | + +Adapters sit between the middle two: each adapter implements a port using one or more HAL +services, and nothing else. + +The resulting call graph is identical on every board. Board differences appear only as different +objects constructed in the composition root, never as different code paths in the domain. + +--- + +## 2. Design principles + +1. **The dependency rule is absolute.** `core/` may depend only on `ports/`. `adapters/` may depend + on `ports/` and `hal/`. `hal/` may depend on hardware libraries. `app/` may depend on everything. + Nothing points outward from an inner layer. + +2. **The domain is hardware-free.** `core/` never includes ``, never uses `String`, + `Serial`, `millis()`, `LOG()`, or a board pin header. It uses standard C++ and `etl::` where a + container is required, because the firmware builds with `-D ETL_NO_STL`. + +3. **Ports describe needs, HAL describes capabilities.** A port answers "what does the domain + want" (`IStepperAxis::moveTo`, `IClock::now`). A HAL interface answers "what can this chip do" + (`IStepperMotor::step`, `ISerialPort::read`). An adapter composes HAL services to satisfy a + port, so a new stepper library needs a new HAL backend and a new domain requirement needs a + port, never the other way round. + +4. **Configuration is read exactly once, in the composition root.** `Configuration*.hpp` macros are + consumed by a `MountConfig` builder in `app/`; every layer below receives runtime objects. + +5. **Runtime polymorphism replaces compile-time feature flags.** Optional axes (AZ, ALT, Focus) are + either constructed or replaced by a null-object controller. Driver variants are selected as an + `IMotorDriver` implementation at composition time. Feature `#ifdef`s therefore collapse into a + single tested call graph instead of 2^N build variants. + +6. **Testability is a design constraint.** If a decision cannot be exercised on the host, it belongs + in an adapter, and the decision itself belongs in `core/`. The domain core is tested with faked + ports; the target is ≥ 85% line coverage of `core/` with behavioural tests. + +7. **ISR boundaries are explicit.** The stepper interrupt loop runs in interrupt context (AVR timer + ISR, or a FreeRTOS task on ESP32). `IStepperAxis` separates ISR-safe reads (`Snapshot()`) from + main-loop-only mutation, so no controller can hide a cross-context call. + +8. **The wire protocol is the only immovable contract.** Meade LX200 behaviour is frozen + byte-for-byte so Stellarium and ASCOM keep working. Internal C++ APIs may change freely. + +9. **Respect the AVR budget.** 256 KB flash and 8 KB SRAM on the ATmega2560: keep the vtable count + small, mark adapters `final`, and allow static dispatch for hot paths if flash pressure demands + it. + +--- + +## 3. Layers + +| Layer | Directory | May depend on | Contains | Test strategy | +|-------|-----------|---------------|----------|---------------| +| **Core** | `src/core/` | `src/ports/` | Controllers (`SlewController`, `TrackingController`, `GuidingController`, `HomingController`, `ParkingController`, `FocusController`, `AzAltController`), algorithms (`SiderealClock`, `CoordinateMath`, `CalendarMath`, `CoordinateFormatter`), value types (`DayTime`, `RightAscension`, `Declination`, `Latitude`, `Longitude`), `MeadeParser`, `MountState`, `EventBus`, `MountConfig` | Host unit tests with faked ports in the `native` PIO env. No hardware, no Arduino. | +| **Ports** | `src/ports/` | nothing | `IClock`, `ILogger`, `IPersistentStore`, `IStepperAxis`, `IMotorDriver`, `IHomingSensor`, `IEndSwitch`, `IDisplay`, `IInfoDisplay`, `ITransport`, `IGps` | Interfaces only — nothing to test. | +| **Adapters** | `src/adapters/` | `src/ports/` + `src/hal/` | `ArduinoClock`, `SerialLogger`, `EepromPersistentStore`, `AccelStepperAxis`, `InterruptAccelStepperAxis`, `Tmc2209Driver`, `NullMotorDriver`, `HallHomingSensor`, `GpioEndSwitch`, `LcdMenuDisplay`, `Ssd1306InfoDisplay`, `SerialTransport`, `WifiTransport`, `TinyGpsAdapter`, `MeadeCommandAdapter` | Integration tests against a faked HAL. | +| **HAL** | `src/hal/` | hardware libraries | Interfaces (`IGpioPin`, `ISerialPort`, `ISpiBus`, `II2cBus`, `IEeprom`, `IStepperMotor`, `ITmcDriver`, `IOledPanel`, `ICharLcd`, `IButtonMatrix`, `ITimerService`, `ISystemClock`, `IWifiStack`) plus per-platform backends (`arduino/`, `avr/`, `esp32/`) | Not host-tested; verified on hardware. | +| **App** | `src/app/` | all layers | Composition root: reads `Configuration*.hpp` into `MountConfig`, selects HAL backends, constructs adapters and controllers, owns the entry point | On-device smoke tests. | + +### Dependency direction + +```mermaid +graph TD + APP["app/
composition root"] + ADAPTERS["adapters/
port -> HAL glue"] + PORTS["ports/
domain interfaces"] + CORE["core/
pure domain logic"] + HAL["hal/
hardware capabilities"] + HW["Arduino, AccelStepper, TMCStepper,
EEPROM, SSD1306, TinyGPS++"] + + APP --> ADAPTERS + APP --> HAL + APP --> PORTS + APP --> CORE + ADAPTERS --> PORTS + ADAPTERS --> HAL + CORE --> PORTS + HAL --> HW +``` + +Adapters are the only layer that touches both a port and a HAL interface; that is their entire +responsibility. `core/` never sees a HAL type, and `hal/` never sees a domain concept such as RA +or DEC. + +--- + +## 4. File layout + +Paths are relative to the repository root. + +```text +OpenAstroTracker-Firmware/ +├── ARCHITECTURE.md +├── PLAN.md +├── platformio.ini # one env per board + the `native` host-test env +├── matrix_build.py # multi-configuration firmware build driver +├── Configuration.hpp # board-independent defaults +├── Configuration_adv.hpp # advanced defaults (steps, speeds, microstepping) +├── Configuration_local_.hpp # per-board configuration +├── Constants.hpp # board IDs, feature-flag values +├── Version.h +│ +├── boards/ # per-board pin maps +│ ├── AVR_MEGA2560/pins_.h +│ ├── AVR_MKS_GEN_L_V1/ +│ ├── AVR_MKS_GEN_L_V2/ +│ ├── AVR_MKS_GEN_L_V21/ +│ ├── ESP32_ESP32DEV/ +│ ├── RAMPS/ +│ ├── RP2040_JACKW01/ +│ └── RP2040_SKR_PICO/ +│ +├── src/ +│ │ +│ ├── app/ # COMPOSITION ROOT +│ │ ├── main.cpp # entry point replacing the .ino sketch +│ │ ├── MountConfigBuilder.hpp/.cpp # Configuration*.hpp -> runtime MountConfig +│ │ └── BoardWiring.hpp/.cpp # assembles the object graph for a board +│ │ +│ ├── adapters/ # PORT -> HAL GLUE +│ │ ├── ArduinoClock.hpp/.cpp # IClock <- ISystemClock +│ │ ├── SerialLogger.hpp/.cpp # ILogger <- ISerialPort +│ │ ├── EepromPersistentStore.* # IPersistentStore <- IEeprom + EepromLayout +│ │ ├── AccelStepperAxis.* # IStepperAxis <- IStepperMotor +│ │ ├── InterruptAccelStepperAxis.* # IStepperAxis <- IStepperMotor + ITimerService +│ │ ├── Tmc2209Driver.hpp/.cpp # IMotorDriver <- ISerialPort / pins +│ │ ├── NullMotorDriver.hpp # IMotorDriver null object for non-TMC builds +│ │ ├── HallHomingSensor.hpp/.cpp # IHomingSensor <- IGpioPin +│ │ ├── GpioEndSwitch.hpp/.cpp # IEndSwitch <- IGpioPin +│ │ ├── LcdMenuDisplay.hpp/.cpp # IDisplay <- ICharLcd / IButtonMatrix +│ │ ├── Ssd1306InfoDisplay.hpp/.cpp # IInfoDisplay <- IOledPanel +│ │ ├── SerialTransport.hpp/.cpp # ITransport <- ISerialPort +│ │ ├── WifiTransport.hpp/.cpp # ITransport <- IWifiStack +│ │ ├── TinyGpsAdapter.hpp/.cpp # IGps <- serial stream + TinyGPS++ +│ │ └── MeadeCommandAdapter.hpp/.cpp # IMeadeHandlers <- core controllers +│ │ +│ ├── ports/ # DOMAIN INTERFACES +│ │ ├── IClock.hpp +│ │ ├── ILogger.hpp +│ │ ├── IPersistentStore.hpp +│ │ ├── IStepperAxis.hpp +│ │ ├── IMotorDriver.hpp +│ │ ├── IHomingSensor.hpp +│ │ ├── IEndSwitch.hpp +│ │ ├── IDisplay.hpp +│ │ ├── IInfoDisplay.hpp +│ │ ├── ITransport.hpp +│ │ └── IGps.hpp +│ │ +│ ├── hal/ # HARDWARE CAPABILITIES +│ │ ├── IGpioPin.hpp +│ │ ├── ISerialPort.hpp +│ │ ├── ISpiBus.hpp +│ │ ├── II2cBus.hpp +│ │ ├── IEeprom.hpp +│ │ ├── IStepperMotor.hpp +│ │ ├── ITmcDriver.hpp +│ │ ├── IOledPanel.hpp +│ │ ├── ICharLcd.hpp +│ │ ├── IButtonMatrix.hpp +│ │ ├── ITimerService.hpp +│ │ ├── ISystemClock.hpp +│ │ ├── IWifiStack.hpp +│ │ ├── EepromLayout.hpp # storage layout constants +│ │ ├── arduino/ # generic Arduino backends +│ │ ├── avr/ # Timer1/Timer3 ISR, fast pin I/O +│ │ └── esp32/ # hardware timers, Wi-Fi glue +│ │ +│ ├── core/ # PURE DOMAIN — no Arduino +│ │ ├── meade/ +│ │ │ ├── MeadeParser.cpp/.hpp +│ │ │ ├── MeadeProtocol.hpp +│ │ │ ├── MeadeParserHelpers.cpp/.hpp +│ │ │ └── MeadeParser{Get,Set,Quit,Distance,Init,SyncControl, +│ │ │ Home,SlewRate,Gps,Focus,Movement,Extra}.cpp +│ │ ├── types/ +│ │ │ ├── DayTime.cpp/.hpp +│ │ │ ├── Declination.cpp/.hpp +│ │ │ ├── Latitude.cpp/.hpp +│ │ │ ├── Longitude.cpp/.hpp +│ │ │ └── RightAscension.cpp/.hpp +│ │ ├── SiderealClock.cpp/.hpp # LST and hour-angle math +│ │ ├── CoordinateMath.cpp/.hpp # RA/DEC -> stepper positions +│ │ ├── CalendarMath.cpp/.hpp # date arithmetic, local -> UTC date +│ │ ├── CoordinateFormatter.cpp/.hpp # RA/DEC text formatting +│ │ ├── TrackingController.hpp/.cpp # sidereal rate, tracker-stop compensation +│ │ ├── SlewController.hpp/.cpp # slew state machine +│ │ ├── GuidingController.hpp/.cpp # guide-pulse math and timing +│ │ ├── HomingController.hpp/.cpp # hall-sensor / end-switch state machine +│ │ ├── ParkingController.hpp/.cpp # park position and transitions +│ │ ├── FocusController.hpp/.cpp # optional focus axis +│ │ ├── AzAltController.hpp/.cpp # optional AZ/ALT axes +│ │ ├── MountState.hpp/.cpp # typed mount-status enum API +│ │ ├── EventBus.hpp/.cpp # PositionChanged, SlewStarted, Parked, ... +│ │ └── MountConfig.hpp # runtime configuration struct +│ │ +│ ├── inc/ # Arduino-facing globals for app/adapters/hal +│ │ ├── Globals.hpp +│ │ └── Globals.cpp +│ ├── macros/gcc/Macros.hpp # LOG() macro and compiler pragmas +│ └── libs/ # vendored libraries +│ ├── MappedDict/ +│ └── TimerInterrupt/ +│ +├── unit_tests/ +│ ├── test_core/ # host tests for core/ (native env) +│ │ ├── test_main.cpp +│ │ ├── test_calendar.cpp +│ │ ├── test_coordinate_math.cpp +│ │ ├── test_coordinate_formatter.cpp +│ │ ├── test_sidereal.cpp +│ │ ├── types/ +│ │ │ ├── test_daytime.cpp +│ │ │ ├── test_declination.cpp +│ │ │ ├── test_latitude.cpp +│ │ │ ├── test_longitude.cpp +│ │ │ └── test_right_ascension.cpp +│ │ └── meade/ # one file per parser family + parser/helpers +│ │ └── test_Meade.cpp +│ ├── test_hal/ +│ │ └── test_main.cpp +│ └── test_common/ +│ └── hal_fakes/ # in-memory EEPROM, virtual GPIO, controllable clock +│ +└── docs/ + └── RP2040-WIRING.md +``` + +--- + +## 5. Cross-cutting rules + +| Rule | Enforcement | +|------|-------------| +| `core/` never includes ``, `String`, `Serial`, `millis()`, `LOG()`, or a board pin header | CI grep check | +| `core/` and `ports/` contain zero feature `#ifdef`s | CI grep check | +| Time comes from `IClock`; no direct `millis()` below `adapters/` | Interface contract | +| Logging goes through `ILogger`; no direct `Serial.print()` below `adapters/` | Interface contract | +| Configuration macros are read only in `app/` | CI grep check | +| Optional axes use `etl::optional` or a null object, never `#ifdef` branches | Code review | +| ISR-safe methods are named and documented on `IStepperAxis` | Interface contract | +| Board-specific code (interrupt registers, pin mappings) lives only in `hal//` | Code review | +| `Mount`, where retained for Meade back-compat, is a facade only and holds no domain logic | Code review | + +--- + +## 6. How a command flows + +```text +Serial or Wi-Fi bytes + │ + ▼ +adapters/SerialTransport ──(ITransport)──┐ +adapters/WifiTransport ──(ITransport)──┘ + │ + ▼ +core/meade/MeadeParser # pure: classifies the command, parses suffixes + │ + ▼ +IMeadeHandlers # implemented by adapters/MeadeCommandAdapter + │ + ▼ +core controllers # SlewController, TrackingController, ... + │ + ▼ +IStepperAxis # port + │ + ▼ +adapters/AccelStepperAxis # HAL: IStepperMotor + ITimerService + │ + ▼ +hal/avr | hal/esp32 | hal/arduino backends +``` + +Displays are not called by controllers. Controllers publish on `core/EventBus` and the display +adapters subscribe. + +--- + +## 7. Adding to the architecture + +**A new Meade command** — add the handler method to the relevant `IMeade*Handlers` interface in +`core/meade/MeadeParser.hpp`, implement parsing in the matching `MeadeParser*.cpp`, add a test in +`unit_tests/test_core/meade/`, then implement the method in `MeadeCommandAdapter`. The first three +steps involve no Arduino code. + +**A new domain behaviour** — put the decision in a `core/` controller, express any hardware need as +a port in `src/ports/`, and fake that port in the unit test. Never call HAL from `core/`. + +**A new peripheral** — define or reuse a HAL interface in `src/hal/`, implement it for each platform +that has the peripheral, write the adapter that satisfies the port, and construct it in `app/`. + +**A new board** — add `boards//pins_.h`, a `Configuration_local_.hpp`, a PIO +env, and a branch in the composition root. Nothing in `core/`, `ports/`, or `adapters/` changes. diff --git a/PLAN.md b/PLAN.md index ef11c483..add8252f 100644 --- a/PLAN.md +++ b/PLAN.md @@ -24,13 +24,13 @@ Approach: **hybrid** — extract pure logic into `core/` first, then test (inver | Category | Constraint | Why | |----------|-----------|-----| -| **Hardware** | Must build & run on all 5 boards (AVR_MEGA2560, MKS_GEN_L_V1/V2/V21, ESP32_ESP32DEV) | Existing user base; CI matrix enforces this. | +| **Hardware** | Must build & run on all supported boards: AVR_MEGA2560, MKS_GEN_L_V1/V2/V21, ESP32_ESP32DEV, RP2040_JACKW01, RP2040_SKR_PICO (`oaeboardv1` is also a PIO env). The CI matrix builds **7** boards | Existing user base; CI matrix enforces this. | | **Memory** | AVR_MEGA2560 has 256 KB flash, 8 KB SRAM | Flash overflow is caught at link time. Mitigation: keep vtable count ≤ ~12, mark adapters `final`, consider template-based static dispatch as escape hatch. | | **C++ standard** | C++17 for `core/` (needs `etl::optional`, `if constexpr`); boards may require C++14 fallback with ETL polyfills | `native` env already uses `-std=gnu++17`. Board envs will be audited per phase. | | **Testability** | `core/` must never include `` or use `String`, `Serial`, `millis()`, `LOG()` | Enforced by CI grep check. `native` env only compiles `core/`, `ports/`, `adapters/`. | | **Back-compat** | Meade LX200 serial protocol behavior is invariant | External interfaces (Stellarium, ASCOM) must not break. Internal C++ APIs may change freely. | | **Shippable increments** | Every phase/step must be independently shippable — all boards green, all tests green | No flag days; thin overlays preserve old call-sites during transition. | -| **Tooling** | PlatformIO + Googletest + FakeIt (bundled with ArduinoFake) + gcovr | Already configured in `platformio.ini` and CI (`ci.yml`). | +| **Tooling** | PlatformIO + Googletest + FakeIt (bundled with ArduinoFake) + gcovr | Googletest + gcovr are configured in `platformio.ini` and CI (`ci.yml`); ArduinoFake/FakeIt is still **not** wired up (Phase 0, item 5). | ### Layers & Dependencies @@ -68,7 +68,7 @@ graph TD | **ports/** | `src/ports/` | nothing | Domain interfaces: `IClock`, `ILogger`, `IPersistentStore`, `IStepperAxis`, `IMotorDriver`, `IDisplay`, `IHomingSensor`, `IEndSwitch`, `IGps`, `ITransport` | Pure abstract interfaces — no implementation to test. | | **adapters/** | `src/adapters/` | `ports/` + `hal/` | Thin glue: `AccelStepperAxis`, `EepromPersistentStore`, `SerialLogger`, `Tmc2209Driver`, `HallHomingSensor`, `SerialTransport`, `WifiTransport`, `MeadeCommandAdapter` | Integration tests: adapter → faked HAL port. | | **hal/** | `src/hal/` | hardware libraries only | Interfaces (`IGpioPin`, `ISerialPort`, `IStepperMotor`, `ITmcDriver`, `ISystemClock`, `ITimerService`, …) + per-platform backends (`hal/arduino/`, `hal/avr/`, `hal/esp32/`) | Not host-tested (wraps hardware). Verified by on-device smoke tests. | -| **app/** | `src/app/` | all layers | Composition root. Board-specific configuration comes from a header (e.g. `Configuration_local_oaeboardv1.hpp`, selected by the PIO environment) which populates a runtime `MountConfig` struct. Wires up the object graph. Replaces the legacy `.ino` entry point. | Verified by on-device smoke tests across all 5 boards. | +| **app/** | `src/app/` | all layers | Composition root. Board-specific configuration comes from a header (e.g. `Configuration_local_oaeboardv1.hpp`, selected by the PIO environment) which populates a runtime `MountConfig` struct. Wires up the object graph. Replaces the legacy `.ino` entry point. | Verified by on-device smoke tests across the CI-matrix boards. | **Why both `hal/` and `ports/`:** - `hal/` describes *what the hardware can do* (pin toggles, UART bytes, timer ticks). One backend per platform. @@ -90,7 +90,7 @@ graph TD --- -## Current State (after Meade parser migration) +## Current State (after Meade parser migration + Phase 1 extraction) ### ✅ Completed: Meade parser migrated to `core/` @@ -98,70 +98,82 @@ The Meade LX200 command parser has been fully extracted into `src/core/meade/` a | Layer | Location | Status | |-------|----------|--------| -| **Parser** (pure) | `src/core/meade/MeadeParser.*` + 11 family-specific `.cpp` files | ✅ Done — 100% covered by 13 test files in `unit_tests/test_core/meade/` | +| **Parser** (pure) | `src/core/meade/MeadeParser.*` + 12 family-specific `.cpp` files | ✅ Done — covered by 14 test files in `unit_tests/test_core/meade/`; `core/meade/` line coverage 87.0% | | **Protocol spec** | `src/core/meade/MeadeProtocol.hpp` | ✅ Done — comprehensive protocol documentation | | **Handler interfaces** | `src/core/meade/MeadeParser.hpp` (12 `IMeade*Handlers` interfaces + aggregate `IMeadeHandlers`) | ✅ Done — clean typed contracts | | **Executor/Adapter** | `src/MeadeCommandProcessor.hpp/cpp` | ✅ Done — implements `IMeadeHandlers`, delegates to `Mount`/`LcdMenu` | -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: +The parser directory (`src/core/meade/`) contains 17 files (12 family dispatchers + `MeadeParser.cpp/.hpp` + `MeadeParserHelpers.cpp/.hpp` + `MeadeProtocol.hpp`) covering all 12 command families (Get, Set, Quit, Distance, Init, SyncControl, Home, SlewRate, GPS, Focus, Movement, Extra). Each family has: - A typed handler interface (e.g. `IMeadeGetHandlers`, `IMeadeMovementHandlers`) - A `handleMeade*` entry point that parses suffixes, invokes handler callbacks, and serializes `MeadeResponse` - Value types for coordinates (e.g. `RaCoordinate`, `DecCoordinate`) -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. +The `MeadeCommandProcessor` adapter bridges the parser to the legacy `Mount` singleton, implementing all 115 handler overrides. It is the **only** file in `src/` root that references the core parser. ### Test coverage -13 test files in `unit_tests/test_core/meade/` provide comprehensive wire-byte coverage for every parser family using fake handler stubs. Tests verify: +14 test files in `unit_tests/test_core/meade/` provide comprehensive wire-byte coverage for every parser family using fake handler stubs. Line coverage of `core/meade/` is 87.0% (`MeadeParser.cpp` 43.1%, `MeadeParserExtra.cpp` 69.6%, most family dispatchers ≥ 95%). Tests verify: - Exact wire-byte formatting (zero-padding, sign rules, terminators) - Suffix classification and handler dispatch routing - Edge cases (malformed input, overflow, unknown sub-commands) - Parser-level validation (the `parseMeadeCommand` classifier) -### What remains +### ✅ Completed: Phase 1 pure-logic extraction (modules + tests) -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. +All seven Phase 1 modules exist under `src/core/`, plus `src/hal/EepromLayout.hpp`, and are covered by host tests — see the status markers on each Phase 1 step below. Pure arithmetic for `DayTime`, `Declination`, `Latitude`, `Longitude`, `RightAscension` and `Sidereal` is fully extracted, and the `src/` originals are now thin overlays that delegate to `core/`. + +**Wiring is complete (Steps 4–7):** + +1. ✅ `Mount::calculateRAandDECSteppers()` now builds a `core::SlewGeometry`, calls `core::calculateStepperPositions()`, and copies the result back. Rounding was reconciled to the legacy expressions (`long(-moveRA) * stepsPerSiderealHour`, product truncated on assignment) so the refactor is bit-for-bit identical; `SlewResult` now reports which candidate was selected (`SlewSolution`) plus the post-flip RA/DEC, so `Mount` keeps its existing `DEBUG_COORD_CALC` logs. Pinned by unit tests. +2. ✅ `Mount::getLocalDate()` now converts the elapsed day-ends into a whole-day count and calls `core::addDays()`; the inline month-length/leap-year loop is gone. +3. ✅ `Mount::RAString()`/`DECString()` call `core::formatRA()`/`core::formatDEC()`. `formatRA` gained a `bufSize` argument (it previously hard-coded `snprintf(..., 32, ...)` against `Mount`'s 24-byte `scratchBuffer`) and no longer forces `abs()`, matching the legacy `sprintf` output exactly. +4. ✅ `src/hal/EepromLayout.hpp` is now the single source of truth for magic markers, item flags, item addresses and `SteppingStorageNormalized`. It exposes them as `hal::eeprom::*` (a namespace rather than a struct, so unscoped enumerators can be brought into `EPROMStore.cpp` with one `using namespace` on pre-C++20 toolchains), `EEPROMStore`'s private aliases point at it, and `test_hal/test_eeprom_layout.cpp` pins the on-device layout on the host. + +### What remains (Phase 2 onward) + +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 ports, adapters, HAL interfaces, or controllers exist yet — `src/ports/`, `src/adapters/`, and `src/app/` contain only READMEs, and `src/hal/` contains only `EepromLayout.hpp`. --- ## Phased Plan (each phase shippable & green in CI) -### Phase 0 — Safety net & tooling (no behavior change) ✅ COMPLETE -*Foundation for everything else; must land first. Already implemented — see audit below.* +### Phase 0 — Safety net & tooling (no behavior change) ✅ COMPLETE (one open item) +*Foundation for everything else; must land first. Mostly implemented — per-item status below.* 1. ~~Add FFF as header-only dep~~ — **Superseded.** FakeIt (bundled with ArduinoFake) replaces FFF. No separate FFF needed. -2. ~~Add `native_core` PIO env~~ — **Superseded.** Only the `native` env is used. The existing `native` env already has strict warnings (`-Wall -Wextra -Werror -Wpedantic -Wshadow`). -3. ✅ gcovr-based **coverage reporting** in `native` env — **Done.** `scripts/test-coverage.py` + `--coverage` flag in `build_src_flags`. `pio run -e native -t coverage` produces HTML + markdown reports. Current: 88.3% lines, 97.0% functions, 76.5% branches. -4. ✅ CI workflow — **Done.** `.github/workflows/ci.yml` runs `pio run -e native -t coverage` (which internally executes `pio test -e native -vvv`), publishes coverage markdown to step summary. Threshold gating deferred to a later phase. -5. ✅ **ArduinoFake** as `test_lib_deps` — **Done.** Configured in `platformio.ini` `[env:native]` as `ArduinoFake@^0.4.0`. Provides stubbing/verification via FakeIt-based API for Arduino API mocking (`millis`, `String`, `pinMode`, `digitalWrite`, fake `EEPROM`, fake `Serial`, fake `Wire`, fake `SPI`). -6. ✅ Folder structure — **Done.** `src/ports/`, `src/hal/`, `src/adapters/`, `src/app/` all exist with descriptive README files. +2. ~~Add `native_core` PIO env~~ — **Superseded.** Only the `native` env is used. The existing `native` env already has strict warnings (`-Wall -Wextra -Werror -Wpedantic -Wshadow`). Note: `src/core/README.md` still mentions `native_core` and needs updating. +3. ✅ gcovr-based **coverage reporting** in `native` env — **Done.** `scripts/test-coverage.py` appends `--coverage` to `LINKFLAGS`; `build_src_flags` carries the compile-side flag. Reports (HTML + markdown) are produced by `./scripts/test-coverage.sh`. **There is no `coverage` PlatformIO target** — `pio run -e native -t coverage` fails with "unknown target". Current: 90.2% lines, 98.2% functions, 79.6% branches (366 test cases). +4. ✅ CI workflow — **Done.** `.github/workflows/ci.yml` runs `./scripts/test-coverage.sh` (which runs `pio test -e native`, then gcovr) and publishes `coverage.md` to the step summary. Threshold gating deferred to a later phase. +5. ⬜ **ArduinoFake** as `test_lib_deps` — **NOT done.** Absent from `platformio.ini` (no `test_lib_deps` key, no ArduinoFake/FakeIt dependency). `unit_tests/test_common/ArduinoFake/` holds only six `CMakeLists.txt` files — no headers or sources — and no test uses FakeIt; all host tests use plain googletest. Required before Phase 2 can add adapter tests that fake the Arduino API. +6. ✅ Folder structure — **Done.** `src/ports/`, `src/hal/`, `src/adapters/`, `src/app/` all exist with descriptive README files (they link to `specs/plan.md`, which does not exist in the repo). -**Verify:** `pio test -e native -v` → 170 tests, 0 failures; coverage report generates; all 5 board matrix builds green via `matrix_build.py`. +**Verify:** `./scripts/test-coverage.sh` → 366 test cases, 0 failures; coverage report generates; the 7-board CI matrix builds green via `matrix_build.py`. -**Changes from original plan (per feedback):** FFF replaced by FakeIt (in ArduinoFake); `native_core` env eliminated (use `native` only); coverage threshold gating deferred to a later phase. +**Changes from original plan (per feedback):** FFF replaced by FakeIt (in ArduinoFake); `native_core` env eliminated (use `native` only); coverage threshold gating deferred to a later phase. **Open item:** ArduinoFake/FakeIt is still not wired up (item 5). -### Phase 1 — Extract pure domain logic into `core/`, then test +### Phase 1 — Extract pure domain logic into `core/`, then test — ✅ COMPLETE *The original plan called for testing before extraction. That doesn't work here: every `src/` root file includes `inc/Globals.hpp` → ``, and uses `String`/`LOG()`/Arduino APIs. The `native` PIO env only builds `core/`, `ports/`, `adapters/` — not `src/` root files. **New approach: extract pure subsets into `core/` first, then write exhaustive tests.** Each step is independently shippable. No behavior change.* -**Structure:** Data containers (`DayTime`, `Declination`, `Latitude`, `Longitude`) go under `src/core/types/`. Algorithm modules (`SiderealClock`, `CoordinateMath`, `CalendarMath`, `CoordinateFormatter`) live at `src/core/` root. EEPROM layout constants (`EepromLayout`) live in `src/hal/` — they describe a storage format, not domain behavior. +**Structure:** Data containers (`DayTime`, `Declination`, `Latitude`, `Longitude`, `RightAscension`) go under `src/core/types/`. Algorithm modules (`SiderealClock`, `CoordinateMath`, `CalendarMath`, `CoordinateFormatter`) live at `src/core/` root. EEPROM layout constants (`EepromLayout`) live in `src/hal/` — they describe a storage format, not domain behavior. ``` src/core/ -├── meade/ # ✅ already extracted — parser + protocol -├── types/ # NEW — pure data containers +├── meade/ # ✅ extracted — parser + protocol +├── types/ # ✅ extracted — pure data containers │ ├── DayTime.hpp/.cpp │ ├── Declination.hpp/.cpp │ ├── Latitude.hpp/.cpp -│ └── Longitude.hpp/.cpp -├── SiderealClock.hpp/.cpp # algorithm producing DayTime outputs -├── CoordinateMath.hpp/.cpp # algorithm consuming coordinate types -├── CalendarMath.hpp/.cpp # pure date arithmetic -├── CoordinateFormatter.hpp/.cpp # char-buffer formatting +│ ├── Longitude.hpp/.cpp +│ └── RightAscension.hpp/.cpp +├── SiderealClock.hpp/.cpp # ✅ algorithm producing DayTime outputs +├── CoordinateMath.hpp/.cpp # ✅ wired into Mount::calculateRAandDECSteppers +├── CalendarMath.hpp/.cpp # ✅ wired into Mount::calculateLst + Mount::getLocalDate +└── CoordinateFormatter.hpp/.cpp # ✅ wired into Mount::RAString/DECString ``` -The `src/` originals become **thin overlays**: they `#include` the `core/` version and add only Arduino-dependent methods (`ParseFromMeade`, `ToString`, `formatString`). This preserves back-compat — no flag day. +The `src/` originals become **thin overlays**: they `#include` the `core/` version and add only Arduino-dependent methods (`ParseFromMeade`, `ToString`; `Declination` and `Longitude` also override `formatString` for their wire formats). This preserves back-compat — no flag day. **Status:** all seven steps are extracted, wired and tested — the `src/` originals no longer hold their own copies of the extracted logic. -#### Step 1: DayTime — extract pure arithmetic subset +#### Step 1: DayTime — extract pure arithmetic subset ✅ DONE *Parallel with Steps 5, 7.* **What's pure** (uses only `long` math, no Arduino types): constructors, `getHours/getMinutes/getSeconds/getTotalHours/getTotalMinutes/getTotalSeconds`, `set`, `addHours/addMinutes/addSeconds`, `addTime/subtractTime`, `sign`, `checkHours`. @@ -169,70 +181,70 @@ The `src/` originals become **thin overlays**: they `#include` the `core/` versi **What stays** (uses Arduino `String` or `LOG()`): `ParseFromMeade`, `ToString`, `formatString`. **Plan:** -1. Create `src/core/types/DayTime.hpp/.cpp` — pure header with arithmetic only. -2. Keep original `src/DayTime.hpp/cpp` as overlay: `#include "core/types/DayTime.hpp"` + Arduino-dependent methods. -3. Write `unit_tests/test_core/test_daytime.cpp` — construction, 24h wrap, negative hours, add/subtract across midnight. +1. ✅ Create `src/core/types/DayTime.hpp/.cpp` — pure header with arithmetic only. +2. ✅ Keep original `src/DayTime.hpp/cpp` as overlay: `#include "core/types/DayTime.hpp"` + Arduino-dependent methods (`ParseFromMeade`, `ToString`). +3. ✅ Write `unit_tests/test_core/types/test_daytime.cpp` — construction, 24h wrap, negative hours, add/subtract across midnight. -#### Step 2: Declination, Latitude, Longitude — extract pure subsets +#### Step 2: Declination, Latitude, Longitude, RightAscension — extract pure subsets ✅ DONE *Depends on Step 1.* -All three inherit `DayTime`. Pure parts: `checkHours()` clamping, degree conversion, constructors. What stays: `ParseFromMeade(String const&)`, `ToString()`, `formatString()`. +All four inherit `DayTime`. Pure parts: `checkHours()` clamping, degree conversion, constructors. What stays: `ParseFromMeade(String const&)`, `ToString()`, `formatString()`. **Plan:** -1. Create `src/core/types/Declination.hpp/.cpp`, `Latitude.hpp/.cpp`, `Longitude.hpp/.cpp` — each inherits `core::DayTime`. -2. Old `src/Declination.hpp` becomes: `#include "core/types/Declination.hpp"` + `ParseFromMeade`/`ToString`/`formatString` overlay. -3. Write `test_declination.cpp`, `test_latitude.cpp`, `test_longitude.cpp` — clamping edges, hemisphere handling. +1. ✅ Create `src/core/types/Declination.hpp/.cpp`, `Latitude.hpp/.cpp`, `Longitude.hpp/.cpp`, `RightAscension.hpp/.cpp` — each inherits `core::DayTime`. +2. ✅ Old `src/Declination.hpp` etc. become: `#include "core/types/.hpp"` + `ParseFromMeade`/`ToString`/`formatString` overlay. +3. ✅ Write `test_declination.cpp`, `test_latitude.cpp`, `test_longitude.cpp`, `test_right_ascension.cpp` — clamping edges, hemisphere handling, 24h wrapping. -#### Step 3: Sidereal — extract pure math, leave GPS behind +#### Step 3: Sidereal — extract pure math, leave GPS behind ✅ DONE *Depends on Step 1.* `Sidereal.cpp` is 130 lines. The GPS path (`calculateByGPS`) uses TinyGPS++. The pure math (`calculateByDateAndTime`, `calculateHa`, `calculateTheta`, `calculateDeltaJd`) uses only `double` + `DayTime`. **Plan:** -1. Create `src/core/SiderealClock.hpp/.cpp` — `calculateByDateAndTime` and `calculateHa`. -2. Keep old `src/Sidereal.cpp` with `calculateByGPS` + thin wrappers delegating to `core::SiderealClock`. -3. Create `test_core/test_sidereal.cpp` — LST/HA from known dates (equinoxes, solstices), leap years. +1. ✅ Create `src/core/SiderealClock.hpp/.cpp` — pure primitives `calculateTheta`/`calculateDeltaJd`/`calculateHaDegrees`; the `calculateByDateAndTime`/`calculateHa` entry points remain in `src/Sidereal.cpp` and compose them. +2. ✅ Keep old `src/Sidereal.cpp` with `calculateByGPS` + thin wrappers delegating to `core::SiderealClock`. +3. ✅ Create `test_core/test_sidereal.cpp` — LST/HA from known dates (equinoxes, solstices), leap years. -#### Step 4: Mount — extract `calculateRAandDECSteppers` math +#### Step 4: Mount — extract `calculateRAandDECSteppers` math ✅ DONE *Depends on Steps 1-2.* The ~200-line function computes stepper positions from RA/DEC targets. Core math is pure arithmetic on floats/longs (hemisphere, meridian flip, 3-solution selection). **Plan:** -1. Create `src/core/CoordinateMath.hpp/.cpp` with plain structs (`MountGeometry`, `EquatorialTarget`, `StepperSolution`) and free function `calculateStepperPositions()`. -2. `Mount::calculateRAandDECSteppers` becomes a thin wrapper that fills `MountGeometry` from member fields, calls the free function. -3. Write `test_core/test_coordinate_math.cpp` — parametrized over hemisphere, meridian-flip boundary, pole targets. +1. ✅ Create `src/core/CoordinateMath.hpp/.cpp` with plain structs (`SlewGeometry`, `SlewResult`) and free function `calculateStepperPositions()`. `SlewResult` returns the three candidate solutions inline rather than via the out-parameters the original plan described. +2. ✅ `Mount::calculateRAandDECSteppers()` fills a `SlewGeometry` and calls the free function. Rounding was reconciled to the legacy expressions — the candidate solutions truncate the hour delta to whole hours before scaling and the selected target scales the unrounded delta in float — so results are identical to the pre-refactor code. `SlewResult` also reports the selected candidate (`SlewSolution`) and the post-flip RA/DEC, letting `Mount` keep its `DEBUG_COORD_CALC` logs. +3. ✅ Write `test_core/test_coordinate_math.cpp` — covers both flip directions, meridian-flip boundaries, pole targets (including a flipped pole target), the reported solution, and the legacy rounding of the candidate solutions. -#### Step 5: Mount — extract calendar math +#### Step 5: Mount — extract calendar math ✅ DONE *Parallel with Steps 1, 7.* `Mount::getLocalDate()` handles year/month/day increment with month-length tables and leap-year logic. Pure integer math. **Plan:** -1. Create `src/core/CalendarMath.hpp/.cpp` with `struct CalendarDate` and `addDays()`. -2. `Mount::getLocalDate()` delegates to this. -3. Write `test_core/test_calendar.cpp` — month boundaries, leap years incl. century rules. +1. ✅ Create `src/core/CalendarMath.hpp/.cpp` with `struct CalendarDate`, `addDays()` and `localToUtcDate()`. +2. ✅ `localToUtcDate()` is used by `Mount::calculateLst`, and `Mount::getLocalDate()` now folds the elapsed day-ends into a whole-day count and delegates to `core::addDays()` — the inline month-length/leap-year loop is gone. +3. ✅ Write `test_core/test_calendar.cpp` — month boundaries, leap years incl. century rules. -#### Step 6: Mount — extract coordinate formatting +#### Step 6: Mount — extract coordinate formatting ✅ DONE *Depends on Steps 1-2.* `Mount::RAString()` and `Mount::DECString()` format RA as HH:MM:SS# and DEC as sDD*MM:SS#. Pure char-buffer manipulation. **Plan:** -1. Create `src/core/CoordinateFormatter.hpp/.cpp` with `formatRA(char*, const DayTime&)` and `formatDEC(char*, const Declination&)`. -2. Old `RAString`/`DECString` allocate `String` and call the formatter. -3. Write `test_core/test_coordinate_format.cpp` — exact wire-byte output, sign rules, zero-padding. +1. ✅ Create `src/core/CoordinateFormatter.hpp/.cpp` with `formatRA(char*, size_t, const core::DayTime&, const char*)` and `formatDEC(char*, const Declination&, const char*)`. `formatRA` takes the buffer capacity because it previously hard-coded `snprintf(..., 32, ...)` while `Mount::scratchBuffer` is only 24 bytes. +2. ✅ `Mount::RAString()`/`DECString()` call the formatter. `formatRA` no longer forces `abs()` on the hours, so output matches the legacy `sprintf` byte-for-byte (including a negative-hours edge case) and DEC still dispatches through the virtual `Declination::formatString`. +3. ✅ Write `test_core/test_coordinate_formatter.cpp` — exact wire-byte output, sign rules, zero-padding. -#### Step 7: EPROMStore — extract validation logic +#### Step 7: EPROMStore — extract validation logic ✅ DONE *Parallel with Steps 1, 5.* `EPROMStore` uses Arduino EEPROM API directly. Data-validation logic (magic markers `0xCE`/`0xCF`, struct layouts) is separable. **Plan:** -1. Create `src/hal/EepromLayout.hpp` — struct definitions and validation constants (no Arduino deps; EEPROM layout is a storage format, not domain logic). +1. ✅ `src/hal/EepromLayout.hpp` holds the layout — magic markers, item flags, extended flags, item addresses and `SteppingStorageNormalized` — with no Arduino deps. It exposes them as `hal::eeprom` (a namespace rather than a struct, so the unscoped enumerators can be pulled into `EPROMStore.cpp` with one `using namespace` on pre-C++20 toolchains instead of being re-listed). `EEPROMStore`'s private `ItemFlag`/`ExtendedItemFlag`/`ItemAddress` are aliases of it, and `unit_tests/test_hal/test_eeprom_layout.cpp` pins every address, flag mask and the store size against the shipped V1.x layout. 2. Full `IPersistentStore` port + adapter comes in Phase 2. -**Verify:** `pio test -e native -v` — all new tests pass; `pio run -e ` for all 5 boards builds green per step; `scripts/test-coverage.sh` shows ≥ 85% line coverage on new `core/` files; manual: mount slews to known coordinates on real hardware. +**Verify:** `./scripts/test-coverage.sh` — all new tests pass (366 test cases); every board env builds green via the CI matrix (`python matrix_build.py -b `; a bare `pio run -e ` needs an untracked `Configuration_local_*.hpp`); coverage shows ≥ 85% line coverage on new `core/` files (**met** — every Phase 1 module is ~100% except `core/meade/` at 87.0%); manual: mount slews to known coordinates on real hardware. ### Phase 2 — Introduce HAL, Ports & Adapters *Define the HAL surface, define the domain ports, and route current call sites through them. Keep current behavior bit-for-bit.* @@ -265,7 +277,7 @@ The ~200-line function computes stepper positions from RA/DEC targets. Core math 6. Replace direct `millis()`, `digitalWrite()`, `EEPROMStore::` calls inside `Mount` with port calls; replace `LOG()` macro with `_logger->log(...)`. 7. **ISR contract for `IStepperAxis`:** This phase MUST settle the threading model. `Mount::interruptLoop()` is called from ISR context (AVR Timer ISR / ESP32 FreeRTOS task). The `IStepperAxis` interface must specify which methods are ISR-safe and which are main-loop-only. `Snapshot()` provides an ISR-safe state read. This contract is a prerequisite for Phase 3's `SlewController`. -**Verify:** All Phase 1 tests still green; firmware builds all 5 boards (toolchain enforces flash limits). +**Verify:** All Phase 1 tests still green; firmware builds all board envs (toolchain enforces flash limits). ### Phase 3 — Decompose `Mount` into controllers *Strangler-fig: move responsibilities out of `Mount` into `core/` controllers, one at a time. Mount becomes a facade.* @@ -296,7 +308,7 @@ Each step: extract → add focused unit tests with FakeIt-faked ports → remove 4. Truly board-specific code (interrupt registers, board pins) lives **only inside the relevant `hal//` backend**; `core/`, `ports/`, and `adapters/` become `#ifdef`-free. 5. Add a `MountConfig` builder in `app/` that reads the `Configuration*.hpp` macros and produces a runtime config object. -**Verify:** `core/` and `ports/` contain zero `#ifdef` for features (CI grep check); all 5 existing board matrix builds still pass (toolchain enforces flash limits). +**Verify:** `core/` and `ports/` contain zero `#ifdef` for features (CI grep check); all board-env builds in the CI matrix still pass (toolchain enforces flash limits). ### Phase 5 — Meade execution layer cleanup *The parser is already in `core/`. This phase finishes the Meade slice by refining the executor and transport layers.* @@ -307,9 +319,9 @@ Each step: extract → add focused unit tests with FakeIt-faked ports → remove 2. Introduce `adapters/SerialTransport` + `adapters/WifiTransport` to feed bytes to `core/meade/MeadeParser`; parsed commands dispatch to the adapter. 3. Remove `Mount* _mount` raw pointer from `MeadeCommandProcessor` — replace with port-based interfaces from Phase 2/3 (e.g. `IMeadeHandlers` implemented over controller interfaces, not the god-object). 4. Remove the legacy `Mount::delay()` blocking call from GPS acquisition handler — replace with `IClock`-based non-blocking state machine. -5. The existing 13 test files in `unit_tests/test_core/meade/` already cover all parser families. Add integration tests wiring `SerialTransport` → `MeadeParser` → `MeadeCommandAdapter` with faked ports. +5. The existing 14 test files in `unit_tests/test_core/meade/` already cover all parser families. Add integration tests wiring `SerialTransport` → `MeadeParser` → `MeadeCommandAdapter` with faked ports. -**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). +**Verify:** Meade test suite green (existing 14 files + new integration tests); Stellarium/ASCOM round-trip smoke test (manual) recorded as a regression checklist; coverage of `core/meade/` ≥ 80% (already achieved — 87.0%). ### Phase 6 — Cleanup & documentation 1. Mount facade slimmed to a thin compat shim (or removed if no external dependents). @@ -324,28 +336,31 @@ Each step: extract → add focused unit tests with FakeIt-faked ports → remove ## Relevant files ### Already migrated to `core/` -- [`src/core/meade/`](src/core/meade/) — 16 files: parser, 12 family dispatchers, helpers, protocol spec, typed handler interfaces -- [`unit_tests/test_core/meade/`](unit_tests/test_core/meade/) — 13 test files covering all parser families with fake handler stubs +- [`src/core/meade/`](src/core/meade/) — 17 files: parser, 12 family dispatchers, helpers, protocol spec, typed handler interfaces +- [`unit_tests/test_core/meade/`](unit_tests/test_core/meade/) — 14 test files covering all parser families with fake handler stubs -### Phase 1 targets — extract & test -- [`src/DayTime.cpp`](src/DayTime.cpp), [`src/DayTime.hpp`](src/DayTime.hpp) — extract pure arithmetic to `core/types/DayTime` (Step 1) -- [`src/Declination.cpp`](src/Declination.cpp), [`src/Latitude.cpp`](src/Latitude.cpp), [`src/Longitude.cpp`](src/Longitude.cpp) — extract pure subsets to `core/types/` (Step 2) -- [`src/Sidereal.cpp`](src/Sidereal.cpp) — extract pure math to `core/SiderealClock` (Step 3) -- [`src/Mount.cpp`](src/Mount.cpp) — extract `calculateRAandDECSteppers` to `core/CoordinateMath` (Step 4), `getLocalDate` to `core/CalendarMath` (Step 5), `RAString`/`DECString` to `core/CoordinateFormatter` (Step 6) -- [`src/EPROMStore.cpp`](src/EPROMStore.cpp) — extract validation logic to `hal/EepromLayout.hpp` (Step 7) +### Phase 1 targets — ✅ modules, tests and wiring done +- [`src/DayTime.cpp`](src/DayTime.cpp), [`src/DayTime.hpp`](src/DayTime.hpp) — pure arithmetic in `core/types/DayTime` (Step 1, wired) +- [`src/Declination.cpp`](src/Declination.cpp), [`src/Latitude.cpp`](src/Latitude.cpp), [`src/Longitude.cpp`](src/Longitude.cpp), [`src/RightAscension.cpp`](src/RightAscension.cpp) — pure subsets in `core/types/` (Step 2, wired) +- [`src/Sidereal.cpp`](src/Sidereal.cpp) — pure math in `core/SiderealClock` (Step 3, wired) +- [`src/Mount.cpp`](src/Mount.cpp) — `calculateRAandDECSteppers` → `core/CoordinateMath` (Step 4, wired), `getLocalDate` → `core/CalendarMath::addDays` (Step 5, wired), `RAString`/`DECString` → `core/CoordinateFormatter` (Step 6, wired) +- [`src/EPROMStore.cpp`](src/EPROMStore.cpp) / [`src/EPROMStore.hpp`](src/EPROMStore.hpp) — layout constants sourced from `hal/EepromLayout.hpp` (Step 7, wired) ### New files created in Phase 1 - `src/core/types/DayTime.hpp`, `src/core/types/DayTime.cpp` - `src/core/types/Declination.hpp`, `src/core/types/Declination.cpp` - `src/core/types/Latitude.hpp`, `src/core/types/Latitude.cpp` - `src/core/types/Longitude.hpp`, `src/core/types/Longitude.cpp` +- `src/core/types/RightAscension.hpp`, `src/core/types/RightAscension.cpp` - `src/core/SiderealClock.hpp`, `src/core/SiderealClock.cpp` - `src/core/CoordinateMath.hpp`, `src/core/CoordinateMath.cpp` - `src/core/CalendarMath.hpp`, `src/core/CalendarMath.cpp` - `src/core/CoordinateFormatter.hpp`, `src/core/CoordinateFormatter.cpp` -- `src/hal/EepromLayout.hpp` -- `unit_tests/test_core/test_daytime.cpp`, `test_declination.cpp`, `test_latitude.cpp`, `test_longitude.cpp` -- `unit_tests/test_core/test_sidereal.cpp`, `test_coordinate_math.cpp`, `test_calendar.cpp`, `test_coordinate_format.cpp` +- `src/hal/EepromLayout.hpp` (definition of the on-device EEPROM layout, used by `EPROMStore`) +- `unit_tests/test_core/types/test_daytime.cpp`, `test_declination.cpp`, `test_latitude.cpp`, `test_longitude.cpp`, `test_right_ascension.cpp` +- `unit_tests/test_core/test_sidereal.cpp`, `test_coordinate_math.cpp`, `test_calendar.cpp`, `test_coordinate_formatter.cpp` +- `unit_tests/test_core/test_main.cpp` (googletest main) +- `unit_tests/test_hal/test_main.cpp` (googletest main) and `unit_tests/test_hal/test_eeprom_layout.cpp` (9 EEPROM layout tests) ### Phase 2 targets - [`src/HallSensorHoming.cpp`](src/HallSensorHoming.cpp), [`src/EndSwitches.cpp`](src/EndSwitches.cpp), [`src/Gyro.cpp`](src/Gyro.cpp), [`src/LcdMenu.cpp`](src/LcdMenu.cpp), [`src/SSD1306_128x64_Display.cpp`](src/SSD1306_128x64_Display.cpp), [`src/WifiControl.cpp`](src/WifiControl.cpp), [`src/LcdButtons.cpp`](src/LcdButtons.cpp) — become adapters behind ports. @@ -356,8 +371,8 @@ Each step: extract → add focused unit tests with FakeIt-faked ports → remove - [`src/f_serial.hpp`](src/f_serial.hpp) — serial framing code that calls `MeadeCommandProcessor::instance()->processCommand()`; becomes `SerialTransport` adapter. ### Infrastructure -- [`platformio.ini`](platformio.ini) — `native` env with coverage flags, ArduinoFake `test_lib_deps`, coverage extra script, `test_filter = test_core` (catches `test_core/meade/` too). -- [`.github/workflows/ci.yml`](.github/workflows/ci.yml) — runs `pio run -e native -t coverage` + publishes summary; builds all 5 boards. +- [`platformio.ini`](platformio.ini) — `native` env with coverage flags (`build_src_flags` + `LINKFLAGS`), coverage extra script, `test_filter = test_core` + `test_hal` (catches `test_core/meade/` too; `test_hal` holds the EEPROM layout tests). **ArduinoFake/FakeIt `test_lib_deps` is not configured yet.** +- [`.github/workflows/ci.yml`](.github/workflows/ci.yml) — runs `./scripts/test-coverage.sh` + publishes `coverage.md` to the step summary; builds 7 boards (ramps, mksgenlv1/v2/v21, esp32, rp2040_jackw01, rp2040_skr_pico). - [`unit_tests/test_common/`](unit_tests/test_common/) — expand with FakeIt-based port fakes for Phase 2+. - [`Configuration.hpp`](Configuration.hpp), [`Configuration_adv.hpp`](Configuration_adv.hpp) — read once by `MountConfig` builder in Phase 4. @@ -366,8 +381,8 @@ Each step: extract → add focused unit tests with FakeIt-faked ports → remove ## Verification strategy Automated: -1. `pio test -e native -v` — full host test suite, runs every PR. Filter: `test_core` (covers `test_core/meade/` and new `test_core/` tests). -2. `pio run -e ` for the existing 5-board matrix — must remain green every phase. +1. `./scripts/test-coverage.sh` — full host test suite, runs every PR (366 test cases). `test_filter` = `test_core` + `test_hal` (covers `test_core/meade/` and `test_core/types/`). +2. Every board env in `platformio.ini` (CI matrix: 7 boards) builds green every phase via `matrix_build.py`, which supplies the generated configuration. A bare `pio run -e ` additionally needs the developer's untracked `Configuration_local_*.hpp`. 3. Coverage gate via gcovr in CI — ratchets up phase by phase; `core/` final target ≥ 85%. 4. CI grep check: `core/` contains zero `#include ` or `#ifdef ` (after Phase 4). @@ -382,10 +397,10 @@ Manual smoke checklist (per shippable phase end): ## Decisions -- **Test stack:** **Googletest** + **FakeIt** (via ArduinoFake) for host-side unit tests. The `native` PIO env uses `test_framework = googletest`. FakeIt is bundled with ArduinoFake (`ArduinoFake@^0.4.0`) and provides the mocking/stubbing API (`When(Method(...)).Return(...)`, `Verify(Method(...))`). No Unity. No GoogleMock. +- **Test stack:** **Googletest** + **FakeIt** (via ArduinoFake) for host-side unit tests. The `native` PIO env uses `test_framework = googletest`. FakeIt is bundled with ArduinoFake (`ArduinoFake@^0.4.0`) and provides the mocking/stubbing API (`When(Method(...)).Return(...)`, `Verify(Method(...))`). No Unity. No GoogleMock. **Current state:** only Googletest is in place — ArduinoFake/FakeIt has not been added to `platformio.ini` yet (see Phase 0, item 5). - **Migration style:** **Hybrid** — extract pure logic first (Phase 1), then strangler-fig hardware-coupled layers (Phase 2–5). - **Config flags:** Migrate to **runtime polymorphism behind interfaces**; composition root reads the `Configuration*.hpp` macros once. `core/` becomes `#ifdef`-free for features. -- **Optional axes:** Use `etl::optional` (from Embedded Template Library) — not `std::optional`. The firmware builds with `-D ETL_NO_STL` on AVR targets. `core/` uses `etl::optional` for consistency across all build targets. The `native` test env adds ETL as a dependency. +- **Optional axes:** Use `etl::optional` (from Embedded Template Library) — not `std::optional`. The firmware builds with `-D ETL_NO_STL` on AVR targets. `core/` uses `etl::optional` for consistency across all build targets. **Open:** ETL is not currently a `lib_deps` entry anywhere (`src/libs/` holds only `MappedDict` and `TimerInterrupt`) and nothing uses `etl::` yet — ETL must be added to both the board and `native` envs before Phase 4 relies on it. - **Mount global migration:** `Mount` is currently a global variable (`extern Mount mount;` in `a_inits.hpp`). Call sites that use `mount.xxx()` will be migrated using the thin-overlay pattern: existing code delegates through the global during transition; when a module becomes a proper adapter (Phase 2+), it receives injected references instead. No flag day needed. - **ISR threading model:** Deferred to Phase 2. The `IStepperAxis` interface contract (which methods are ISR-safe, `Snapshot()` semantics) will be settled before Phase 3 begins extracting controllers. Without this contract, `SlewController`'s threading model is undefined. - **Back-compat:** Meade serial protocol behavior is invariant (external interface); internal C++ APIs may change freely. diff --git a/src/EPROMStore.cpp b/src/EPROMStore.cpp index b4c2108d..609f5124 100644 --- a/src/EPROMStore.cpp +++ b/src/EPROMStore.cpp @@ -8,16 +8,16 @@ POP_NO_WARNINGS #include "EPROMStore.hpp" // The platform-independant EEPROM class - -// Steps/deg are normalized to this value and stored. -const float SteppingStorageNormalized = 25600.0; +// +// The on-device layout (addresses, flags, magic markers) is defined in hal::eeprom. +using namespace hal::eeprom; /////////////////////////////////////// // PLATFORM-SPECIFIC IMPLEMENTATIONS #if USE_DUMMY_EEPROM == true -static uint8_t dummyEepromStorage[EEPROMStore::STORE_SIZE]; +static uint8_t dummyEepromStorage[STORE_SIZE]; // Initialize the EEPROM object for ESP boards, setting aside storage void EEPROMStore::initialize() diff --git a/src/EPROMStore.hpp b/src/EPROMStore.hpp index 11acd189..9dbb9052 100644 --- a/src/EPROMStore.hpp +++ b/src/EPROMStore.hpp @@ -3,6 +3,7 @@ #include "DayTime.hpp" #include "Latitude.hpp" #include "Longitude.hpp" +#include "hal/EepromLayout.hpp" // Platform independant abstraction of the EEPROM storage capability of the boards. // This is needed because the ESP boards require two things that the Arduino boards don't: @@ -75,160 +76,12 @@ class EEPROMStore static void storeALTPosition(int32_t altPosition); private: - ///////////////////////////////// - // - // EEPROM storage location 5 ("magic marker") must be 0xCE or 0xCF for the mount to read any data. - // The lowest bit of location 5 indicates if extended flags are available in (21/22). - // - // Location 4 indicates what has been stored so far: 0000 0000 - // ^^^^ ^^^^ - // |||| |||| - // Roll angle offset (19/20) -----+||| |||| - // Pitch angle offset (17/18) ------+|| |||| - // Longitude (14/15) -------+| |||| - // Latitude (12/13) --------+ |||| - // Backlash steps (10/11) ----------+||| - // Speed factor (0/3) -----------+|| - // DEC stepper motor steps per degree (8/9) ------------+| - // RA stepper motor steps per degree (6/7) -------------+ - // - // Locations for HATime (1/2), Brightness (16) - // - // If Location 5 is 0xCF, then an extended 16-bit flag is stored in 21/22 and - // indicates the additional fields that have been stored: 0000 0000 0000 0000 - // ^^^^ ^^^^ ^^^^ ^^^^ - // |||| |||| |||| - // ALT Steps/deg, normalized to 256MS (70-73) -------------+||| |||| |||| - // AZ Steps/deg, normalized to 256MS (66-69) --------------+|| |||| |||| - // ALT position (62-65) ---------------+| |||| |||| - // AZ Position (58-61) ----------------+ |||| |||| - // Last flashed version (56-57) ------------------+||| |||| - // DEC Homing Offet (52-55) -------------------+|| |||| - // DEC Steps/deg, normalized to 256MS (48-51) --------------------+| |||| - // RA Steps/deg, normalized to 256MS (44-47) ---------------------+ |||| - // RA Homing Offet (40-43) -----------------------+||| - // UTC Offset (39) ------------------------+|| - // DEC lower (31-34) and upper (35-38) limits -------------------------+| - // RA (23-26) and DEC (27-30) Parking offsets --------------------------+ ( ==== Obsolete V1.13.0 and beyond ==== ) - // - ///////////////////////////////// - - enum ItemFlag - { - MAGIC_MARKER_VALUE - = 0xCE00, // Changed to 0xCxxx in V1.8.60 since we changed RA and DEC Steps to be 10x (previous settings ignored) - MAGIC_MARKER_MASK = 0xFE00, // If these bits are set to MAGIC_MARKER_VALUE, something has been written to the EEPROM - - // The marker bits for the first 8 values stored in EEPROM. - RA_STEPS_FLAG = 0x0001, - DEC_STEPS_FLAG = 0x0002, - SPEED_FACTOR_FLAG = 0x0004, - BACKLASH_STEPS_FLAG = 0x0008, - LATITUDE_FLAG = 0x0010, - LONGITUDE_FLAG = 0x0020, - PITCH_OFFSET_FLAG = 0x0040, - ROLL_OFFSET_FLAG = 0x0080, - EXTENDED_FLAG = 0x0100, - FLAGS_MASK = 0x01FF // The bitwise OR of all preceding values - }; - - enum ExtendedItemFlag - { - // The marker bits for the extended values - PARKING_POS_MARKER_FLAG = 0x0001, - DEC_LIMIT_MARKER_FLAG = 0x0002, - UTC_OFFSET_MARKER_FLAG = 0x0004, - RA_HOMING_MARKER_FLAG = 0x0008, - RA_NORM_STEPS_MARKER_FLAG = 0x0010, - DEC_NORM_STEPS_MARKER_FLAG = 0x0020, - DEC_HOMING_MARKER_FLAG = 0x0040, - LAST_FLASHED_MARKER_FLAG = 0x0080, - AZ_POSITION_MARKER_FLAG = 0x0100, - ALT_POSITION_MARKER_FLAG = 0x0200, - AZ_NORM_STEPS_MARKER_FLAG = 0x0400, - ALT_NORM_STEPS_MARKER_FLAG = 0x0800, - }; - - // These are the offsets to each item stored in the EEPROM - enum ItemAddress - { - SPEED_FACTOR_LOW_ADDR = 0, - SPEED_FACTOR_HIGH_ADDR = 3, // Split as two discontinuous Uint8 - HA_HOUR_ADDR = 1, - HA_MINUTE_ADDR = 2, // Both Uint8 - FLAGS_ADDR = 4, // Uint8 - MAGIC_MARKER_ADDR = 5, // Uint8 - MAGIC_MARKER_AND_FLAGS_ADDR = 4, // Alias for Uint16 access - RA_STEPS_DEGREE_ADDR = 6, - _RA_STEPS_DEGREE_ADDR_1 = 7, // Int16 - DEC_STEPS_DEGREE_ADDR = 8, - _DEC_STEPS_DEGREE_ADDR_1 = 9, // Int16 - BACKLASH_STEPS_ADDR = 10, - _BACKLASH_STEPS_ADDR_1 = 11, // Int16 - LATITUDE_ADDR = 12, - _LATITUDE_ADDR_1 = 13, // Int16 - LONGITUDE_ADDR = 14, - _LONGITUDE_ADDR_1 = 15, // Int16 - LCD_BRIGHTNESS_ADDR = 16, // Uint8 - PITCH_OFFSET_ADDR = 17, - _PITCH_OFFSET_ADDR_1 = 18, // Uint16 - ROLL_OFFSET_ADDR = 19, - _ROLL_OFFSET_ADDR_1 = 20, // Uint16 - EXTENDED_FLAGS_ADDR = 21, - _EXTENDED_FLAGS_ADDR_1 = 22, // Uint16 - RA_PARKING_POS_ADDR = 23, - _RA_PARKING_POS_ADDR_1, - _RA_PARKING_POS_ADDR_2, - _RA_PARKING_POS_ADDR_3, // Int32 - DEC_PARKING_POS_ADDR = 27, - _DEC_PARKING_POS_ADDR_1, - _DEC_PARKING_POS_ADDR_2, - _DEC_PARKING_POS_ADDR_3, // Int32 - DEC_LOWER_LIMIT_ADDR = 31, - _DEC_LOWER_LIMIT_ADDR_1, - _DEC_LOWER_LIMIT_ADDR_2, - _DEC_LOWER_LIMIT_ADDR_3, // Int32 - DEC_UPPER_LIMIT_ADDR = 35, - _DEC_UPPER_LIMIT_ADDR_1, - _DEC_UPPER_LIMIT_ADDR_2, - _DEC_UPPER_LIMIT_ADDR_3, // Int32 - UTC_OFFSET_ADDR = 39, // Int8 - RA_HOMING_OFFSET_ADDR = 40, - _RA_HOMING_OFFSET_ADDR_1, - _RA_HOMING_OFFSET_ADDR_2, - _RA_HOMING_OFFSET_ADDR_3, // Int32 - RA_NORM_STEPS_DEGREE_ADDR = 44, - _RA_NORM_STEPS_DEGREE_ADDR_1, - _RA_NORM_STEPS_DEGREE_ADDR_2, - _RA_NORM_STEPS_DEGREE_ADDR_3, // Int32 - DEC_NORM_STEPS_DEGREE_ADDR = 48, - _DEC_NORM_STEPS_DEGREE_ADDR_1, - _DEC_NORM_STEPS_DEGREE_ADDR_2, - _DEC_NORM_STEPS_DEGREE_ADDR_3, // Int32 - DEC_HOMING_OFFSET_ADDR = 52, - _DEC_HOMING_OFFSET_ADDR_1, - _DEC_HOMING_OFFSET_ADDR_2, - _DEC_HOMING_OFFSET_ADDR_3, // Int32 - LAST_FLASHED_VERSION = 56, - _LAST_FLASHED_VERSION_1, - AZ_POSITION_ADDR = 58, - _AZ_POSITION_ADDR_1, - _AZ_POSITION_ADDR_2, - _AZ_POSITION_ADDR_3, - ALT_POSITION_ADDR = 62, - _ALT_POSITION_ADDR_1, - _ALT_POSITION_ADDR_2, - _ALT_POSITION_ADDR_3, - AZ_NORM_STEPS_DEGREE_ADDR = 66, - _AZ_NORM_STEPS_DEGREE_ADDR_1, - _AZ_NORM_STEPS_DEGREE_ADDR_2, - _AZ_NORM_STEPS_DEGREE_ADDR_3, // Int32 - ALT_NORM_STEPS_DEGREE_ADDR = 70, - _ALT_NORM_STEPS_DEGREE_ADDR_1, - _ALT_NORM_STEPS_DEGREE_ADDR_2, - _ALT_NORM_STEPS_DEGREE_ADDR_3, // Int32 - STORE_SIZE = 74 - }; + // The storage format itself (magic markers, flags, addresses) lives in + // hal::eeprom (src/hal/EepromLayout.hpp) so it can be tested without any + // Arduino dependencies. + using ItemFlag = hal::eeprom::ItemFlag; + using ExtendedItemFlag = hal::eeprom::ExtendedItemFlag; + using ItemAddress = hal::eeprom::ItemAddress; // Helper functions static void displayContents(); diff --git a/src/Mount.cpp b/src/Mount.cpp index db551328..cf2ee581 100644 --- a/src/Mount.cpp +++ b/src/Mount.cpp @@ -7,6 +7,8 @@ #include "Mount.hpp" #include "Sidereal.hpp" #include "core/CalendarMath.hpp" +#include "core/CoordinateFormatter.hpp" +#include "core/CoordinateMath.hpp" #include "libs/MappedDict/MappedDict.hpp" PUSH_NO_WARNINGS @@ -3709,64 +3711,64 @@ void Mount::calculateRAandDECSteppers(long &targetRASteps, long &targetDECSteps, float const RALimitR = inNorthernHemisphere ? RA_LIMIT_RIGHT : RA_LIMIT_LEFT; LOG(DEBUG_COORD_CALC, "[MOUNT]: CalcSteppersIn: Limits are : %f to %f", RALimitL, RALimitR); + core::SlewGeometry geometry; + geometry.moveRA = moveRA; + geometry.moveDEC = moveDEC; + geometry.homeTargetDeltaRA = homeTargetDeltaRA; + geometry.stepsPerSiderealHour = stepsPerSiderealHour; + geometry.stepsPerDECDegree = _stepsPerDECDegree; + geometry.zeroPosDEC = _zeroPosDEC; + geometry.raLimitL = RALimitL; + geometry.raLimitR = RALimitR; + + const core::SlewResult result = core::calculateStepperPositions(geometry); + if (pSolutions != nullptr) { - pSolutions[0] = long(-moveRA) * stepsPerSiderealHour; - pSolutions[1] = long(moveDEC) * _stepsPerDECDegree; - pSolutions[2] = long(-(moveRA - 12.0f)) * stepsPerSiderealHour; - pSolutions[3] = long(-moveDEC) * _stepsPerDECDegree; - pSolutions[4] = long(-(moveRA + 12.0f)) * stepsPerSiderealHour; - pSolutions[5] = long(-moveDEC) * _stepsPerDECDegree; + for (int i = 0; i < 6; i++) + { + pSolutions[i] = result.solutions[i]; + } } LOG(DEBUG_COORD_CALC, "[MOUNT]: CalcSteppersIn: Solution 1: %f, %f", -moveRA, moveDEC); LOG(DEBUG_COORD_CALC, "[MOUNT]: CalcSteppersIn: Solution 2: %f, %f", -(moveRA - 12.0f), -moveDEC); LOG(DEBUG_COORD_CALC, "[MOUNT]: CalcSteppersIn: Solution 3: %f, %f", -(moveRA + 12.0f), -moveDEC); - // If we reach the limit in the positive direction ... - if (homeTargetDeltaRA > RALimitR) - { - LOG(DEBUG_COORD_CALC, - "[MOUNT]: CalcSteppersIn: Using Solution 2, since hometargetDeltaRA %f (RA:%f) is past max limit %f, inverting both axes", - homeTargetDeltaRA, - moveRA, - RALimitR); - - // ... turn both RA and DEC axis around - moveRA -= 12.0f; - moveDEC = -moveDEC; - LOG(DEBUG_COORD_CALC, "[MOUNT]: CalcSteppersIn: Adjusted Target. RA: %f, DEC: %f", moveRA, moveDEC); - } - // If we reach the limit in the negative direction... - else if (homeTargetDeltaRA < RALimitL) + switch (result.selected) { - LOG(DEBUG_COORD_CALC, - "[MOUNT]: CalcSteppersIn: Using solution 3 since homeTargetDeltaRA %f (RA:%f) is past min limit: %f, inverting both axes", - homeTargetDeltaRA, - moveRA, - RALimitL); - // ... turn both RA and DEC axis around - - moveRA += 12.0f; - moveDEC = -moveDEC; - LOG(DEBUG_COORD_CALC, "[MOUNT]: CalcSteppersIn: Adjusted Target. RA: %f, DEC: %f", moveRA, moveDEC); - } - else - { - LOG(DEBUG_COORD_CALC, - "[MOUNT]: CalcSteppersIn: Using solution 1 since targetRA %f is in range. RA: %f, DEC: %f", - homeTargetDeltaRA, - moveRA, - moveDEC); + case core::SlewSolution::FlipUpper: + LOG(DEBUG_COORD_CALC, + "[MOUNT]: CalcSteppersIn: Using Solution 2, since hometargetDeltaRA %f (RA:%f) is past max limit %f, inverting both axes", + homeTargetDeltaRA, + moveRA, + RALimitR); + break; + case core::SlewSolution::FlipLower: + LOG(DEBUG_COORD_CALC, + "[MOUNT]: CalcSteppersIn: Using solution 3 since homeTargetDeltaRA %f (RA:%f) is past min limit: %f, inverting both axes", + homeTargetDeltaRA, + moveRA, + RALimitL); + break; + case core::SlewSolution::Direct: + LOG(DEBUG_COORD_CALC, + "[MOUNT]: CalcSteppersIn: Using solution 1 since targetRA %f is in range. RA: %f, DEC: %f", + homeTargetDeltaRA, + moveRA, + moveDEC); + break; } - // zeroPosDEC will be zero unless one or more Sync commands have moved it, in which case it is the - // accumulated offset from zero (home) in degrees. - moveDEC -= _zeroPosDEC; - LOG(DEBUG_COORD_CALC, "[MOUNT]: CalcSteppersIn: adjusted DEC by _zeroPosDEC: %f => DEC: %f", _zeroPosDEC, moveDEC); + // zeroPosDEC is the accumulated offset from zero (home) in degrees that one or more Sync commands have applied. + LOG(DEBUG_COORD_CALC, "[MOUNT]: CalcSteppersIn: Adjusted Target. RA: %f, DEC: %f", result.adjustedRA, result.adjustedDEC); + LOG(DEBUG_COORD_CALC, + "[MOUNT]: CalcSteppersIn: adjusted DEC by _zeroPosDEC: %f => DEC: %f", + _zeroPosDEC, + result.adjustedDEC - _zeroPosDEC); - targetRASteps = -moveRA * stepsPerSiderealHour; - targetDECSteps = moveDEC * _stepsPerDECDegree; + targetRASteps = result.targetRASteps; + targetDECSteps = result.targetDECSteps; LOG(DEBUG_COORD_CALC, "[MOUNT]: CalcSteppersPost: ResultTarget Steps RA: %l, DEC: %l", targetRASteps, targetDECSteps); } @@ -4066,7 +4068,7 @@ String Mount::DECString(byte type, byte active) dec = currentDEC(); } - dec.formatString(scratchBuffer, formatStringsDEC[type & FORMAT_STRING_MASK]); + core::formatDEC(scratchBuffer, dec, formatStringsDEC[type & FORMAT_STRING_MASK]); if ((type & FORMAT_STRING_MASK) == LCDMENU_STRING) { @@ -4094,7 +4096,7 @@ String Mount::RAString(byte type, byte active) ra = currentRA(); } - sprintf(scratchBuffer, formatStringsRA[type & FORMAT_STRING_MASK], ra.getHours(), ra.getMinutes(), ra.getSeconds()); + core::formatRA(scratchBuffer, sizeof(scratchBuffer), ra, formatStringsRA[type & FORMAT_STRING_MASK]); if ((type & FORMAT_STRING_MASK) == LCDMENU_STRING) { scratchBuffer[active * 4] = '>'; @@ -4136,46 +4138,13 @@ LocalDate Mount::getLocalDate() LocalDate localDate = _localStartDate; long secondsSinceSetDayEnd = ((millis() - _localStartTimeSetMillis) / 1000) - (86400 - _localStartTime.getTotalSeconds()); - while (secondsSinceSetDayEnd >= 0) - { - localDate.day++; - secondsSinceSetDayEnd -= 86400; - - int maxDays = 31; - switch (localDate.month) - { - case 2: - if (((localDate.year % 4 == 0) && (localDate.year % 100 != 0)) || (localDate.year % 400 == 0)) - { - maxDays = 29; - } - else - { - maxDays = 28; - } - break; - - case 4: - case 6: - case 9: - case 11: - maxDays = 30; - break; - } + // Number of day ends passed since the date was set; a partial day still counts as one. + const long daysElapsed = secondsSinceSetDayEnd >= 0 ? (secondsSinceSetDayEnd / 86400) + 1 : 0; - //calculate day overflow - if (localDate.day > maxDays) - { - localDate.day = 1; - localDate.month++; - } - //calculate year overflow - if (localDate.month > 12) - { - localDate.month = 1; - localDate.year++; - } - } + const core::CalendarDate rolled = core::addDays({localDate.year, localDate.month, localDate.day}, daysElapsed); + localDate.year = rolled.year; + localDate.month = rolled.month; + localDate.day = rolled.day; return localDate; } diff --git a/src/core/CoordinateFormatter.cpp b/src/core/CoordinateFormatter.cpp index a7af38db..e8bc8422 100644 --- a/src/core/CoordinateFormatter.cpp +++ b/src/core/CoordinateFormatter.cpp @@ -1,18 +1,16 @@ #include "CoordinateFormatter.hpp" -#include "types/RightAscension.hpp" +#include "types/DayTime.hpp" #include "types/Declination.hpp" #include namespace core { -void formatRA(char *buf, const RightAscension &ra, const char *fmt) +void formatRA(char *buf, size_t bufSize, const DayTime &ra, const char *fmt) { int h, m, s; ra.getTime(h, m, s); - // RA is always non-negative in display (0–24h range) - int absHours = abs(h); - snprintf(buf, 32, fmt, absHours, m, s); + snprintf(buf, bufSize, fmt, h, m, s); } void formatDEC(char *buf, const Declination &dec, const char *fmt) diff --git a/src/core/CoordinateFormatter.hpp b/src/core/CoordinateFormatter.hpp index 92d17949..208314cc 100644 --- a/src/core/CoordinateFormatter.hpp +++ b/src/core/CoordinateFormatter.hpp @@ -1,22 +1,24 @@ #pragma once +#include + namespace core { /// Pure coordinate formatting to char buffers (no Arduino deps). -/// Formats RA as HH:MM:SS and DEC using the formatString mini-language. -class RightAscension; // forward -class Declination; // forward +class DayTime; // forward +class Declination; // forward /// Format RA (hours component) using a printf-style format string. -/// @param buf output buffer (must be large enough) +/// @param buf output buffer +/// @param bufSize capacity of buf in bytes; the result is truncated to fit /// @param ra RA value (hours, minutes, seconds) /// @param fmt printf format with 3 %d placeholders for h, m, s -void formatRA(char *buf, const RightAscension &ra, const char *fmt); +void formatRA(char *buf, size_t bufSize, const DayTime &ra, const char *fmt); /// Format DEC using the formatString mini-language ({d}, {m}, {s}, {+}). -/// @param buf output buffer +/// @param buf output buffer, which must be large enough for the result /// @param dec DEC value /// @param fmt formatString format (e.g. "{d}*{m}'{s}#") void formatDEC(char *buf, const Declination &dec, const char *fmt); diff --git a/src/core/CoordinateMath.cpp b/src/core/CoordinateMath.cpp index 179eb812..1998fcf4 100644 --- a/src/core/CoordinateMath.cpp +++ b/src/core/CoordinateMath.cpp @@ -10,29 +10,38 @@ SlewResult calculateStepperPositions(const SlewGeometry &geo) float moveRA = geo.moveRA; float moveDEC = geo.moveDEC; - // Pre-compute all three candidate solutions - result.solutions[0] = static_cast(-moveRA) * static_cast(geo.stepsPerSiderealHour); - result.solutions[1] = static_cast(moveDEC) * static_cast(geo.stepsPerDECDegree); - result.solutions[2] = static_cast(-(moveRA - 12.0f)) * static_cast(geo.stepsPerSiderealHour); - result.solutions[3] = static_cast(-moveDEC) * static_cast(geo.stepsPerDECDegree); - result.solutions[4] = static_cast(-(moveRA + 12.0f)) * static_cast(geo.stepsPerSiderealHour); - result.solutions[5] = static_cast(-moveDEC) * static_cast(geo.stepsPerDECDegree); + // Pre-compute all three candidate solutions. The RA candidates truncate the + // hour delta to whole hours before scaling, which is the rounding the mount + // has always used for this diagnostic output; only the selected target below + // is scaled from the unrounded delta. + result.solutions[0] = static_cast(static_cast(-moveRA) * geo.stepsPerSiderealHour); + result.solutions[1] = static_cast(static_cast(moveDEC) * geo.stepsPerDECDegree); + result.solutions[2] = static_cast(static_cast(-(moveRA - 12.0f)) * geo.stepsPerSiderealHour); + result.solutions[3] = static_cast(static_cast(-moveDEC) * geo.stepsPerDECDegree); + result.solutions[4] = static_cast(static_cast(-(moveRA + 12.0f)) * geo.stepsPerSiderealHour); + result.solutions[5] = static_cast(static_cast(-moveDEC) * geo.stepsPerDECDegree); // Select the appropriate solution based on RA limits + result.selected = SlewSolution::Direct; if (geo.homeTargetDeltaRA > geo.raLimitR) { // Past upper limit: flip both axes (Solution 2) moveRA -= 12.0f; - moveDEC = -moveDEC; + moveDEC = -moveDEC; + result.selected = SlewSolution::FlipUpper; } else if (geo.homeTargetDeltaRA < geo.raLimitL) { // Past lower limit: flip both axes (Solution 3) moveRA += 12.0f; - moveDEC = -moveDEC; + moveDEC = -moveDEC; + result.selected = SlewSolution::FlipLower; } // else: Solution 1 (direct, no flip) + result.adjustedRA = moveRA; + result.adjustedDEC = moveDEC; + // Apply DEC zero offset moveDEC -= geo.zeroPosDEC; diff --git a/src/core/CoordinateMath.hpp b/src/core/CoordinateMath.hpp index a5767774..5eaf0382 100644 --- a/src/core/CoordinateMath.hpp +++ b/src/core/CoordinateMath.hpp @@ -15,6 +15,14 @@ struct SlewGeometry { float raLimitR; /// Upper RA limit (hours, positive) }; +/// The candidate the selector picked. +enum class SlewSolution +{ + Direct, ///< Solution 1: target is within the RA limits, no flip + FlipUpper, ///< Solution 2: target past the upper RA limit, both axes inverted + FlipLower, ///< Solution 3: target past the lower RA limit, both axes inverted +}; + /// Result of the slew calculation. struct SlewResult { long targetRASteps; @@ -22,6 +30,14 @@ struct SlewResult { /// Three candidate solutions (6 values: RA,DEC, RA,DEC, RA,DEC) long solutions[6]; + + /// Which candidate was selected. + SlewSolution selected; + + /// RA (hours) and DEC (degrees) after any meridian flip but before the + /// accumulated sync offset (zeroPosDEC) is applied. + float adjustedRA; + float adjustedDEC; }; /// Calculate RA and DEC stepper target positions given the geometry. diff --git a/src/hal/EepromLayout.hpp b/src/hal/EepromLayout.hpp index a373ad67..435d446a 100644 --- a/src/hal/EepromLayout.hpp +++ b/src/hal/EepromLayout.hpp @@ -5,50 +5,168 @@ namespace hal { -/// EEPROM layout constants — magic markers, flags, and storage addresses. -/// Mirrors the enum definitions in EPROMStore.hpp for validation and testing. -struct EepromLayout { - // Magic marker values - static constexpr uint16_t MAGIC_MARKER_VALUE = 0xCE00; - static constexpr uint16_t MAGIC_MARKER_MASK = 0xFE00; - - // Storage size (bytes) - static constexpr int STORE_SIZE = 512; // Atmel: no explicit size needed; ESP32: 512 bytes +/// On-device EEPROM layout: magic markers, item flags and item addresses. +/// +/// This header describes a storage format, not domain behaviour, and has no +/// Arduino or firmware dependencies so it can be unit-tested on the host. +/// EEPROMStore is the only reader/writer of this layout today; Phase 2 wraps +/// it in an IPersistentStore port + adapter. +/// +/// EEPROM storage location 5 ("magic marker") must be 0xCE or 0xCF for the +/// mount to read any data. The lowest bit of location 5 indicates whether the +/// extended flags are available in (21/22). +/// +/// Location 4 indicates what has been stored so far: 0000 0000 +/// ^^^^ ^^^^ +/// |||| |||| +/// Roll angle offset (19/20) -----+||| |||| +/// Pitch angle offset (17/18) ------+|| |||| +/// Longitude (14/15) -------+| |||| +/// Latitude (12/13) --------+ |||| +/// Backlash steps (10/11) ----------+||| +/// Speed factor (0/3) -----------+|| +/// DEC stepper motor steps per degree (8/9) ------------+| +/// RA stepper motor steps per degree (6/7) -------------+ +/// +/// Locations for HATime (1/2) and Brightness (16). +/// +/// If location 5 is 0xCF, then an extended 16-bit flag is stored in 21/22 and +/// indicates the additional fields that have been stored: +/// 0000 0000 0000 0000 +/// ^^^^ ^^^^ ^^^^ ^^^^ +/// |||| |||| |||| +/// ALT Steps/deg, normalized to 256MS (70-73) -------------+||| |||| |||| +/// AZ Steps/deg, normalized to 256MS (66-69) --------------+|| |||| |||| +/// ALT position (62-65) ---------------+| |||| |||| +/// AZ Position (58-61) ----------------+ |||| |||| +/// Last flashed version (56-57) ------------------+||| |||| +/// DEC Homing Offset (52-55) -------------------+|| |||| +/// DEC Steps/deg, normalized to 256MS (48-51) --------------------+| |||| +/// RA Steps/deg, normalized to 256MS (44-47) ---------------------+ |||| +/// RA Homing Offset (40-43) -----------------------+||| +/// UTC Offset (39) ------------------------+|| +/// DEC lower (31-34) and upper (35-38) limits -------------------------+| +/// RA (23-26) and DEC (27-30) Parking offsets --------------------------+ (==== Obsolete V1.13.0 and beyond ====) +namespace eeprom +{ +enum ItemFlag : uint16_t +{ + MAGIC_MARKER_VALUE = 0xCE00, // Changed to 0xCxxx in V1.8.60 since we changed RA and DEC Steps to be 10x (previous settings ignored) + MAGIC_MARKER_MASK = 0xFE00, // If these bits are set to MAGIC_MARKER_VALUE, something has been written to the EEPROM - // Item flags (bitfield indicating which fields have been stored) - enum ItemFlag : uint16_t - { - RA_STEPS_FLAG = 0x0001, - DEC_STEPS_FLAG = 0x0002, - SPEED_FACTOR_FLAG = 0x0004, - BACKLASH_STEPS_FLAG = 0x0008, - LATITUDE_FLAG = 0x0010, - LONGITUDE_FLAG = 0x0020, - PITCH_OFFSET_FLAG = 0x0040, - ROLL_OFFSET_FLAG = 0x0080, - EXTENDED_FLAG = 0x0100, - FLAGS_MASK = 0x01FF - }; + // The marker bits for the first 8 values stored in EEPROM. + RA_STEPS_FLAG = 0x0001, + DEC_STEPS_FLAG = 0x0002, + SPEED_FACTOR_FLAG = 0x0004, + BACKLASH_STEPS_FLAG = 0x0008, + LATITUDE_FLAG = 0x0010, + LONGITUDE_FLAG = 0x0020, + PITCH_OFFSET_FLAG = 0x0040, + ROLL_OFFSET_FLAG = 0x0080, + EXTENDED_FLAG = 0x0100, + FLAGS_MASK = 0x01FF // The bitwise OR of all preceding values +}; - // Extended item flags - enum ExtendedItemFlag : uint16_t - { - PARKING_POS_MARKER_FLAG = 0x0001, - DEC_LIMIT_MARKER_FLAG = 0x0002, - UTC_OFFSET_MARKER_FLAG = 0x0004, - RA_HOMING_MARKER_FLAG = 0x0008, - RA_NORM_STEPS_MARKER_FLAG = 0x0010, - DEC_NORM_STEPS_MARKER_FLAG = 0x0020, - DEC_HOMING_MARKER_FLAG = 0x0040, - LAST_FLASHED_MARKER_FLAG = 0x0080, - AZ_POSITION_MARKER_FLAG = 0x0100, - ALT_POSITION_MARKER_FLAG = 0x0200, - AZ_NORM_STEPS_MARKER_FLAG = 0x0400, - ALT_NORM_STEPS_MARKER_FLAG = 0x0800, - }; +enum ExtendedItemFlag : uint16_t +{ + // The marker bits for the extended values + PARKING_POS_MARKER_FLAG = 0x0001, + DEC_LIMIT_MARKER_FLAG = 0x0002, + UTC_OFFSET_MARKER_FLAG = 0x0004, + RA_HOMING_MARKER_FLAG = 0x0008, + RA_NORM_STEPS_MARKER_FLAG = 0x0010, + DEC_NORM_STEPS_MARKER_FLAG = 0x0020, + DEC_HOMING_MARKER_FLAG = 0x0040, + LAST_FLASHED_MARKER_FLAG = 0x0080, + AZ_POSITION_MARKER_FLAG = 0x0100, + ALT_POSITION_MARKER_FLAG = 0x0200, + AZ_NORM_STEPS_MARKER_FLAG = 0x0400, + ALT_NORM_STEPS_MARKER_FLAG = 0x0800, + EXTENDED_FLAGS_MASK = 0x0FFF // The bitwise OR of all preceding values +}; - // Normalization factor for steps-per-degree storage - static constexpr float SteppingStorageNormalized = 25600.0; +// These are the offsets to each item stored in the EEPROM +enum ItemAddress : uint8_t +{ + SPEED_FACTOR_LOW_ADDR = 0, + HA_HOUR_ADDR = 1, + HA_MINUTE_ADDR = 2, // Both Uint8 + SPEED_FACTOR_HIGH_ADDR = 3, // Split as two discontinuous Uint8 + FLAGS_ADDR = 4, // Uint8 + MAGIC_MARKER_ADDR = 5, // Uint8 + MAGIC_MARKER_AND_FLAGS_ADDR = 4, // Alias for Uint16 access + RA_STEPS_DEGREE_ADDR = 6, + _RA_STEPS_DEGREE_ADDR_1 = 7, // Int16 + DEC_STEPS_DEGREE_ADDR = 8, + _DEC_STEPS_DEGREE_ADDR_1 = 9, // Int16 + BACKLASH_STEPS_ADDR = 10, + _BACKLASH_STEPS_ADDR_1 = 11, // Int16 + LATITUDE_ADDR = 12, + _LATITUDE_ADDR_1 = 13, // Int16 + LONGITUDE_ADDR = 14, + _LONGITUDE_ADDR_1 = 15, // Int16 + LCD_BRIGHTNESS_ADDR = 16, // Uint8 + PITCH_OFFSET_ADDR = 17, + _PITCH_OFFSET_ADDR_1 = 18, // Uint16 + ROLL_OFFSET_ADDR = 19, + _ROLL_OFFSET_ADDR_1 = 20, // Uint16 + EXTENDED_FLAGS_ADDR = 21, + _EXTENDED_FLAGS_ADDR_1 = 22, // Uint16 + RA_PARKING_POS_ADDR = 23, + _RA_PARKING_POS_ADDR_1 = 24, + _RA_PARKING_POS_ADDR_2 = 25, + _RA_PARKING_POS_ADDR_3 = 26, // Int32 + DEC_PARKING_POS_ADDR = 27, + _DEC_PARKING_POS_ADDR_1 = 28, + _DEC_PARKING_POS_ADDR_2 = 29, + _DEC_PARKING_POS_ADDR_3 = 30, // Int32 + DEC_LOWER_LIMIT_ADDR = 31, + _DEC_LOWER_LIMIT_ADDR_1 = 32, + _DEC_LOWER_LIMIT_ADDR_2 = 33, + _DEC_LOWER_LIMIT_ADDR_3 = 34, // Int32 + DEC_UPPER_LIMIT_ADDR = 35, + _DEC_UPPER_LIMIT_ADDR_1 = 36, + _DEC_UPPER_LIMIT_ADDR_2 = 37, + _DEC_UPPER_LIMIT_ADDR_3 = 38, // Int32 + UTC_OFFSET_ADDR = 39, // Int8 + RA_HOMING_OFFSET_ADDR = 40, + _RA_HOMING_OFFSET_ADDR_1 = 41, + _RA_HOMING_OFFSET_ADDR_2 = 42, + _RA_HOMING_OFFSET_ADDR_3 = 43, // Int32 + RA_NORM_STEPS_DEGREE_ADDR = 44, + _RA_NORM_STEPS_DEGREE_ADDR_1 = 45, + _RA_NORM_STEPS_DEGREE_ADDR_2 = 46, + _RA_NORM_STEPS_DEGREE_ADDR_3 = 47, // Int32 + DEC_NORM_STEPS_DEGREE_ADDR = 48, + _DEC_NORM_STEPS_DEGREE_ADDR_1 = 49, + _DEC_NORM_STEPS_DEGREE_ADDR_2 = 50, + _DEC_NORM_STEPS_DEGREE_ADDR_3 = 51, // Int32 + DEC_HOMING_OFFSET_ADDR = 52, + _DEC_HOMING_OFFSET_ADDR_1 = 53, + _DEC_HOMING_OFFSET_ADDR_2 = 54, + _DEC_HOMING_OFFSET_ADDR_3 = 55, // Int32 + LAST_FLASHED_VERSION = 56, + _LAST_FLASHED_VERSION_1 = 57, + AZ_POSITION_ADDR = 58, + _AZ_POSITION_ADDR_1 = 59, + _AZ_POSITION_ADDR_2 = 60, + _AZ_POSITION_ADDR_3 = 61, + ALT_POSITION_ADDR = 62, + _ALT_POSITION_ADDR_1 = 63, + _ALT_POSITION_ADDR_2 = 64, + _ALT_POSITION_ADDR_3 = 65, + AZ_NORM_STEPS_DEGREE_ADDR = 66, + _AZ_NORM_STEPS_DEGREE_ADDR_1 = 67, + _AZ_NORM_STEPS_DEGREE_ADDR_2 = 68, + _AZ_NORM_STEPS_DEGREE_ADDR_3 = 69, // Int32 + ALT_NORM_STEPS_DEGREE_ADDR = 70, + _ALT_NORM_STEPS_DEGREE_ADDR_1 = 71, + _ALT_NORM_STEPS_DEGREE_ADDR_2 = 72, + _ALT_NORM_STEPS_DEGREE_ADDR_3 = 73, // Int32 + STORE_SIZE = 74 }; +// Steps/deg are normalized to this value and stored. +constexpr float SteppingStorageNormalized = 25600.0f; +} // namespace eeprom } // namespace hal diff --git a/unit_tests/test_core/test_coordinate_formatter.cpp b/unit_tests/test_core/test_coordinate_formatter.cpp index 3796681e..c5980b6e 100644 --- a/unit_tests/test_core/test_coordinate_formatter.cpp +++ b/unit_tests/test_core/test_coordinate_formatter.cpp @@ -11,7 +11,7 @@ TEST(CoordinateFormatterTest, FormatRABasic) { RightAscension ra(12, 34, 56); char buf[32]; - core::formatRA(buf, ra, "%02d:%02d:%02d#"); + core::formatRA(buf, sizeof(buf), ra, "%02d:%02d:%02d#"); EXPECT_STREQ("12:34:56#", buf); } @@ -19,7 +19,7 @@ TEST(CoordinateFormatterTest, FormatRAPadded) { RightAscension ra(5, 7, 9); char buf[32]; - core::formatRA(buf, ra, "%02d:%02d:%02d#"); + core::formatRA(buf, sizeof(buf), ra, "%02d:%02d:%02d#"); EXPECT_STREQ("05:07:09#", buf); } @@ -27,7 +27,7 @@ TEST(CoordinateFormatterTest, FormatRALcdStyle) { RightAscension ra(23, 59, 59); char buf[32]; - core::formatRA(buf, ra, " %02dh %02dm %02ds"); + core::formatRA(buf, sizeof(buf), ra, " %02dh %02dm %02ds"); EXPECT_STREQ(" 23h 59m 59s", buf); } @@ -54,3 +54,20 @@ TEST(CoordinateFormatterTest, FormatDECCompact) core::formatDEC(buf, dec, "{d}{m}{s}"); EXPECT_STREQ("+053000", buf); } + +TEST(CoordinateFormatterTest, FormatRATruncatesToBuffer) +{ + RightAscension ra(12, 34, 56); + char buf[8]; + core::formatRA(buf, sizeof(buf), ra, "%02d:%02d:%02d#"); + EXPECT_STREQ("12:34:5", buf); +} + +TEST(CoordinateFormatterTest, FormatRAFitsMountScratchBuffer) +{ + // Mount::scratchBuffer is 24 bytes; the longest RA format must fit. + RightAscension ra(23, 59, 59); + char buf[24]; + core::formatRA(buf, sizeof(buf), ra, " %02dh %02dm %02ds"); + EXPECT_STREQ(" 23h 59m 59s", buf); +} diff --git a/unit_tests/test_core/test_coordinate_math.cpp b/unit_tests/test_core/test_coordinate_math.cpp index 4ff249c2..eb5d2c0f 100644 --- a/unit_tests/test_core/test_coordinate_math.cpp +++ b/unit_tests/test_core/test_coordinate_math.cpp @@ -89,3 +89,85 @@ TEST(CoordinateMathTest, BoundaryExactlyAtLimit) EXPECT_EQ(static_cast(-3.0f * 1000.0f), result.targetRASteps); EXPECT_EQ(static_cast(30.0f * 500.0f), result.targetDECSteps); } + +TEST(CoordinateMathTest, NorthPoleTarget) +{ + SlewGeometry geo = makeGeo(3.0f, 90.0f, 2.0f); + SlewResult result = calculateStepperPositions(geo); + + EXPECT_EQ(45000L, result.targetDECSteps); + EXPECT_FLOAT_EQ(90.0f, result.adjustedDEC); +} + +TEST(CoordinateMathTest, SouthPoleTarget) +{ + SlewGeometry geo = makeGeo(3.0f, -90.0f, 2.0f); + SlewResult result = calculateStepperPositions(geo); + + EXPECT_EQ(-45000L, result.targetDECSteps); + EXPECT_FLOAT_EQ(-90.0f, result.adjustedDEC); +} + +TEST(CoordinateMathTest, PoleTargetWithMeridianFlip) +{ + // Past the upper RA limit the DEC axis is inverted, pole targets included + SlewGeometry geo = makeGeo(4.0f, 90.0f, 7.0f); + SlewResult result = calculateStepperPositions(geo); + + EXPECT_EQ(-45000L, result.targetDECSteps); + EXPECT_FLOAT_EQ(-90.0f, result.adjustedDEC); +} + +TEST(CoordinateMathTest, ReportsSelectedSolution) +{ + EXPECT_EQ(core::SlewSolution::Direct, calculateStepperPositions(makeGeo(3.0f, 45.0f, 2.0f)).selected); + EXPECT_EQ(core::SlewSolution::FlipUpper, calculateStepperPositions(makeGeo(4.0f, 30.0f, 7.0f)).selected); + EXPECT_EQ(core::SlewSolution::FlipLower, calculateStepperPositions(makeGeo(-4.0f, 20.0f, -7.0f)).selected); + EXPECT_EQ(core::SlewSolution::Direct, calculateStepperPositions(makeGeo(3.0f, 30.0f, 6.0f)).selected); +} + +TEST(CoordinateMathTest, ReportsAdjustedCoordinatesBeforeZeroOffset) +{ + // FlipUpper: RA -12, DEC negated; adjusted values exclude the zeroPosDEC offset + SlewGeometry geo = makeGeo(4.0f, 30.0f, 7.0f); + geo.zeroPosDEC = 5.0f; + SlewResult result = calculateStepperPositions(geo); + + EXPECT_FLOAT_EQ(-8.0f, result.adjustedRA); + EXPECT_FLOAT_EQ(-30.0f, result.adjustedDEC); + // The offset still lands in the target + EXPECT_EQ(static_cast((-30.0f - 5.0f) * 500.0f), result.targetDECSteps); + + // Direct: unchanged + SlewResult direct = calculateStepperPositions(makeGeo(3.0f, 45.0f, 2.0f)); + EXPECT_FLOAT_EQ(3.0f, direct.adjustedRA); + EXPECT_FLOAT_EQ(45.0f, direct.adjustedDEC); +} + +TEST(CoordinateMathTest, CandidateSolutionsUseWholeRoundedHours) +{ + // The candidates truncate RA to whole hours before scaling (legacy behaviour), + // while the selected target scales the unrounded delta. + SlewGeometry geo = makeGeo(2.5f, 45.5f, 1.0f); + SlewResult result = calculateStepperPositions(geo); + + // -2 (not -2.5) hours * 1000 = -2000 + EXPECT_EQ(-2000L, result.solutions[0]); + // 45 (not 45.5) degrees * 500 = 22500 + EXPECT_EQ(22500L, result.solutions[1]); + // The real target keeps the fraction + EXPECT_EQ(-2500L, result.targetRASteps); + EXPECT_EQ(22750L, result.targetDECSteps); +} + +TEST(CoordinateMathTest, CandidateSolutionsScaleBeforeTruncating) +{ + // A non-integral steps-per-hour value is multiplied in float, not truncated first + SlewGeometry geo = makeGeo(3.0f, 10.0f, 0.0f); + geo.stepsPerSiderealHour = 1000.5f; + geo.stepsPerDECDegree = 500.5f; + SlewResult result = calculateStepperPositions(geo); + + EXPECT_EQ(-3001L, result.solutions[0]); // long(-3 * 1000.5) == -3001 + EXPECT_EQ(5005L, result.solutions[1]); // long(10 * 500.5) == 5005 +} diff --git a/unit_tests/test_core/test_sidereal.cpp b/unit_tests/test_core/test_sidereal.cpp index 06c475f7..fd1d86ae 100644 --- a/unit_tests/test_core/test_sidereal.cpp +++ b/unit_tests/test_core/test_sidereal.cpp @@ -4,6 +4,15 @@ using core::SiderealClock; +// Compose the two core primitives exactly as Sidereal::calculateByDateAndTime +// does in the firmware, and return local sidereal time in hours. +static double lstHours(int year, int month, int day, float utcHours, double longitude) +{ + const int deltaJd = SiderealClock::calculateDeltaJd(year, month, day); + const double deltaJ = deltaJd + (utcHours / 24.0); + return SiderealClock::calculateTheta(deltaJ, longitude, utcHours) / 15.0; +} + // Known-good values sourced from stargazing.net/kepler/altaz.html TEST(SiderealClockTest, DeltaJdJ2000) { @@ -34,6 +43,44 @@ TEST(SiderealClockTest, DeltaJdMonotonic) EXPECT_LT(d1, d2); } +// Reference GMST values computed independently with the Meeus formula +// (280.46061837 + 360.98564736629 * (JD - 2451545.0)), 0.01 h tolerance. +TEST(SiderealClockTest, LstAtJ2000Epoch) +{ + // 2000-01-01 12:00 UTC at Greenwich + EXPECT_NEAR(18.6974, lstHours(2000, 1, 1, 12.0f, 0.0), 0.01); +} + +TEST(SiderealClockTest, LstAtMarchEquinox) +{ + // 2024-03-20 12:00 UTC at Greenwich + EXPECT_NEAR(23.9008, lstHours(2024, 3, 20, 12.0f, 0.0), 0.01); +} + +TEST(SiderealClockTest, LstAtJuneSolstice) +{ + // 2024-06-20 12:00 UTC at Greenwich + EXPECT_NEAR(5.9461, lstHours(2024, 6, 20, 12.0f, 0.0), 0.01); +} + +TEST(SiderealClockTest, LstAtSeptemberEquinox) +{ + // 2024-09-22 12:00 UTC at Greenwich + EXPECT_NEAR(12.1228, lstHours(2024, 9, 22, 12.0f, 0.0), 0.01); +} + +TEST(SiderealClockTest, LstAtDecemberSolstice) +{ + // 2024-12-21 12:00 UTC at Greenwich + EXPECT_NEAR(18.0367, lstHours(2024, 12, 21, 12.0f, 0.0), 0.01); +} + +TEST(SiderealClockTest, LstShiftsEastWithLongitudeAndWraps) +{ + // 13.5 deg east of Greenwich advances LST by 0.9 h, past 24 h here + EXPECT_NEAR(0.8008, lstHours(2024, 3, 20, 12.0f, 13.5), 0.01); +} + TEST(SiderealClockTest, ThetaOutputInRange) { double theta = SiderealClock::calculateTheta(100.0, 0.0, 12.0f); diff --git a/unit_tests/test_hal/test_eeprom_layout.cpp b/unit_tests/test_hal/test_eeprom_layout.cpp new file mode 100644 index 00000000..a31eb019 --- /dev/null +++ b/unit_tests/test_hal/test_eeprom_layout.cpp @@ -0,0 +1,152 @@ +#include + +#include "hal/EepromLayout.hpp" + +namespace L = hal::eeprom; + +// The EEPROM layout is a persistent on-device format: these tests pin it so an +// accidental reorder or renumbering is caught on the host instead of on a mount +// that has already been calibrated. + +static void expectFourByteField(int base, int b1, int b2, int b3) +{ + EXPECT_EQ(base + 1, b1); + EXPECT_EQ(base + 2, b2); + EXPECT_EQ(base + 3, b3); +} + +static void expectTwoByteField(int base, int b1) +{ + EXPECT_EQ(base + 1, b1); +} + +TEST(EepromLayoutTest, MagicMarkerIsEncodedInTheHighByte) +{ + EXPECT_EQ(0xCE00, L::MAGIC_MARKER_VALUE); + EXPECT_EQ(0xFE00, L::MAGIC_MARKER_MASK); + // The mask keeps the marker bits and drops the extended-flag bit. + EXPECT_EQ(L::MAGIC_MARKER_VALUE, L::MAGIC_MARKER_VALUE & L::MAGIC_MARKER_MASK); +} + +TEST(EepromLayoutTest, ExtendedMarkerStillMatchesTheMask) +{ + // 0xCF marks "extended flags present"; it must still read as present. + EXPECT_EQ(L::MAGIC_MARKER_VALUE, 0xCF00 & L::MAGIC_MARKER_MASK); + // Anything outside 0xCE/0xCF is "nothing stored". + EXPECT_NE(L::MAGIC_MARKER_VALUE, 0x0000 & L::MAGIC_MARKER_MASK); + EXPECT_NE(L::MAGIC_MARKER_VALUE, 0xCD00 & L::MAGIC_MARKER_MASK); +} + +TEST(EepromLayoutTest, ItemFlagsFillTheFlagsMask) +{ + uint16_t all = L::RA_STEPS_FLAG | L::DEC_STEPS_FLAG | L::SPEED_FACTOR_FLAG | L::BACKLASH_STEPS_FLAG | L::LATITUDE_FLAG + | L::LONGITUDE_FLAG | L::PITCH_OFFSET_FLAG | L::ROLL_OFFSET_FLAG | L::EXTENDED_FLAG; + + EXPECT_EQ(static_cast(L::FLAGS_MASK), all); + // Every flag is a distinct single bit, so the OR and the sum agree. + EXPECT_EQ(static_cast(all), + L::RA_STEPS_FLAG + L::DEC_STEPS_FLAG + L::SPEED_FACTOR_FLAG + L::BACKLASH_STEPS_FLAG + L::LATITUDE_FLAG + L::LONGITUDE_FLAG + + L::PITCH_OFFSET_FLAG + L::ROLL_OFFSET_FLAG + L::EXTENDED_FLAG); +} + +TEST(EepromLayoutTest, ExtendedItemFlagsFillTheExtendedMask) +{ + uint16_t all = L::PARKING_POS_MARKER_FLAG | L::DEC_LIMIT_MARKER_FLAG | L::UTC_OFFSET_MARKER_FLAG | L::RA_HOMING_MARKER_FLAG + | L::RA_NORM_STEPS_MARKER_FLAG | L::DEC_NORM_STEPS_MARKER_FLAG | L::DEC_HOMING_MARKER_FLAG | L::LAST_FLASHED_MARKER_FLAG + | L::AZ_POSITION_MARKER_FLAG | L::ALT_POSITION_MARKER_FLAG | L::AZ_NORM_STEPS_MARKER_FLAG + | L::ALT_NORM_STEPS_MARKER_FLAG; + + EXPECT_EQ(static_cast(L::EXTENDED_FLAGS_MASK), all); + EXPECT_EQ(static_cast(all), + L::PARKING_POS_MARKER_FLAG + L::DEC_LIMIT_MARKER_FLAG + L::UTC_OFFSET_MARKER_FLAG + L::RA_HOMING_MARKER_FLAG + + L::RA_NORM_STEPS_MARKER_FLAG + L::DEC_NORM_STEPS_MARKER_FLAG + L::DEC_HOMING_MARKER_FLAG + L::LAST_FLASHED_MARKER_FLAG + + L::AZ_POSITION_MARKER_FLAG + L::ALT_POSITION_MARKER_FLAG + L::AZ_NORM_STEPS_MARKER_FLAG + + L::ALT_NORM_STEPS_MARKER_FLAG); +} + +TEST(EepromLayoutTest, LegacyAddressesAreUnchanged) +{ + // The V1.x layout that every mount's stored calibration was written with. + EXPECT_EQ(0, L::SPEED_FACTOR_LOW_ADDR); + EXPECT_EQ(1, L::HA_HOUR_ADDR); + EXPECT_EQ(2, L::HA_MINUTE_ADDR); + EXPECT_EQ(3, L::SPEED_FACTOR_HIGH_ADDR); + EXPECT_EQ(4, L::FLAGS_ADDR); + EXPECT_EQ(5, L::MAGIC_MARKER_ADDR); + EXPECT_EQ(4, L::MAGIC_MARKER_AND_FLAGS_ADDR); + EXPECT_EQ(6, L::RA_STEPS_DEGREE_ADDR); + EXPECT_EQ(8, L::DEC_STEPS_DEGREE_ADDR); + EXPECT_EQ(10, L::BACKLASH_STEPS_ADDR); + EXPECT_EQ(12, L::LATITUDE_ADDR); + EXPECT_EQ(14, L::LONGITUDE_ADDR); + EXPECT_EQ(16, L::LCD_BRIGHTNESS_ADDR); + EXPECT_EQ(17, L::PITCH_OFFSET_ADDR); + EXPECT_EQ(19, L::ROLL_OFFSET_ADDR); + EXPECT_EQ(21, L::EXTENDED_FLAGS_ADDR); +} + +TEST(EepromLayoutTest, ExtendedAddressesAreUnchanged) +{ + EXPECT_EQ(23, L::RA_PARKING_POS_ADDR); + EXPECT_EQ(27, L::DEC_PARKING_POS_ADDR); + EXPECT_EQ(31, L::DEC_LOWER_LIMIT_ADDR); + EXPECT_EQ(35, L::DEC_UPPER_LIMIT_ADDR); + EXPECT_EQ(39, L::UTC_OFFSET_ADDR); + EXPECT_EQ(40, L::RA_HOMING_OFFSET_ADDR); + EXPECT_EQ(44, L::RA_NORM_STEPS_DEGREE_ADDR); + EXPECT_EQ(48, L::DEC_NORM_STEPS_DEGREE_ADDR); + EXPECT_EQ(52, L::DEC_HOMING_OFFSET_ADDR); + EXPECT_EQ(56, L::LAST_FLASHED_VERSION); + EXPECT_EQ(58, L::AZ_POSITION_ADDR); + EXPECT_EQ(62, L::ALT_POSITION_ADDR); + EXPECT_EQ(66, L::AZ_NORM_STEPS_DEGREE_ADDR); + EXPECT_EQ(70, L::ALT_NORM_STEPS_DEGREE_ADDR); +} + +TEST(EepromLayoutTest, MultiByteFieldsAreContiguous) +{ + expectTwoByteField(L::RA_STEPS_DEGREE_ADDR, L::_RA_STEPS_DEGREE_ADDR_1); + expectTwoByteField(L::DEC_STEPS_DEGREE_ADDR, L::_DEC_STEPS_DEGREE_ADDR_1); + expectTwoByteField(L::BACKLASH_STEPS_ADDR, L::_BACKLASH_STEPS_ADDR_1); + expectTwoByteField(L::LATITUDE_ADDR, L::_LATITUDE_ADDR_1); + expectTwoByteField(L::LONGITUDE_ADDR, L::_LONGITUDE_ADDR_1); + expectTwoByteField(L::PITCH_OFFSET_ADDR, L::_PITCH_OFFSET_ADDR_1); + expectTwoByteField(L::ROLL_OFFSET_ADDR, L::_ROLL_OFFSET_ADDR_1); + expectTwoByteField(L::EXTENDED_FLAGS_ADDR, L::_EXTENDED_FLAGS_ADDR_1); + expectTwoByteField(L::LAST_FLASHED_VERSION, L::_LAST_FLASHED_VERSION_1); + + expectFourByteField(L::RA_PARKING_POS_ADDR, L::_RA_PARKING_POS_ADDR_1, L::_RA_PARKING_POS_ADDR_2, L::_RA_PARKING_POS_ADDR_3); + expectFourByteField(L::DEC_PARKING_POS_ADDR, L::_DEC_PARKING_POS_ADDR_1, L::_DEC_PARKING_POS_ADDR_2, L::_DEC_PARKING_POS_ADDR_3); + expectFourByteField(L::DEC_LOWER_LIMIT_ADDR, L::_DEC_LOWER_LIMIT_ADDR_1, L::_DEC_LOWER_LIMIT_ADDR_2, L::_DEC_LOWER_LIMIT_ADDR_3); + expectFourByteField(L::DEC_UPPER_LIMIT_ADDR, L::_DEC_UPPER_LIMIT_ADDR_1, L::_DEC_UPPER_LIMIT_ADDR_2, L::_DEC_UPPER_LIMIT_ADDR_3); + expectFourByteField(L::RA_HOMING_OFFSET_ADDR, L::_RA_HOMING_OFFSET_ADDR_1, L::_RA_HOMING_OFFSET_ADDR_2, L::_RA_HOMING_OFFSET_ADDR_3); + expectFourByteField( + L::RA_NORM_STEPS_DEGREE_ADDR, L::_RA_NORM_STEPS_DEGREE_ADDR_1, L::_RA_NORM_STEPS_DEGREE_ADDR_2, L::_RA_NORM_STEPS_DEGREE_ADDR_3); + expectFourByteField(L::DEC_NORM_STEPS_DEGREE_ADDR, + L::_DEC_NORM_STEPS_DEGREE_ADDR_1, + L::_DEC_NORM_STEPS_DEGREE_ADDR_2, + L::_DEC_NORM_STEPS_DEGREE_ADDR_3); + expectFourByteField( + L::DEC_HOMING_OFFSET_ADDR, L::_DEC_HOMING_OFFSET_ADDR_1, L::_DEC_HOMING_OFFSET_ADDR_2, L::_DEC_HOMING_OFFSET_ADDR_3); + expectFourByteField(L::AZ_POSITION_ADDR, L::_AZ_POSITION_ADDR_1, L::_AZ_POSITION_ADDR_2, L::_AZ_POSITION_ADDR_3); + expectFourByteField(L::ALT_POSITION_ADDR, L::_ALT_POSITION_ADDR_1, L::_ALT_POSITION_ADDR_2, L::_ALT_POSITION_ADDR_3); + expectFourByteField( + L::AZ_NORM_STEPS_DEGREE_ADDR, L::_AZ_NORM_STEPS_DEGREE_ADDR_1, L::_AZ_NORM_STEPS_DEGREE_ADDR_2, L::_AZ_NORM_STEPS_DEGREE_ADDR_3); + expectFourByteField(L::ALT_NORM_STEPS_DEGREE_ADDR, + L::_ALT_NORM_STEPS_DEGREE_ADDR_1, + L::_ALT_NORM_STEPS_DEGREE_ADDR_2, + L::_ALT_NORM_STEPS_DEGREE_ADDR_3); +} + +TEST(EepromLayoutTest, StoreSizeCoversEveryField) +{ + EXPECT_EQ(74, L::STORE_SIZE); + EXPECT_EQ(L::STORE_SIZE, L::_ALT_NORM_STEPS_DEGREE_ADDR_3 + 1); + // Addresses must stay addressable with a single byte. + EXPECT_LE(L::STORE_SIZE, 255); +} + +TEST(EepromLayoutTest, SteppingStorageNormalizedIs256Microsteps) +{ + EXPECT_FLOAT_EQ(25600.0f, L::SteppingStorageNormalized); +}