From 3e90292737a956f3bd064e9eb5c9f9fd78918537 Mon Sep 17 00:00:00 2001 From: Lutz Date: Sat, 14 Mar 2026 16:58:46 -0700 Subject: [PATCH 1/7] V1.13.20 - Updates - Code improvement to stability, multithreading, memory usage, user input, etc. - Fixed a bug that caused Latitude to overwrite Longitude in the EEPROM code. --- CLAUDE.md | 101 ++++++++++++++++++++++++++++++++++ Changelog.md | 4 ++ Version.h | 2 +- src/DayTime.cpp | 6 +- src/Declination.cpp | 11 +--- src/EPROMStore.hpp | 2 +- src/EndSwitches.hpp | 4 +- src/Gyro.cpp | 12 +++- src/MeadeCommandProcessor.cpp | 64 +++++++++++++++++---- src/Mount.cpp | 64 +++++++++++---------- src/Mount.hpp | 8 +-- src/WifiControl.cpp | 6 +- src/WifiControl.hpp | 4 +- 13 files changed, 224 insertions(+), 64 deletions(-) create mode 100644 CLAUDE.md diff --git a/CLAUDE.md b/CLAUDE.md new file mode 100644 index 00000000..a7271db8 --- /dev/null +++ b/CLAUDE.md @@ -0,0 +1,101 @@ +# CLAUDE.md + +This file provides guidance to Claude Code (claude.ai/code) when working with code in this repository. + +## Project Overview + +OpenAstroTracker-Firmware is embedded C++ firmware for the OpenAstroTracker automated astronomy mount. It supports multiple hardware platforms (AVR ATmega2560 and ESP32) with various stepper motor drivers, display types, and accessories. + +## Build System + +PlatformIO is the build system. The project uses the Arduino framework. + +### Common Commands + +```shell +# Build for a specific board environment +pio run -e ramps +pio run -e esp32 +pio run -e mksgenlv21 +pio run -e mksgenlv2 +pio run -e mksgenlv1 +pio run -e oaeboardv1 + +# Upload firmware +pio run -e ramps -t upload + +# Run unit tests (native platform) +pio test -e native + +# Run matrix build (tests many configuration combinations across boards) +python matrix_build.py -b ramps # single board +python matrix_build.py # all boards + +# Format code (clang-format v12 required, enforced by CI) +bash -c 'shopt -s nullglob globstar;GLOBIGNORE=./src/libs/TimerInterrupt/*; for i in ./{.,src/**,unit_tests,boards/**}/*.{c,cpp,h,hpp}; do clang-format -i $i; done' + +# Debug (ATmega2560 only, uses avr-stub) +pio run -e ramps -t clean && piodebuggdb -e ramps + +# Generate Meade command Wiki docs +python scripts/MeadeCommandParser.py +``` + +### Build Notes + +- `-Werror` is enabled: all warnings are errors +- Uses ETL (Embedded Template Library) in non-STL mode (`-D ETL_NO_STL`), not the C++ standard library +- Optimization: `-O2` (production), `-Og` (debug) +- Monitor baud rate: 19200 +- `src/libs/TimerInterrupt/` is excluded from formatting checks +- CI runs on every PR: build all boards, clang-format check, unit tests + +## Architecture + +### Configuration Hierarchy (evaluated in this order) + +1. **`Configuration_local.hpp`** - User hardware config (not tracked by git). Generate at https://config.openastrotech.com/ +2. **`Configuration_local_.hpp`** - Board-specific local config variant +3. **`Configuration.hpp`** - Default values for anything not set in local config +4. **`Configuration_adv.hpp`** - Advanced settings (motor steps, speed, microstepping, TMC driver params) +5. **`Constants.hpp`** - System constants (board IDs, feature flag values). **Do not modify for user configuration.** + +### Core Modules + +- **`Mount` (src/Mount.hpp/cpp)** - Central controller. Manages stepper motors (RA, DEC, AZ, ALT, Focus), coordinate calculations, slewing, tracking, parking, and guiding. This is by far the largest module (~150KB source). +- **`MeadeCommandProcessor` (src/MeadeCommandProcessor.hpp/cpp)** - Serial command interface implementing the Meade LX200 protocol. Handles all external communication. +- **`EPROMStore` (src/EPROMStore.hpp/cpp)** - Platform-agnostic non-volatile storage abstraction (coordinates, calibration, motor config). Uses magic markers `0xCE`/`0xCF` for validation. +- **`b_setup.hpp`** - Arduino `setup()` implementation and hardware initialization. +- **`a_inits.hpp`** - Global variable initialization. + +### Entry Point + +`OpenAstroTracker-Firmware.ino` is the Arduino sketch entry point. It delegates to `src/a_inits.hpp` (globals) and `src/b_setup.hpp` (setup/loop). + +### Stepper Control + +Two modes depending on platform: + +- **AVR (NEW_STEPPER_LIB)**: Interrupt-driven via Timer 3 (RA) and Timer 4 (DEC) at 2 kHz. Uses `InterruptAccelStepper` library. Configured via `StepperConfiguration.hpp`. +- **ESP32**: FreeRTOS task on Core 0 at 1 kHz (`stepperControlTask`). Core 1 runs `loop()` for serial/UI. + +Both call `Mount::interruptLoop()` for step generation. + +### Board Pin Definitions + +Board-specific pin mappings live in `boards//pins_.h`. Each board has its own directory. + +### Display & UI + +Optional LCD menu system (`LcdMenu`, `LcdButtons`) supporting multiple display types: direct LCD keypad, I2C LCD (MCP23008/MCP23017), and I2C SSD1306 OLED with joystick. An optional separate 128x64 info display shows real-time status. + +## Key Conventions + +- Primary branch: `develop` +- PRs must pass CI (build all boards + clang-format + unit tests) +- PR comments are resolved by OAT developers, not PR authors +- clang-format v12 is the enforced formatter (column limit: 140, Allman-style braces, 4-space indent, no tabs) +- Pointer alignment: right (`int *ptr`) +- Supported driver types: `DRIVER_TYPE_A4988_GENERIC`, `DRIVER_TYPE_TMC2209_STANDALONE`, `DRIVER_TYPE_TMC2209_UART` +- Supported stepper types: `STEPPER_TYPE_NONE`, `STEPPER_TYPE_ENABLED` +- Debug logging uses bit-flag levels (INFO, SERIAL, WIFI, MOUNT, MEADE, STEPPERS, EEPROM, GYRO, GPS, FOCUS, etc.) diff --git a/Changelog.md b/Changelog.md index dbb8a601..5e9e57c6 100644 --- a/Changelog.md +++ b/Changelog.md @@ -1,3 +1,7 @@ +**V1.13.20 - Updates** +- Code improvement to stability, multithreading, memory usage, user input, etc. +- Fixed a bug that caused Latitude to overwrite Longitude in the EEPROM code. + **V1.13.19 - Updates** - Support inverting and mirroring InfoDisplays. diff --git a/Version.h b/Version.h index 1f1c5155..bfd8e288 100644 --- a/Version.h +++ b/Version.h @@ -3,4 +3,4 @@ // Also, numbers are interpreted as simple numbers. _ __ _ // So 1.8 is actually 1.08, meaning that 1.12 is a later version than 1.8. \_(..)_/ -#define VERSION "V1.13.19" +#define VERSION "V1.13.20" diff --git a/src/DayTime.cpp b/src/DayTime.cpp index 05a24e9a..eab4519e 100644 --- a/src/DayTime.cpp +++ b/src/DayTime.cpp @@ -239,10 +239,8 @@ const char *DayTime::ToString() const } *p++ = '0' + (secs % 10); - *p++ = ' '; - *p++ = '('; - strcpy(p, String(this->getTotalHours(), 5).c_str()); - strcat(p, ")"); + size_t used = p - achBuf; + snprintf(p, sizeof(achBuf) - used, " (%.5f)", this->getTotalHours()); return achBuf; } void DayTime::printTwoDigits(char *achDegs, int num) const diff --git a/src/Declination.cpp b/src/Declination.cpp index e19ac817..a43ca10b 100644 --- a/src/Declination.cpp +++ b/src/Declination.cpp @@ -84,14 +84,9 @@ const char *Declination::ToString() const { ToDisplayString('*', ':'); - char *p = achBufDeg + strlen(achBufDeg); - - *p++ = ' '; - *p++ = '('; - strcpy(p, String(inNorthernHemisphere ? 90 - fabsf(getTotalHours()) : -90 + fabsf(getTotalHours()), 4).c_str()); - strcat(p, ", "); - strcat(p, String(getTotalHours(), 4).c_str()); - strcat(p, ")"); + size_t used = strlen(achBufDeg); + float displayVal = inNorthernHemisphere ? 90 - fabsf(getTotalHours()) : -90 + fabsf(getTotalHours()); + snprintf(achBufDeg + used, sizeof(achBufDeg) - used, " (%.4f, %.4f)", displayVal, getTotalHours()); return achBufDeg; } diff --git a/src/EPROMStore.hpp b/src/EPROMStore.hpp index 506e987c..11acd189 100644 --- a/src/EPROMStore.hpp +++ b/src/EPROMStore.hpp @@ -168,7 +168,7 @@ class EEPROMStore LATITUDE_ADDR = 12, _LATITUDE_ADDR_1 = 13, // Int16 LONGITUDE_ADDR = 14, - _LONGITUDE_ADDR_1 = 13, // Int16 + _LONGITUDE_ADDR_1 = 15, // Int16 LCD_BRIGHTNESS_ADDR = 16, // Uint8 PITCH_OFFSET_ADDR = 17, _PITCH_OFFSET_ADDR_1 = 18, // Uint16 diff --git a/src/EndSwitches.hpp b/src/EndSwitches.hpp index 9af0d98c..b979ed64 100644 --- a/src/EndSwitches.hpp +++ b/src/EndSwitches.hpp @@ -27,10 +27,10 @@ enum EndSwitchState class EndSwitch { private: - EndSwitchState _state; + volatile EndSwitchState _state; Mount *_pMount; StepperAxis _axis; - long _posWhenTriggered; + volatile long _posWhenTriggered; int _activeState; int _inactiveState; int _dir; diff --git a/src/Gyro.cpp b/src/Gyro.cpp index b1b9ee2a..f084f1a9 100644 --- a/src/Gyro.cpp +++ b/src/Gyro.cpp @@ -91,9 +91,17 @@ angle_t Gyro::getCurrentAngles() int16_t AcZ = Wire.read() << 8 | Wire.read(); // Z-axis value // Calculating the Pitch angle (rotation around Y-axis) - result.pitchAngle += ((atanf(-1 * AcX / sqrtf(powf(AcY, 2) + powf(AcZ, 2))) * 180.0f / static_cast(PI)) * 2.0f) / 2.0f; + float denomPitch = sqrtf((float) AcY * AcY + (float) AcZ * AcZ); + if (denomPitch > 0.0f) + { + result.pitchAngle += atanf(-1.0f * AcX / denomPitch) * 180.0f / static_cast(PI); + } // Calculating the Roll angle (rotation around X-axis) - result.rollAngle += ((atanf(-1 * AcY / sqrtf(powf(AcX, 2) + powf(AcZ, 2))) * 180.0f / static_cast(PI)) * 2.0f) / 2.0f; + float denomRoll = sqrtf((float) AcX * AcX + (float) AcZ * AcZ); + if (denomRoll > 0.0f) + { + result.rollAngle += atanf(-1.0f * AcY / denomRoll) * 180.0f / static_cast(PI); + } delay(10); // Decorrelate measurements } diff --git a/src/MeadeCommandProcessor.cpp b/src/MeadeCommandProcessor.cpp index de66f0f3..5b1ad708 100644 --- a/src/MeadeCommandProcessor.cpp +++ b/src/MeadeCommandProcessor.cpp @@ -1258,6 +1258,10 @@ String MeadeCommandProcessor::handleMeadeInit(String inCmd) ///////////////////////////// String MeadeCommandProcessor::handleMeadeGetInfo(String inCmd) { + if (inCmd.length() < 1) + { + return ""; + } char cmdOne = inCmd[0]; char cmdTwo = (inCmd.length() > 1) ? inCmd[1] : '\0'; char achBuffer[20]; @@ -1330,7 +1334,7 @@ String MeadeCommandProcessor::handleMeadeGetInfo(String inCmd) case 'G': // :GG { int offset = _mount->getLocalUtcOffset(); - sprintf(achBuffer, "%+03d#", -offset); + snprintf(achBuffer, sizeof(achBuffer), "%+03d#", -offset); return String(achBuffer); } case 'a': // :Ga @@ -1352,7 +1356,7 @@ String MeadeCommandProcessor::handleMeadeGetInfo(String inCmd) case 'C': // :GC { LocalDate date = _mount->getLocalDate(); - sprintf(achBuffer, "%02d/%02d/%02d#", date.month, date.day, date.year % 100); + snprintf(achBuffer, sizeof(achBuffer), "%02d/%02d/%02d#", date.month, date.day, date.year % 100); return String(achBuffer); } case 'M': // :GM @@ -1385,6 +1389,10 @@ String MeadeCommandProcessor::handleMeadeGetInfo(String inCmd) ///////////////////////////// String MeadeCommandProcessor::handleMeadeGPSCommands(String inCmd) { + if (inCmd.length() < 1) + { + return "0"; + } #if USE_GPS == 1 if (inCmd[0] == 'T') { @@ -1415,6 +1423,10 @@ String MeadeCommandProcessor::handleMeadeGPSCommands(String inCmd) ///////////////////////////// String MeadeCommandProcessor::handleMeadeSyncControl(String inCmd) { + if (inCmd.length() < 1) + { + return "FAIL#"; + } if (inCmd[0] == 'M') // :CM { _mount->syncPosition(_mount->targetRA(), _mount->targetDEC()); @@ -1429,6 +1441,10 @@ String MeadeCommandProcessor::handleMeadeSyncControl(String inCmd) ///////////////////////////// String MeadeCommandProcessor::handleMeadeSetInfo(String inCmd) { + if (inCmd.length() < 1) + { + return "0"; + } if ((inCmd[0] == 'd') && (inCmd.length() == 10)) { // Set DEC @@ -1561,6 +1577,10 @@ String MeadeCommandProcessor::handleMeadeSetInfo(String inCmd) ///////////////////////////// String MeadeCommandProcessor::handleMeadeMovement(String inCmd) { + if (inCmd.length() < 1) + { + return ""; + } LOG(DEBUG_MEADE, "[MEADE]: Process Move command: [%s]", inCmd.c_str()); if (inCmd[0] == 'S') // :MS# { @@ -1605,12 +1625,12 @@ String MeadeCommandProcessor::handleMeadeMovement(String inCmd) direction = EAST; else if (inCmd[1] == 'w') direction = WEST; - int duration = (inCmd[2] - '0') * 1000 + (inCmd[3] - '0') * 100 + (inCmd[4] - '0') * 10 + (inCmd[5] - '0'); + int duration = inCmd.substring(2, 6).toInt(); _mount->guidePulse(direction, duration); return ""; } } - else if (inCmd[0] == 'A') + else if (inCmd[0] == 'A' && inCmd.length() > 1) { LOG(DEBUG_MEADE, "[MEADE]: Move Az/Alt"); @@ -1730,6 +1750,10 @@ String MeadeCommandProcessor::handleMeadeMovement(String inCmd) ///////////////////////////// String MeadeCommandProcessor::handleMeadeHome(String inCmd) { + if (inCmd.length() < 1) + { + return ""; + } if (inCmd[0] == 'P') // :hP { // Park _mount->park(); @@ -1765,6 +1789,10 @@ String MeadeCommandProcessor::handleMeadeDistance(String inCmd) ///////////////////////////// String MeadeCommandProcessor::handleMeadeExtraCommands(String inCmd) { + if (inCmd.length() < 1) + { + return ""; + } #if SUPPORT_DRIFT_ALIGNMENT == 1 // :XDmmm if (inCmd[0] == 'D') // :XD @@ -1874,8 +1902,8 @@ String MeadeCommandProcessor::handleMeadeExtraCommands(String inCmd) { long azPos, altPos; _mount->getAZALTPositions(azPos, altPos); - char scratchBuffer[20]; - sprintf(scratchBuffer, "%ld|%ld#", azPos, altPos); + char scratchBuffer[24]; + snprintf(scratchBuffer, sizeof(scratchBuffer), "%ld|%ld#", azPos, altPos); return String(scratchBuffer); } else if (inCmd[1] == 'C') // :XGCn.nn*m.mm# @@ -1888,8 +1916,8 @@ String MeadeCommandProcessor::handleMeadeExtraCommands(String inCmd) float raCoord = coords.substring(0, star).toFloat(); float decCoord = coords.substring(star + 1).toFloat(); _mount->calculateStepperPositions(raCoord, decCoord, raPos, decPos); - char scratchBuffer[20]; - sprintf(scratchBuffer, "%ld|%ld#", raPos, decPos); + char scratchBuffer[24]; + snprintf(scratchBuffer, sizeof(scratchBuffer), "%ld|%ld#", raPos, decPos); return String(scratchBuffer); } } @@ -1933,7 +1961,7 @@ String MeadeCommandProcessor::handleMeadeExtraCommands(String inCmd) { char scratchBuffer[10]; DayTime ha = _mount->calculateHa(); - sprintf(scratchBuffer, "%02d%02d%02d#", ha.getHours(), ha.getMinutes(), ha.getSeconds()); + snprintf(scratchBuffer, sizeof(scratchBuffer), "%02d%02d%02d#", ha.getHours(), ha.getMinutes(), ha.getSeconds()); return String(scratchBuffer); } } @@ -1941,7 +1969,7 @@ String MeadeCommandProcessor::handleMeadeExtraCommands(String inCmd) { char scratchBuffer[10]; DayTime lst = _mount->calculateLst(); - sprintf(scratchBuffer, "%02d%02d%02d#", lst.getHours(), lst.getMinutes(), lst.getSeconds()); + snprintf(scratchBuffer, sizeof(scratchBuffer), "%02d%02d%02d#", lst.getHours(), lst.getMinutes(), lst.getSeconds()); return String(scratchBuffer); } else if (inCmd[1] == 'N') // :XGN# @@ -2164,6 +2192,10 @@ String MeadeCommandProcessor::handleMeadeQuit(String inCmd) ///////////////////////////// String MeadeCommandProcessor::handleMeadeSetSlewRate(String inCmd) { + if (inCmd.length() < 1) + { + return ""; + } switch (inCmd[0]) { case 'S': @@ -2189,6 +2221,10 @@ String MeadeCommandProcessor::handleMeadeSetSlewRate(String inCmd) ///////////////////////////// String MeadeCommandProcessor::handleMeadeFocusCommands(String inCmd) { + if (inCmd.length() < 1) + { + return ""; + } #if (FOCUS_STEPPER_TYPE != STEPPER_TYPE_NONE) if (inCmd[0] == '+') // :F+ { @@ -2261,6 +2297,10 @@ String MeadeCommandProcessor::handleMeadeFocusCommands(String inCmd) String MeadeCommandProcessor::processCommand(String inCmd) { + if (inCmd.length() < 2) + { + return ""; + } if (inCmd[0] == ':') { LOG(DEBUG_MEADE, "[MEADE]: Received command '%s'", inCmd.c_str()); @@ -2272,6 +2312,10 @@ String MeadeCommandProcessor::processCommand(String inCmd) inCmd.remove(spacePos, 1); } + if (inCmd.length() < 2) + { + return ""; + } LOG(DEBUG_MEADE, "[MEADE]: Processing command '%s'", inCmd.c_str()); char command = inCmd[1]; inCmd = inCmd.substring(2); diff --git a/src/Mount.cpp b/src/Mount.cpp index e41eb05a..68e43f86 100644 --- a/src/Mount.cpp +++ b/src/Mount.cpp @@ -1522,7 +1522,7 @@ void Mount::startSlewingToTarget() LOG(DEBUG_STEPPERS, "[STEPPERS]: startSlewingToTarget: TRK stopped at %lms", _trackerStoppedAt); } - _mountStatus |= STATUS_SLEWING | STATUS_SLEWING_TO_TARGET; + setStatusFlag(STATUS_SLEWING | STATUS_SLEWING_TO_TARGET); #if DEC_DRIVER_TYPE == DRIVER_TYPE_TMC2209_UART // Since normal state for DEC is guide microstepping, switch to slew microstepping here. LOG(DEBUG_STEPPERS, "[STEPPERS]: startSlewingToTarget: Switching DEC driver to microsteps(%d)", DEC_SLEW_MICROSTEPPING); @@ -1586,7 +1586,7 @@ void Mount::startSlewingToHome() LOG(DEBUG_STEPPERS, "[STEPPERS]: startSlewingToHome: TRK stopped at %lms", _trackerStoppedAt); } - _mountStatus |= STATUS_SLEWING | STATUS_SLEWING_TO_TARGET; + setStatusFlag(STATUS_SLEWING | STATUS_SLEWING_TO_TARGET); #if DEC_DRIVER_TYPE == DRIVER_TYPE_TMC2209_UART // Since normal state for DEC is guide microstepping, switch to slew microstepping here. LOG(DEBUG_STEPPERS, "[STEPPERS]: startSlewingToHome: Switching DEC driver to microsteps(%d)", DEC_SLEW_MICROSTEPPING); @@ -1615,7 +1615,7 @@ void Mount::stopGuiding(bool ra, bool dec) LOG(DEBUG_STEPPERS | DEBUG_GUIDE, "[GUIDE]: stopGuide: TRK stop guide at : %l", _stepperTRK->currentPosition()); _stepperTRK->setSpeed(_trackingSpeed); LOG(DEBUG_STEPPERS | DEBUG_GUIDE, "[GUIDE]: stopGuide: TRK speed set to : %f", _trackingSpeed); - _mountStatus &= ~STATUS_GUIDE_PULSE_RA; + clearStatusFlag(STATUS_GUIDE_PULSE_RA); } if (dec && (_mountStatus & STATUS_GUIDE_PULSE_DEC)) @@ -1633,14 +1633,14 @@ void Mount::stopGuiding(bool ra, bool dec) } #endif - _mountStatus &= ~STATUS_GUIDE_PULSE_DEC; + clearStatusFlag(STATUS_GUIDE_PULSE_DEC); } //disable pulse state if no direction is active if ((_mountStatus & STATUS_GUIDE_PULSE_DIR) == 0) { LOG(DEBUG_STEPPERS | DEBUG_GUIDE, "[GUIDE]: Clear guiding state"); - _mountStatus &= ~STATUS_GUIDE_PULSE_MASK; + clearStatusFlag(STATUS_GUIDE_PULSE_MASK); } else { @@ -1714,7 +1714,7 @@ void Mount::guidePulse(byte direction, int duration) "[GUIDE]: guidePulse: DEC guide speed : %f", DEC_PULSE_MULTIPLIER * decGuidingSpeed); _stepperGUIDE->setSpeed(DEC_PULSE_MULTIPLIER * decGuidingSpeed); - _mountStatus |= STATUS_GUIDE_PULSE | STATUS_GUIDE_PULSE_DEC; + setStatusFlag(STATUS_GUIDE_PULSE | STATUS_GUIDE_PULSE_DEC); _guideDecEndTime = millis() + duration; } break; @@ -1735,7 +1735,7 @@ void Mount::guidePulse(byte direction, int duration) "[GUIDE]: guidePulse: DEC guide speed : %f", -DEC_PULSE_MULTIPLIER * decGuidingSpeed); _stepperGUIDE->setSpeed(-DEC_PULSE_MULTIPLIER * decGuidingSpeed); - _mountStatus |= STATUS_GUIDE_PULSE | STATUS_GUIDE_PULSE_DEC; + setStatusFlag(STATUS_GUIDE_PULSE | STATUS_GUIDE_PULSE_DEC); _guideDecEndTime = millis() + duration; } break; @@ -1751,7 +1751,7 @@ void Mount::guidePulse(byte direction, int duration) (RA_PULSE_MULTIPLIER * raGuidingSpeed), RA_PULSE_MULTIPLIER); _stepperTRK->setSpeed(RA_PULSE_MULTIPLIER * raGuidingSpeed); // Faster than siderael - _mountStatus |= STATUS_GUIDE_PULSE | STATUS_GUIDE_PULSE_RA; + setStatusFlag(STATUS_GUIDE_PULSE | STATUS_GUIDE_PULSE_RA); _guideRaEndTime = millis() + duration; break; @@ -1766,7 +1766,7 @@ void Mount::guidePulse(byte direction, int duration) (2.0 - RA_PULSE_MULTIPLIER * raGuidingSpeed), (2.0 - RA_PULSE_MULTIPLIER)); _stepperTRK->setSpeed(raGuidingSpeed * (2.0f - RA_PULSE_MULTIPLIER)); // Slower than siderael - _mountStatus |= STATUS_GUIDE_PULSE | STATUS_GUIDE_PULSE_RA; + setStatusFlag(STATUS_GUIDE_PULSE | STATUS_GUIDE_PULSE_RA); _guideRaEndTime = millis() + duration; break; } @@ -1853,7 +1853,7 @@ void Mount::setManualSlewMode(bool state) stopSlewing(ALL_DIRECTIONS); stopSlewing(TRACKING); waitUntilStopped(ALL_DIRECTIONS); - _mountStatus |= STATUS_SLEWING | STATUS_SLEWING_MANUAL; + setStatusFlag(STATUS_SLEWING | STATUS_SLEWING_MANUAL); #if RA_DRIVER_TYPE == DRIVER_TYPE_TMC2209_UART LOG(DEBUG_STEPPERS, "[STEPPERS]: setManualSlewMode: Switching RA driver to microsteps(%d)", RA_SLEW_MICROSTEPPING); _driverRA->microsteps(RA_SLEW_MICROSTEPPING == 1 ? 0 : RA_SLEW_MICROSTEPPING); @@ -1861,7 +1861,7 @@ void Mount::setManualSlewMode(bool state) } else { - _mountStatus &= ~STATUS_SLEWING_MANUAL; + clearStatusFlag(STATUS_SLEWING_MANUAL); stopSlewing(ALL_DIRECTIONS); waitUntilStopped(ALL_DIRECTIONS); LOG(DEBUG_STEPPERS, "[STEPPERS]: setManualSlewMode: Set RA speed/accel: %l / %l", _maxRASpeed, _maxRAAcceleration); @@ -1986,7 +1986,7 @@ void Mount::park() stopSlewing(ALL_DIRECTIONS | TRACKING); waitUntilStopped(ALL_DIRECTIONS); startSlewingToHome(); - _mountStatus |= STATUS_PARKING; + setStatusFlag(STATUS_PARKING); } bool Mount::isAxisRunning(StepperAxis axis) @@ -2281,12 +2281,16 @@ void Mount::setTrackingStepperPos(long stepPos) void Mount::setStatusFlag(int flag) { + noInterrupts(); _mountStatus |= flag; + interrupts(); } void Mount::clearStatusFlag(int flag) { + noInterrupts(); _mountStatus &= ~flag; + interrupts(); } ///////////////////////////////// @@ -2518,7 +2522,7 @@ void Mount::startSlewing(int direction) _stepperTRK->setSpeed(_trackingSpeed); // Turn on tracking - _mountStatus |= STATUS_TRACKING; + setStatusFlag(STATUS_TRACKING); } else { @@ -2563,7 +2567,7 @@ void Mount::startSlewing(int direction) } _stepperDEC->moveTo(targetLocation); - _mountStatus |= STATUS_SLEWING; + setStatusFlag(STATUS_SLEWING); } if (direction & SOUTH) @@ -2583,10 +2587,10 @@ void Mount::startSlewing(int direction) } _stepperDEC->moveTo(targetLocation); - _mountStatus |= STATUS_SLEWING; + setStatusFlag(STATUS_SLEWING); } - const float trackedHours = (_stepperTRK->currentPosition() / _trackingSpeed) / 3600.0F; // steps / steps/s / 3600 = hours + const float trackedHours = (_trackingSpeed > 0) ? ((_stepperTRK->currentPosition() / _trackingSpeed) / 3600.0F) : 0.0F; // steps / steps/s / 3600 = hours if (direction & EAST) { // We need to subtract the distance tracked from the physical RA home coordinate @@ -2596,7 +2600,7 @@ void Mount::startSlewing(int direction) -sign * targetEastPos, trackedHours); _stepperRA->moveTo(-sign * targetEastPos); - _mountStatus |= STATUS_SLEWING; + setStatusFlag(STATUS_SLEWING); } if (direction & WEST) { @@ -2607,7 +2611,7 @@ void Mount::startSlewing(int direction) sign * targetWestPos, trackedHours); _stepperRA->moveTo(sign * targetWestPos); - _mountStatus |= STATUS_SLEWING; + setStatusFlag(STATUS_SLEWING); } } } @@ -2624,7 +2628,7 @@ void Mount::stopSlewing(int direction) if (direction & TRACKING) { // Turn off tracking - _mountStatus &= ~STATUS_TRACKING; + clearStatusFlag(STATUS_TRACKING); LOG(DEBUG_STEPPERS, "[STEPPERS]: stopSlewing: TRK stepper stop()"); _stepperTRK->stop(); @@ -2642,7 +2646,7 @@ void Mount::stopSlewing(int direction) _stepperRA->stop(); if (isFindingHome()) { - _mountStatus &= ~STATUS_FINDING_HOME; + clearStatusFlag(STATUS_FINDING_HOME); } } } @@ -3150,7 +3154,7 @@ void Mount::loop() // // Arrived at target after Slew! // - _mountStatus &= ~(STATUS_SLEWING | STATUS_SLEWING_TO_TARGET | STATUS_SLEWING_MANUAL); + clearStatusFlag(STATUS_SLEWING | STATUS_SLEWING_TO_TARGET | STATUS_SLEWING_MANUAL); if (_stepperWasRunning) { @@ -3253,7 +3257,7 @@ void Mount::loop() _driverDEC->microsteps(DEC_SLEW_MICROSTEPPING == 1 ? 0 : DEC_SLEW_MICROSTEPPING); #endif LOG(DEBUG_MOUNT | DEBUG_STEPPERS, "[MOUNT]: Loop: Was parking, so no tracking. Proceeding to park position..."); - _mountStatus &= ~STATUS_PARKING; + clearStatusFlag(STATUS_PARKING); _slewingToPark = true; _stepperRA->moveTo(-getHomingOffset(StepperAxis::RA_STEPS)); _stepperDEC->moveTo(-getHomingOffset(StepperAxis::DEC_STEPS)); @@ -3268,12 +3272,14 @@ void Mount::loop() if ((_stepperDEC->distanceToGo() != 0) || (_stepperRA->distanceToGo() != 0)) { LOG(DEBUG_MOUNT | DEBUG_STEPPERS, "[MOUNT]: Loop: Distance to Parking is non-zero, slewing to park position..."); - _mountStatus |= STATUS_PARKING_POS | STATUS_SLEWING; + setStatusFlag(STATUS_PARKING_POS | STATUS_SLEWING); } else { LOG(DEBUG_MOUNT | DEBUG_STEPPERS, "[MOUNT]: Loop: Already at Parking pos, so done."); + noInterrupts(); _mountStatus = STATUS_PARKED; + interrupts(); } } else @@ -3291,7 +3297,9 @@ void Mount::loop() else if (_slewingToPark) { LOG(DEBUG_MOUNT | DEBUG_STEPPERS, "[MOUNT]: Loop: Arrived at park position..."); - _mountStatus = STATUS_PARKED; + noInterrupts(); + _mountStatus = STATUS_PARKED; + interrupts(); _slewingToPark = false; } _totalDECMove = _totalRAMove = 0; @@ -3581,7 +3589,7 @@ void Mount::calculateRAandDECSteppers(long &targetRASteps, long &targetDECSteps, LOG(DEBUG_COORD_CALC, "[MOUNT]: CalcSteppersIn: moveRA (target) is %f", moveRA); // Total hours of tracking-to-date - float trackedHours = (_stepperTRK->currentPosition() / _trackingSpeed) / 3600.0F; // steps / steps/s / 3600 = hours + float trackedHours = (_trackingSpeed > 0) ? ((_stepperTRK->currentPosition() / _trackingSpeed) / 3600.0F) : 0.0F; // steps / steps/s / 3600 = hours LOG(DEBUG_COORD_CALC, "[MOUNT]: CalcSteppersIn: Tracked time is %l steps (%f h).", _stepperTRK->currentPosition(), trackedHours); // The current RA of the home position, taking tracking-to-date into account @@ -3792,7 +3800,7 @@ void Mount::moveStepperBy(StepperAxis direction, long steps) LOG(DEBUG_STEPPERS, "[STEPPERS]: moveStepperBy: Switching RA driver to microsteps(%d)", RA_SLEW_MICROSTEPPING); _driverRA->microsteps(RA_SLEW_MICROSTEPPING == 1 ? 0 : RA_SLEW_MICROSTEPPING); #endif - _mountStatus |= STATUS_SLEWING | STATUS_SLEWING_TO_TARGET; + setStatusFlag(STATUS_SLEWING | STATUS_SLEWING_TO_TARGET); _stepperWasRunning = true; moveSteppersTo(_stepperRA->currentPosition() + steps, 0, direction); _totalRAMove = 1.0f * _stepperRA->distanceToGo(); @@ -3800,7 +3808,7 @@ void Mount::moveStepperBy(StepperAxis direction, long steps) case DEC_STEPS: { - _mountStatus |= STATUS_SLEWING | STATUS_SLEWING_TO_TARGET; + setStatusFlag(STATUS_SLEWING | STATUS_SLEWING_TO_TARGET); #if DEC_DRIVER_TYPE == DRIVER_TYPE_TMC2209_UART // Since normal state for DEC is guide microstepping, switch to slew microstepping here. LOG(DEBUG_STEPPERS, "[STEPPERS]: moveStepperBy: Switching DEC driver to microsteps(%d)", DEC_SLEW_MICROSTEPPING); @@ -4276,7 +4284,7 @@ void Mount::testUART_vactual(TMC2209Stepper *driver, int _speed, int _duration) ///////////////////////////////// float Mount::checkRALimit() { - const float trackedHours = (_stepperTRK->currentPosition() / _trackingSpeed) / 3600.0F; // steps / steps/s / 3600 = hours + const float trackedHours = (_trackingSpeed > 0) ? ((_stepperTRK->currentPosition() / _trackingSpeed) / 3600.0F) : 0.0F; // steps / steps/s / 3600 = hours const float homeRA = _zeroPosRA.getTotalHours() + trackedHours; const float RALimit = RA_TRACKING_LIMIT; LOG(DEBUG_MOUNT_VERBOSE, diff --git a/src/Mount.hpp b/src/Mount.hpp index 7cf00a73..e5328330 100644 --- a/src/Mount.hpp +++ b/src/Mount.hpp @@ -656,14 +656,14 @@ class Mount EndSwitch *_decEndSwitch; #endif - unsigned long _guideRaEndTime; - unsigned long _guideDecEndTime; + volatile unsigned long _guideRaEndTime; + volatile unsigned long _guideDecEndTime; unsigned long _lastMountPrint = 0; float _trackingSpeed; // RA u-steps/sec when in tracking mode float _trackingSpeedCalibration; // Dimensionless, very close to 1.0 unsigned long _lastDisplayUpdate; - unsigned long _trackerStoppedAt; - bool _compensateForTrackerOff; + volatile unsigned long _trackerStoppedAt; + volatile bool _compensateForTrackerOff; volatile int _mountStatus; char scratchBuffer[24]; diff --git a/src/WifiControl.cpp b/src/WifiControl.cpp index a3dedabb..d9299b41 100644 --- a/src/WifiControl.cpp +++ b/src/WifiControl.cpp @@ -129,10 +129,12 @@ void WifiControl::loop() LOG(DEBUG_WIFI, "[WIFI]: Connected status changed to %s", wifiStatus(_status).c_str()); if (_status == WL_CONNECTED) { + delete _tcpServer; _tcpServer = new WiFiServer(WIFI_PORT); _tcpServer->begin(); _tcpServer->setNoDelay(true); + delete _udp; _udp = new WiFiUDP(); _udp->begin(4031); @@ -226,8 +228,8 @@ void WifiControl::udpLoop() packetSize, _udp->remoteIP().toString().c_str(), _udp->remotePort()); - char incomingPacket[255]; - int len = _udp->read(incomingPacket, 255); + char incomingPacket[256]; + int len = _udp->read(incomingPacket, sizeof(incomingPacket) - 1); incomingPacket[len] = 0; LOG(DEBUG_WIFI, "[WIFIUDP]: Received: %s", incomingPacket); diff --git a/src/WifiControl.hpp b/src/WifiControl.hpp index c42a24d2..943aec2e 100644 --- a/src/WifiControl.hpp +++ b/src/WifiControl.hpp @@ -34,8 +34,8 @@ class WifiControl LcdMenu *_lcdMenu; MeadeCommandProcessor *_cmdProcessor; - WiFiServer *_tcpServer; - WiFiUDP *_udp; + WiFiServer *_tcpServer = nullptr; + WiFiUDP *_udp = nullptr; WiFiClient client; unsigned long _infraStart = 0; From 01baab8a50a63f22a81b4a07d5d00367317ac039 Mon Sep 17 00:00:00 2001 From: Lutz Date: Sat, 14 Mar 2026 17:01:36 -0700 Subject: [PATCH 2/7] Lint fixes --- src/DayTime.cpp | 2 +- src/Declination.cpp | 2 +- src/Mount.cpp | 13 ++++++++----- 3 files changed, 10 insertions(+), 7 deletions(-) diff --git a/src/DayTime.cpp b/src/DayTime.cpp index eab4519e..cd698962 100644 --- a/src/DayTime.cpp +++ b/src/DayTime.cpp @@ -238,7 +238,7 @@ const char *DayTime::ToString() const *p++ = '0' + (secs / 10); } - *p++ = '0' + (secs % 10); + *p++ = '0' + (secs % 10); size_t used = p - achBuf; snprintf(p, sizeof(achBuf) - used, " (%.5f)", this->getTotalHours()); return achBuf; diff --git a/src/Declination.cpp b/src/Declination.cpp index a43ca10b..9a8a76c8 100644 --- a/src/Declination.cpp +++ b/src/Declination.cpp @@ -84,7 +84,7 @@ const char *Declination::ToString() const { ToDisplayString('*', ':'); - size_t used = strlen(achBufDeg); + size_t used = strlen(achBufDeg); float displayVal = inNorthernHemisphere ? 90 - fabsf(getTotalHours()) : -90 + fabsf(getTotalHours()); snprintf(achBufDeg + used, sizeof(achBufDeg) - used, " (%.4f, %.4f)", displayVal, getTotalHours()); diff --git a/src/Mount.cpp b/src/Mount.cpp index 68e43f86..2ee9ab4c 100644 --- a/src/Mount.cpp +++ b/src/Mount.cpp @@ -2590,7 +2590,8 @@ void Mount::startSlewing(int direction) setStatusFlag(STATUS_SLEWING); } - const float trackedHours = (_trackingSpeed > 0) ? ((_stepperTRK->currentPosition() / _trackingSpeed) / 3600.0F) : 0.0F; // steps / steps/s / 3600 = hours + const float trackedHours = (_trackingSpeed > 0) ? ((_stepperTRK->currentPosition() / _trackingSpeed) / 3600.0F) + : 0.0F; // steps / steps/s / 3600 = hours if (direction & EAST) { // We need to subtract the distance tracked from the physical RA home coordinate @@ -3589,7 +3590,8 @@ void Mount::calculateRAandDECSteppers(long &targetRASteps, long &targetDECSteps, LOG(DEBUG_COORD_CALC, "[MOUNT]: CalcSteppersIn: moveRA (target) is %f", moveRA); // Total hours of tracking-to-date - float trackedHours = (_trackingSpeed > 0) ? ((_stepperTRK->currentPosition() / _trackingSpeed) / 3600.0F) : 0.0F; // steps / steps/s / 3600 = hours + float trackedHours + = (_trackingSpeed > 0) ? ((_stepperTRK->currentPosition() / _trackingSpeed) / 3600.0F) : 0.0F; // steps / steps/s / 3600 = hours LOG(DEBUG_COORD_CALC, "[MOUNT]: CalcSteppersIn: Tracked time is %l steps (%f h).", _stepperTRK->currentPosition(), trackedHours); // The current RA of the home position, taking tracking-to-date into account @@ -4284,9 +4286,10 @@ void Mount::testUART_vactual(TMC2209Stepper *driver, int _speed, int _duration) ///////////////////////////////// float Mount::checkRALimit() { - const float trackedHours = (_trackingSpeed > 0) ? ((_stepperTRK->currentPosition() / _trackingSpeed) / 3600.0F) : 0.0F; // steps / steps/s / 3600 = hours - const float homeRA = _zeroPosRA.getTotalHours() + trackedHours; - const float RALimit = RA_TRACKING_LIMIT; + const float trackedHours + = (_trackingSpeed > 0) ? ((_stepperTRK->currentPosition() / _trackingSpeed) / 3600.0F) : 0.0F; // steps / steps/s / 3600 = hours + const float homeRA = _zeroPosRA.getTotalHours() + trackedHours; + const float RALimit = RA_TRACKING_LIMIT; LOG(DEBUG_MOUNT_VERBOSE, "[MOUNT]: checkRALimit: homeRA: %f (ZeroPos: %f + TrkHrs: %f)", homeRA, From 4f35c589185486543a115791bdd96f6ac7b2d240 Mon Sep 17 00:00:00 2001 From: Lutz Date: Sun, 15 Mar 2026 11:31:36 -0700 Subject: [PATCH 3/7] Code Review comments --- src/DayTime.cpp | 8 +++++--- src/Declination.cpp | 5 ++++- src/Gyro.cpp | 12 ++---------- 3 files changed, 11 insertions(+), 14 deletions(-) diff --git a/src/DayTime.cpp b/src/DayTime.cpp index cd698962..b8345b13 100644 --- a/src/DayTime.cpp +++ b/src/DayTime.cpp @@ -238,9 +238,11 @@ const char *DayTime::ToString() const *p++ = '0' + (secs / 10); } - *p++ = '0' + (secs % 10); - size_t used = p - achBuf; - snprintf(p, sizeof(achBuf) - used, " (%.5f)", this->getTotalHours()); + *p++ = '0' + (secs % 10); + size_t used = p - achBuf; + size_t remaining = sizeof(achBuf) - used; + String floatStr = String(this->getTotalHours(), 5); + snprintf(p, remaining, " (%s)", floatStr.c_str()); return achBuf; } void DayTime::printTwoDigits(char *achDegs, int num) const diff --git a/src/Declination.cpp b/src/Declination.cpp index 9a8a76c8..3970f07f 100644 --- a/src/Declination.cpp +++ b/src/Declination.cpp @@ -85,8 +85,11 @@ const char *Declination::ToString() const ToDisplayString('*', ':'); size_t used = strlen(achBufDeg); + size_t remaining = sizeof(achBufDeg) - used; float displayVal = inNorthernHemisphere ? 90 - fabsf(getTotalHours()) : -90 + fabsf(getTotalHours()); - snprintf(achBufDeg + used, sizeof(achBufDeg) - used, " (%.4f, %.4f)", displayVal, getTotalHours()); + String valStr = String(displayVal, 4); + String hoursStr = String(getTotalHours(), 4); + snprintf(achBufDeg + used, remaining, " (%s, %s)", valStr.c_str(), hoursStr.c_str()); return achBufDeg; } diff --git a/src/Gyro.cpp b/src/Gyro.cpp index f084f1a9..4f0f4cb0 100644 --- a/src/Gyro.cpp +++ b/src/Gyro.cpp @@ -91,17 +91,9 @@ angle_t Gyro::getCurrentAngles() int16_t AcZ = Wire.read() << 8 | Wire.read(); // Z-axis value // Calculating the Pitch angle (rotation around Y-axis) - float denomPitch = sqrtf((float) AcY * AcY + (float) AcZ * AcZ); - if (denomPitch > 0.0f) - { - result.pitchAngle += atanf(-1.0f * AcX / denomPitch) * 180.0f / static_cast(PI); - } + result.pitchAngle += atan2f(-1.0f * AcX, sqrtf((float) AcY * AcY + (float) AcZ * AcZ)) * 180.0f / static_cast(PI); // Calculating the Roll angle (rotation around X-axis) - float denomRoll = sqrtf((float) AcX * AcX + (float) AcZ * AcZ); - if (denomRoll > 0.0f) - { - result.rollAngle += atanf(-1.0f * AcY / denomRoll) * 180.0f / static_cast(PI); - } + result.rollAngle += atan2f(-1.0f * AcY, sqrtf((float) AcX * AcX + (float) AcZ * AcZ)) * 180.0f / static_cast(PI); delay(10); // Decorrelate measurements } From ae39b2d2ea833cf89c61f4842f69c443949e143d Mon Sep 17 00:00:00 2001 From: Lutz Date: Sun, 15 Mar 2026 11:33:40 -0700 Subject: [PATCH 4/7] Lint fix --- src/DayTime.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/DayTime.cpp b/src/DayTime.cpp index b8345b13..1197f256 100644 --- a/src/DayTime.cpp +++ b/src/DayTime.cpp @@ -238,10 +238,10 @@ const char *DayTime::ToString() const *p++ = '0' + (secs / 10); } - *p++ = '0' + (secs % 10); - size_t used = p - achBuf; + *p++ = '0' + (secs % 10); + size_t used = p - achBuf; size_t remaining = sizeof(achBuf) - used; - String floatStr = String(this->getTotalHours(), 5); + String floatStr = String(this->getTotalHours(), 5); snprintf(p, remaining, " (%s)", floatStr.c_str()); return achBuf; } From 921ef004571e8f6461d6fb870bcec5186fefeb59 Mon Sep 17 00:00:00 2001 From: Lutz Date: Sun, 15 Mar 2026 12:09:02 -0700 Subject: [PATCH 5/7] Fixes to make Gyro more stable - Changed to using moving window average (8 samples every 5ms) without blocking. --- src/Gyro.cpp | 89 +++++++++++++++++++++++++++++++++++++--------------- src/Gyro.hpp | 13 +++++++- 2 files changed, 76 insertions(+), 26 deletions(-) diff --git a/src/Gyro.cpp b/src/Gyro.cpp index 4f0f4cb0..a1b4fa22 100644 --- a/src/Gyro.cpp +++ b/src/Gyro.cpp @@ -16,6 +16,11 @@ POP_NO_WARNINGS * */ bool Gyro::isPresent(false); +float Gyro::_pitchSamples[WINDOW_SIZE] = {}; +float Gyro::_rollSamples[WINDOW_SIZE] = {}; +int Gyro::_ringIndex(0); +int Gyro::_samplesCollected(0); +unsigned long Gyro::_lastSampleTime(0); void Gyro::startup() /* Starts up the MPU-6050 device. @@ -54,6 +59,16 @@ void Gyro::startup() Wire.write(6); // 5Hz bandwidth (lowest) for smoothing Wire.endTransmission(true); + // Reset moving average state + for (int i = 0; i < WINDOW_SIZE; i++) + { + _pitchSamples[i] = 0; + _rollSamples[i] = 0; + } + _ringIndex = 0; + _samplesCollected = 0; + _lastSampleTime = 0; + LOG(DEBUG_INFO, "[GYRO]:: Started"); } @@ -66,45 +81,69 @@ void Gyro::shutdown() // Nothing to do } +void Gyro::collectSample() +/* Reads one accelerometer sample and stores it in the circular buffer. +*/ +{ + // Execute 6 byte read from MPU6050_REG_ACCEL_XOUT_H + Wire.beginTransmission(MPU6050_I2C_ADDR); + Wire.write(MPU6050_REG_ACCEL_XOUT_H); + Wire.endTransmission(false); + Wire.requestFrom(MPU6050_I2C_ADDR, 6, 1); // Read 6 registers total, each axis value is stored in 2 registers + int16_t AcX = Wire.read() << 8 | Wire.read(); // X-axis value + int16_t AcY = Wire.read() << 8 | Wire.read(); // Y-axis value + int16_t AcZ = Wire.read() << 8 | Wire.read(); // Z-axis value + + // Calculating the Pitch angle (rotation around Y-axis) + _pitchSamples[_ringIndex] = atan2f(-1.0f * AcX, sqrtf((float) AcY * AcY + (float) AcZ * AcZ)) * 180.0f / static_cast(PI); + // Calculating the Roll angle (rotation around X-axis) + _rollSamples[_ringIndex] = atan2f(-1.0f * AcY, sqrtf((float) AcX * AcX + (float) AcZ * AcZ)) * 180.0f / static_cast(PI); + + _ringIndex = (_ringIndex + 1) % WINDOW_SIZE; + if (_samplesCollected < WINDOW_SIZE) + { + _samplesCollected++; + } +} + angle_t Gyro::getCurrentAngles() /* Returns roll & tilt angles from MPU-6050 device in angle_t object in degrees. - If MPU-6050 is not found then returns {0,0}. + Non-blocking: collects one sample per call if at least SAMPLE_INTERVAL ms + have elapsed since the last sample. Returns the moving average over the + last WINDOW_SIZE samples. */ { - const int windowSize = 16; - // Read the accelerometer data - struct angle_t result; - result.pitchAngle = 0; - result.rollAngle = 0; + angle_t result; + if (!isPresent) - return result; // Gyro is not available + return result; + + unsigned long now = millis(); + if (now - _lastSampleTime >= SAMPLE_INTERVAL) + { + _lastSampleTime = now; + collectSample(); + } - for (int i = 0; i < windowSize; i++) + if (_samplesCollected == 0) + return result; + + float pitchSum = 0; + float rollSum = 0; + for (int i = 0; i < _samplesCollected; i++) { - // Execute 6 byte read from MPU6050_REG_WHO_AM_I - Wire.beginTransmission(MPU6050_I2C_ADDR); - Wire.write(MPU6050_REG_ACCEL_XOUT_H); - Wire.endTransmission(false); - Wire.requestFrom(MPU6050_I2C_ADDR, 6, 1); // Read 6 registers total, each axis value is stored in 2 registers - int16_t AcX = Wire.read() << 8 | Wire.read(); // X-axis value - int16_t AcY = Wire.read() << 8 | Wire.read(); // Y-axis value - int16_t AcZ = Wire.read() << 8 | Wire.read(); // Z-axis value - - // Calculating the Pitch angle (rotation around Y-axis) - result.pitchAngle += atan2f(-1.0f * AcX, sqrtf((float) AcY * AcY + (float) AcZ * AcZ)) * 180.0f / static_cast(PI); - // Calculating the Roll angle (rotation around X-axis) - result.rollAngle += atan2f(-1.0f * AcY, sqrtf((float) AcX * AcX + (float) AcZ * AcZ)) * 180.0f / static_cast(PI); - - delay(10); // Decorrelate measurements + pitchSum += _pitchSamples[i]; + rollSum += _rollSamples[i]; } + result.pitchAngle = pitchSum / _samplesCollected; + result.rollAngle = rollSum / _samplesCollected; - result.pitchAngle /= windowSize; - result.rollAngle /= windowSize; #if GYRO_AXIS_SWAP == 1 float temp = result.pitchAngle; result.pitchAngle = result.rollAngle; result.rollAngle = temp; #endif + return result; } diff --git a/src/Gyro.hpp b/src/Gyro.hpp index 563a21f5..10d3cd71 100644 --- a/src/Gyro.hpp +++ b/src/Gyro.hpp @@ -15,6 +15,8 @@ class Gyro static float getCurrentTemperature(); private: + static void collectSample(); + // MPU6050 constants enum { @@ -28,6 +30,15 @@ class Gyro MPU6050_REG_WHO_AM_I = 0x75 }; - static bool isPresent; // True if gyro correctly detected on startup + // Use about 72 bytes of RAM for the moving average buffer + static const int WINDOW_SIZE = 8; // Number of samples to keep for moving average + static const unsigned long SAMPLE_INTERVAL = 5; // ms between samples + + static bool isPresent; + static float _pitchSamples[WINDOW_SIZE]; + static float _rollSamples[WINDOW_SIZE]; + static int _ringIndex; + static int _samplesCollected; // Counts up to WINDOW_SIZE, then stays there + static unsigned long _lastSampleTime; }; #endif From 246d2d960f111f6d4eff1e7f8b597d9ca8e39b5e Mon Sep 17 00:00:00 2001 From: Lutz Date: Sun, 15 Mar 2026 15:57:09 -0700 Subject: [PATCH 6/7] Fixed excessive memory thrashing (string->char*) --- src/MeadeCommandProcessor.cpp | 192 +++++++++++++++++++--------------- src/MeadeCommandProcessor.hpp | 34 +++--- src/WifiControl.cpp | 8 +- src/f_serial.hpp | 6 +- src/testmenu.cpp | 4 +- 5 files changed, 138 insertions(+), 106 deletions(-) diff --git a/src/MeadeCommandProcessor.cpp b/src/MeadeCommandProcessor.cpp index 5b1ad708..24dfda39 100644 --- a/src/MeadeCommandProcessor.cpp +++ b/src/MeadeCommandProcessor.cpp @@ -7,6 +7,9 @@ #include "WifiControl.hpp" #include "Gyro.hpp" +#include +#include + #if USE_GPS == 1 bool gpsAqcuisitionComplete(int &indicator); // defined in c72_menuHA_GPS.hpp #endif @@ -1211,6 +1214,29 @@ bool gpsAqcuisitionComplete(int &indicator); // defined in c72_menuHA_GPS.hpp ///////////////////////////////////////////////////////////////////////////////////////// MeadeCommandProcessor *MeadeCommandProcessor::_instance = nullptr; +char MeadeCommandProcessor::_responseBuffer[200] = {}; + +const char *MeadeCommandProcessor::copyToResponse(const char *src) +{ + strlcpy(_responseBuffer, src, sizeof(_responseBuffer)); + return _responseBuffer; +} + +const char *MeadeCommandProcessor::formatResponse(const char *fmt, ...) +{ + va_list args; + va_start(args, fmt); + vsnprintf(_responseBuffer, sizeof(_responseBuffer), fmt, args); + va_end(args); + return _responseBuffer; +} + +const char *MeadeCommandProcessor::copyToResponse_P(const char *pgmSrc) +{ + strncpy_P(_responseBuffer, pgmSrc, sizeof(_responseBuffer) - 1); + _responseBuffer[sizeof(_responseBuffer) - 1] = '\0'; + return _responseBuffer; +} ///////////////////////////// // Create the processor @@ -1243,7 +1269,7 @@ MeadeCommandProcessor::MeadeCommandProcessor(Mount *mount, LcdMenu *lcdMenu) ///////////////////////////// // INIT ///////////////////////////// -String MeadeCommandProcessor::handleMeadeInit(String inCmd) +const char *MeadeCommandProcessor::handleMeadeInit(const String &inCmd) { inSerialControl = true; _lcdMenu->setCursor(0, 0); @@ -1256,7 +1282,7 @@ String MeadeCommandProcessor::handleMeadeInit(String inCmd) ///////////////////////////// // GET INFO ///////////////////////////// -String MeadeCommandProcessor::handleMeadeGetInfo(String inCmd) +const char *MeadeCommandProcessor::handleMeadeGetInfo(const String &inCmd) { if (inCmd.length() < 1) { @@ -1271,7 +1297,7 @@ String MeadeCommandProcessor::handleMeadeGetInfo(String inCmd) case 'V': if (cmdTwo == 'N') // :GVN { - return String(VERSION) + "#"; + return formatResponse("%s#", VERSION); } else if (cmdTwo == 'P') // :GVP { @@ -1285,47 +1311,47 @@ String MeadeCommandProcessor::handleMeadeGetInfo(String inCmd) } break; - case 'r': // :Gr - return _mount->RAString(MEADE_STRING | TARGET_STRING); // returns trailing # + case 'r': // :Gr + return copyToResponse(_mount->RAString(MEADE_STRING | TARGET_STRING).c_str()); // returns trailing # - case 'd': // :Gd - return _mount->DECString(MEADE_STRING | TARGET_STRING); // returns trailing # + case 'd': // :Gd + return copyToResponse(_mount->DECString(MEADE_STRING | TARGET_STRING).c_str()); // returns trailing # - case 'R': // :GR - return _mount->RAString(MEADE_STRING | CURRENT_STRING); // returns trailing # + case 'R': // :GR + return copyToResponse(_mount->RAString(MEADE_STRING | CURRENT_STRING).c_str()); // returns trailing # - case 'D': // :GD - return _mount->DECString(MEADE_STRING | CURRENT_STRING); // returns trailing # + case 'D': // :GD + return copyToResponse(_mount->DECString(MEADE_STRING | CURRENT_STRING).c_str()); // returns trailing # case 'X': // :GX - return _mount->getStatusString() + "#"; + return formatResponse("%s#", _mount->getStatusString().c_str()); case 'I': { - String retVal = ""; + const char *val = ""; if (cmdTwo == 'S') // :GIS { - retVal = _mount->isSlewingRAorDEC() ? "1" : "0"; + val = _mount->isSlewingRAorDEC() ? "1" : "0"; } else if (cmdTwo == 'T') // :GIT { - retVal = _mount->isSlewingTRK() ? "1" : "0"; + val = _mount->isSlewingTRK() ? "1" : "0"; } else if (cmdTwo == 'G') // :GIG { - retVal = _mount->isGuiding() ? "1" : "0"; + val = _mount->isGuiding() ? "1" : "0"; } - return retVal + "#"; + return formatResponse("%s#", val); } case 't': // :Gt { _mount->latitude().formatString(achBuffer, "{d}*{m}#"); - return String(achBuffer); + return copyToResponse(achBuffer); } case 'g': // :Gg { _mount->longitude().formatStringForMeade(achBuffer); - return String(achBuffer) + "#"; + return formatResponse("%s#", achBuffer); } case 'c': // :Gc { @@ -1335,7 +1361,7 @@ String MeadeCommandProcessor::handleMeadeGetInfo(String inCmd) { int offset = _mount->getLocalUtcOffset(); snprintf(achBuffer, sizeof(achBuffer), "%+03d#", -offset); - return String(achBuffer); + return copyToResponse(achBuffer); } case 'a': // :Ga { @@ -1345,19 +1371,19 @@ String MeadeCommandProcessor::handleMeadeGetInfo(String inCmd) time.addHours(-12); } time.formatString(achBuffer, "{d}:{m}:{s}#"); - return String(achBuffer + 1); + return copyToResponse(achBuffer + 1); } case 'L': // :GL { DayTime time = _mount->getLocalTime(); time.formatString(achBuffer, "{d}:{m}:{s}#"); - return String(achBuffer + 1); + return copyToResponse(achBuffer + 1); } case 'C': // :GC { LocalDate date = _mount->getLocalDate(); snprintf(achBuffer, sizeof(achBuffer), "%02d/%02d/%02d#", date.month, date.day, date.year % 100); - return String(achBuffer); + return copyToResponse(achBuffer); } case 'M': // :GM { @@ -1387,7 +1413,7 @@ String MeadeCommandProcessor::handleMeadeGetInfo(String inCmd) ///////////////////////////// // GPS CONTROL ///////////////////////////// -String MeadeCommandProcessor::handleMeadeGPSCommands(String inCmd) +const char *MeadeCommandProcessor::handleMeadeGPSCommands(const String &inCmd) { if (inCmd.length() < 1) { @@ -1421,7 +1447,7 @@ String MeadeCommandProcessor::handleMeadeGPSCommands(String inCmd) ///////////////////////////// // SYNC CONTROL ///////////////////////////// -String MeadeCommandProcessor::handleMeadeSyncControl(String inCmd) +const char *MeadeCommandProcessor::handleMeadeSyncControl(const String &inCmd) { if (inCmd.length() < 1) { @@ -1439,7 +1465,7 @@ String MeadeCommandProcessor::handleMeadeSyncControl(String inCmd) ///////////////////////////// // SET INFO ///////////////////////////// -String MeadeCommandProcessor::handleMeadeSetInfo(String inCmd) +const char *MeadeCommandProcessor::handleMeadeSetInfo(const String &inCmd) { if (inCmd.length() < 1) { @@ -1561,10 +1587,10 @@ String MeadeCommandProcessor::handleMeadeSetInfo(String inCmd) /* From https://www.astro.louisville.edu/software/xmtel/archive/xmtel-indi-6.0/xmtel-6.0l/support/lx200/CommandSet.html : - SC: Calendar: If the date is valid 2 s are returned, each string is 31 bytes long. + SC: Calendar: If the date is valid 2 s are returned, each string is 31 bytes long. The first is: "Updating planetary data#" followed by a second string of 30 spaces terminated by '#' */ - return F("1Updating Planetary Data# #"); // + return copyToResponse_P(PSTR("1Updating Planetary Data# #")); } else { @@ -1575,7 +1601,7 @@ String MeadeCommandProcessor::handleMeadeSetInfo(String inCmd) ///////////////////////////// // MOVEMENT ///////////////////////////// -String MeadeCommandProcessor::handleMeadeMovement(String inCmd) +const char *MeadeCommandProcessor::handleMeadeMovement(const String &inCmd) { if (inCmd.length() < 1) { @@ -1616,16 +1642,18 @@ String MeadeCommandProcessor::handleMeadeMovement(String inCmd) if (inCmd.length() == 6) { byte direction = EAST; - inCmd.toLowerCase(); - if (inCmd[1] == 'n') + // inCmd is const ref, so make a local copy for toLowerCase + String lowerCmd = inCmd; + lowerCmd.toLowerCase(); + if (lowerCmd[1] == 'n') direction = NORTH; - else if (inCmd[1] == 's') + else if (lowerCmd[1] == 's') direction = SOUTH; - else if (inCmd[1] == 'e') + else if (lowerCmd[1] == 'e') direction = EAST; - else if (inCmd[1] == 'w') + else if (lowerCmd[1] == 'w') direction = WEST; - int duration = inCmd.substring(2, 6).toInt(); + int duration = lowerCmd.substring(2, 6).toInt(); _mount->guidePulse(direction, duration); return ""; } @@ -1748,7 +1776,7 @@ String MeadeCommandProcessor::handleMeadeMovement(String inCmd) ///////////////////////////// // HOME ///////////////////////////// -String MeadeCommandProcessor::handleMeadeHome(String inCmd) +const char *MeadeCommandProcessor::handleMeadeHome(const String &inCmd) { if (inCmd.length() < 1) { @@ -1775,7 +1803,7 @@ String MeadeCommandProcessor::handleMeadeHome(String inCmd) return ""; } -String MeadeCommandProcessor::handleMeadeDistance(String inCmd) +const char *MeadeCommandProcessor::handleMeadeDistance(const String &inCmd) { if (_mount->isSlewingRAorDEC()) { @@ -1787,7 +1815,7 @@ String MeadeCommandProcessor::handleMeadeDistance(String inCmd) ///////////////////////////// // EXTRA COMMANDS ///////////////////////////// -String MeadeCommandProcessor::handleMeadeExtraCommands(String inCmd) +const char *MeadeCommandProcessor::handleMeadeExtraCommands(const String &inCmd) { if (inCmd.length() < 1) { @@ -1827,7 +1855,7 @@ String MeadeCommandProcessor::handleMeadeExtraCommands(String inCmd) { // Get RA/DEC steps/deg, speedfactor if (inCmd[1] == 'R') // :XGR# { - return String(_mount->getStepsPerDegree(RA_STEPS), 1) + "#"; + return formatResponse("%s#", String(_mount->getStepsPerDegree(RA_STEPS), 1).c_str()); } else if (inCmd[1] == 'D') { @@ -1841,11 +1869,11 @@ String MeadeCommandProcessor::handleMeadeExtraCommands(String inCmd) { if (inCmd[3] == 'L') // :XGDLL# { - return String(loLimit, 1) + "#"; + return formatResponse("%s#", String(loLimit, 1).c_str()); } else if (inCmd[3] == 'U') // :XGDLU# { - return String(hiLimit, 1) + "#"; + return formatResponse("%s#", String(hiLimit, 1).c_str()); } else { @@ -1854,7 +1882,7 @@ String MeadeCommandProcessor::handleMeadeExtraCommands(String inCmd) } else // :XGDL# { - return String(loLimit, 1) + "|" + String(hiLimit, 1) + "#"; + return formatResponse("%s|%s#", String(loLimit, 1).c_str(), String(hiLimit, 1).c_str()); } } if (inCmd[2] == 'P') // :XGDP# @@ -1864,47 +1892,45 @@ String MeadeCommandProcessor::handleMeadeExtraCommands(String inCmd) } else // :XGD# { - return String(_mount->getStepsPerDegree(DEC_STEPS), 1) + "#"; + return formatResponse("%s#", String(_mount->getStepsPerDegree(DEC_STEPS), 1).c_str()); } } else if (inCmd[1] == 'S') { if (inCmd.length() == 2) // :XGS# { - return String(_mount->getSpeedCalibration(), 5) + "#"; + return formatResponse("%s#", String(_mount->getSpeedCalibration(), 5).c_str()); } else if ((inCmd.length() == 3) && (inCmd[2] == 'T')) // :XGST# { - return String(_mount->checkRALimit(), 7) + "#"; + return formatResponse("%s#", String(_mount->checkRALimit(), 7).c_str()); } } else if (inCmd[1] == 'T') // :XGT# { - return String(_mount->getSpeed(TRACKING), 7) + "#"; + return formatResponse("%s#", String(_mount->getSpeed(TRACKING), 7).c_str()); } else if (inCmd[1] == 'B') // :XGB# { - return String(_mount->getBacklashCorrection()) + "#"; + return formatResponse("%s#", String(_mount->getBacklashCorrection()).c_str()); } else if ((inCmd[1] == 'A') && (inCmd.length() == 2)) // :XGA# { - return String(_mount->getStepsPerDegree(ALTITUDE_STEPS), 1) + "#"; + return formatResponse("%s#", String(_mount->getStepsPerDegree(ALTITUDE_STEPS), 1).c_str()); } else if ((inCmd[1] == 'Z') && (inCmd.length() == 2)) // :XGZ# { - return String(_mount->getStepsPerDegree(AZIMUTH_STEPS), 1) + "#"; + return formatResponse("%s#", String(_mount->getStepsPerDegree(AZIMUTH_STEPS), 1).c_str()); } else if ((inCmd[1] == 'A') && (inCmd.length() > 2) && (inCmd[2] == 'H')) // :XGAH# { - return _mount->getAutoHomingStates() + "#"; + return formatResponse("%s#", _mount->getAutoHomingStates().c_str()); } else if ((inCmd[1] == 'A') && (inCmd.length() > 2) && (inCmd[2] == 'A')) // :XGAA# { long azPos, altPos; _mount->getAZALTPositions(azPos, altPos); - char scratchBuffer[24]; - snprintf(scratchBuffer, sizeof(scratchBuffer), "%ld|%ld#", azPos, altPos); - return String(scratchBuffer); + return formatResponse("%ld|%ld#", azPos, altPos); } else if (inCmd[1] == 'C') // :XGCn.nn*m.mm# { @@ -1916,22 +1942,20 @@ String MeadeCommandProcessor::handleMeadeExtraCommands(String inCmd) float raCoord = coords.substring(0, star).toFloat(); float decCoord = coords.substring(star + 1).toFloat(); _mount->calculateStepperPositions(raCoord, decCoord, raPos, decPos); - char scratchBuffer[24]; - snprintf(scratchBuffer, sizeof(scratchBuffer), "%ld|%ld#", raPos, decPos); - return String(scratchBuffer); + return formatResponse("%ld|%ld#", raPos, decPos); } } else if (inCmd[1] == 'M') { if ((inCmd.length() > 2) && (inCmd[2] == 'S')) // :XGMS# { - return _mount->getStepperInfo() + "#"; + return formatResponse("%s#", _mount->getStepperInfo().c_str()); } - return _mount->getMountHardwareInfo() + "#"; // :XGM# + return formatResponse("%s#", _mount->getMountHardwareInfo().c_str()); // :XGM# } else if (inCmd[1] == 'O') // :XGO# { - return getLogBuffer(); + return copyToResponse(getLogBuffer().c_str()); } else if (inCmd[1] == 'H') // :XGH# { @@ -1941,17 +1965,17 @@ String MeadeCommandProcessor::handleMeadeExtraCommands(String inCmd) if (inCmd[2] == 'R') // :XGHR# { LOG(DEBUG_MEADE, "[MEADE]: XGHR -> %s", inCmd.c_str()); - return String(_mount->getHomingOffset(StepperAxis::RA_STEPS)) + "#"; + return formatResponse("%ld#", _mount->getHomingOffset(StepperAxis::RA_STEPS)); } else if (inCmd[2] == 'D') // :XGHD# { LOG(DEBUG_MEADE, "[MEADE]: XGHD -> %s", inCmd.c_str()); - return String(_mount->getHomingOffset(StepperAxis::DEC_STEPS)) + "#"; + return formatResponse("%ld#", _mount->getHomingOffset(StepperAxis::DEC_STEPS)); } else if (inCmd[2] == 'S') // :XGHS# { LOG(DEBUG_MEADE, "[MEADE]: XGHS -> %s", inCmd.c_str()); - return String(inNorthernHemisphere ? "N#" : "S#"); + return inNorthernHemisphere ? "N#" : "S#"; } LOG(DEBUG_MEADE, "[MEADE]: XGH? -> %s", inCmd.c_str()); @@ -1959,23 +1983,19 @@ String MeadeCommandProcessor::handleMeadeExtraCommands(String inCmd) } else { - char scratchBuffer[10]; DayTime ha = _mount->calculateHa(); - snprintf(scratchBuffer, sizeof(scratchBuffer), "%02d%02d%02d#", ha.getHours(), ha.getMinutes(), ha.getSeconds()); - return String(scratchBuffer); + return formatResponse("%02d%02d%02d#", ha.getHours(), ha.getMinutes(), ha.getSeconds()); } } else if (inCmd[1] == 'L') // :XGL# { - char scratchBuffer[10]; DayTime lst = _mount->calculateLst(); - snprintf(scratchBuffer, sizeof(scratchBuffer), "%02d%02d%02d#", lst.getHours(), lst.getMinutes(), lst.getSeconds()); - return String(scratchBuffer); + return formatResponse("%02d%02d%02d#", lst.getHours(), lst.getMinutes(), lst.getSeconds()); } else if (inCmd[1] == 'N') // :XGN# { #if (WIFI_ENABLED == 1) - return wifiControl.getStatus() + "#"; + return formatResponse("%s#", wifiControl.getStatus().c_str()); #endif return "0,#"; @@ -2091,17 +2111,21 @@ String MeadeCommandProcessor::handleMeadeExtraCommands(String inCmd) { // get values if (inCmd[2] == 'R') // :XLGR { // get Calibration/Reference values - return String(_mount->getPitchCalibrationAngle(), 4) + "," + String(_mount->getRollCalibrationAngle(), 4) + "#"; + return formatResponse("%s,%s#", + String(_mount->getPitchCalibrationAngle(), 4).c_str(), + String(_mount->getRollCalibrationAngle(), 4).c_str()); } else if (inCmd[2] == 'C') // :XLGC { // Get current values auto angles = Gyro::getCurrentAngles(); - return String(angles.pitchAngle, 4) + "," + String(angles.rollAngle, 4) + "#"; + return formatResponse("%s,%s#", + String(angles.pitchAngle, 4).c_str(), + String(angles.rollAngle, 4).c_str()); } else if (inCmd[2] == 'T') // :XLGT { // Get current temp float temp = Gyro::getCurrentTemperature(); - return String(temp, 1) + "#"; + return formatResponse("%s#", String(temp, 1).c_str()); } } else if (inCmd[1] == 'S') @@ -2109,35 +2133,35 @@ String MeadeCommandProcessor::handleMeadeExtraCommands(String inCmd) if (inCmd[2] == 'P') // :XLSP { // get Calibration/Reference values _mount->setPitchCalibrationAngle(inCmd.substring(3).toFloat()); - return String("1#"); + return "1#"; } else if (inCmd[2] == 'R') // :XLSR { _mount->setRollCalibrationAngle(inCmd.substring(3).toFloat()); - return String("1#"); + return "1#"; } } else if (inCmd[1] == '1') // :XL1 { // Turn on Gyro Gyro::startup(); - return String("1#"); + return "1#"; } else if (inCmd[1] == '0') // :XL0 { // Turn off Gyro Gyro::shutdown(); - return String("1#"); + return "1#"; } else { - return "Unknown Level command: X" + inCmd; + return formatResponse("Unknown Level command: X%s", inCmd.c_str()); } #endif - return String("0#"); + return "0#"; } else if ((inCmd[0] == 'F') && (inCmd[1] == 'R')) { _mount->clearConfiguration(); // :XFR - return String("1#"); + return "1#"; } return ""; @@ -2146,7 +2170,7 @@ String MeadeCommandProcessor::handleMeadeExtraCommands(String inCmd) ///////////////////////////// // QUIT ///////////////////////////// -String MeadeCommandProcessor::handleMeadeQuit(String inCmd) +const char *MeadeCommandProcessor::handleMeadeQuit(const String &inCmd) { // :Q# stops a motors - remains in Control mode // :Qq# command does not stop motors, but quits Control mode @@ -2190,7 +2214,7 @@ String MeadeCommandProcessor::handleMeadeQuit(String inCmd) ///////////////////////////// // Set Slew Rates ///////////////////////////// -String MeadeCommandProcessor::handleMeadeSetSlewRate(String inCmd) +const char *MeadeCommandProcessor::handleMeadeSetSlewRate(const String &inCmd) { if (inCmd.length() < 1) { @@ -2219,7 +2243,7 @@ String MeadeCommandProcessor::handleMeadeSetSlewRate(String inCmd) ///////////////////////////// // FOCUS COMMANDS ///////////////////////////// -String MeadeCommandProcessor::handleMeadeFocusCommands(String inCmd) +const char *MeadeCommandProcessor::handleMeadeFocusCommands(const String &inCmd) { if (inCmd.length() < 1) { @@ -2262,7 +2286,7 @@ String MeadeCommandProcessor::handleMeadeFocusCommands(String inCmd) { LOG(DEBUG_MEADE, "[MEADE]: Focus get stepperPosition"); long focusPos = _mount->focusGetStepperPosition(); - return String(focusPos) + "#"; + return formatResponse("%ld#", focusPos); } else if (inCmd[0] == 'P') // :FPnnn { @@ -2295,7 +2319,7 @@ String MeadeCommandProcessor::handleMeadeFocusCommands(String inCmd) return ""; } -String MeadeCommandProcessor::processCommand(String inCmd) +const char *MeadeCommandProcessor::processCommand(String inCmd) { if (inCmd.length() < 2) { diff --git a/src/MeadeCommandProcessor.hpp b/src/MeadeCommandProcessor.hpp index b8861f17..2c0d93a0 100644 --- a/src/MeadeCommandProcessor.hpp +++ b/src/MeadeCommandProcessor.hpp @@ -1,5 +1,7 @@ #pragma once +#include + // Forward declarations class Mount; class LcdMenu; @@ -9,23 +11,29 @@ class MeadeCommandProcessor public: static MeadeCommandProcessor *createProcessor(Mount *mount, LcdMenu *lcdMenu); static MeadeCommandProcessor *instance(); - String processCommand(String incmd); + const char *processCommand(String inCmd); private: MeadeCommandProcessor(Mount *mount, LcdMenu *lcdMenu); - String handleMeadeSetInfo(String inCmd); - String handleMeadeMovement(String inCmd); - String handleMeadeGetInfo(String inCmd); - String handleMeadeGPSCommands(String inCmd); - String handleMeadeSyncControl(String inCmd); - String handleMeadeHome(String inCmd); - String handleMeadeInit(String inCmd); - String handleMeadeQuit(String inCmd); - String handleMeadeDistance(String inCmd); - String handleMeadeSetSlewRate(String inCmd); - String handleMeadeExtraCommands(String inCmd); - String handleMeadeFocusCommands(String inCmd); + const char *handleMeadeSetInfo(const String &inCmd); + const char *handleMeadeMovement(const String &inCmd); + const char *handleMeadeGetInfo(const String &inCmd); + const char *handleMeadeGPSCommands(const String &inCmd); + const char *handleMeadeSyncControl(const String &inCmd); + const char *handleMeadeHome(const String &inCmd); + const char *handleMeadeInit(const String &inCmd); + const char *handleMeadeQuit(const String &inCmd); + const char *handleMeadeDistance(const String &inCmd); + const char *handleMeadeSetSlewRate(const String &inCmd); + const char *handleMeadeExtraCommands(const String &inCmd); + const char *handleMeadeFocusCommands(const String &inCmd); + + static const char *copyToResponse(const char *src); + static const char *formatResponse(const char *fmt, ...); + static const char *copyToResponse_P(const char *pgmSrc); + Mount *_mount; LcdMenu *_lcdMenu; static MeadeCommandProcessor *_instance; + static char _responseBuffer[200]; }; diff --git a/src/WifiControl.cpp b/src/WifiControl.cpp index d9299b41..e3c1dacb 100644 --- a/src/WifiControl.cpp +++ b/src/WifiControl.cpp @@ -191,12 +191,12 @@ void WifiControl::tcpLoop() { String cmd = client.readStringUntil('#'); LOG(DEBUG_WIFI, "[WIFITCP]: Query <-- %s#", cmd.c_str()); - String retVal = _cmdProcessor->processCommand(cmd); + const char *retVal = _cmdProcessor->processCommand(cmd); - if (retVal != "") + if (retVal[0] != '\0') { - client.write(retVal.c_str()); - LOG(DEBUG_WIFI, "[WIFITCP]: Reply --> %s", retVal.c_str()); + client.write(retVal); + LOG(DEBUG_WIFI, "[WIFITCP]: Reply --> %s", retVal); } else { diff --git a/src/f_serial.hpp b/src/f_serial.hpp index ad3741a7..7f8458a1 100644 --- a/src/f_serial.hpp +++ b/src/f_serial.hpp @@ -132,10 +132,10 @@ void processSerialData() const String inCmd = String(buffer); LOG(DEBUG_SERIAL, "[SERIAL]: ReceivedCommand(%d chars): [%s]", inCmd.length(), inCmd.c_str()); - const String retVal = MeadeCommandProcessor::instance()->processCommand(inCmd); - if (retVal != "") + const char *retVal = MeadeCommandProcessor::instance()->processCommand(inCmd); + if (retVal[0] != '\0') { - LOG(DEBUG_SERIAL, "[SERIAL]: RepliedWith: [%s]", retVal.c_str()); + LOG(DEBUG_SERIAL, "[SERIAL]: RepliedWith: [%s]", retVal); // When not debugging, print the result to the serial port . // When debugging, only print the result to Serial if we're on seperate ports. #if (DEBUG_LEVEL == DEBUG_NONE) || (DEBUG_SEPARATE_SERIAL == 1) diff --git a/src/testmenu.cpp b/src/testmenu.cpp index 1b0d1763..2649ea7d 100644 --- a/src/testmenu.cpp +++ b/src/testmenu.cpp @@ -336,9 +336,9 @@ void TestMenu::onCommandReceived(String s) s = s.substring(0, s.length() - 1); } - String reply = MeadeCommandProcessor::instance()->processCommand(s); + const char *reply = MeadeCommandProcessor::instance()->processCommand(s); - if (reply.length() > 0) + if (reply[0] != '\0') { Serial.println(F("-- Command Response --------")); Serial.println(reply); From 95decf14a0edb1526438e99e29d921d8e80ccf25 Mon Sep 17 00:00:00 2001 From: Lutz Date: Sun, 15 Mar 2026 15:58:33 -0700 Subject: [PATCH 7/7] PR feedback - Removed votatile from variables that aren't accessed in other threads - Extracted common function that calculates elapsed tracking time - Created function to set status to a specific value in a thread-safe way --- src/Mount.cpp | 25 +++++++++++++++++-------- src/Mount.hpp | 12 ++++++++---- 2 files changed, 25 insertions(+), 12 deletions(-) diff --git a/src/Mount.cpp b/src/Mount.cpp index 2ee9ab4c..7dd46299 100644 --- a/src/Mount.cpp +++ b/src/Mount.cpp @@ -2279,6 +2279,13 @@ void Mount::setTrackingStepperPos(long stepPos) _stepperTRK->setCurrentPosition(stepPos); } +void Mount::setStatus(int state) +{ + noInterrupts(); + _mountStatus = state; + interrupts(); +} + void Mount::setStatusFlag(int flag) { noInterrupts(); @@ -2590,8 +2597,7 @@ void Mount::startSlewing(int direction) setStatusFlag(STATUS_SLEWING); } - const float trackedHours = (_trackingSpeed > 0) ? ((_stepperTRK->currentPosition() / _trackingSpeed) / 3600.0F) - : 0.0F; // steps / steps/s / 3600 = hours + const float trackedHours = getTrackedHours(); if (direction & EAST) { // We need to subtract the distance tracked from the physical RA home coordinate @@ -3279,7 +3285,7 @@ void Mount::loop() { LOG(DEBUG_MOUNT | DEBUG_STEPPERS, "[MOUNT]: Loop: Already at Parking pos, so done."); noInterrupts(); - _mountStatus = STATUS_PARKED; + setStatus(STATUS_PARKED); interrupts(); } } @@ -3299,7 +3305,7 @@ void Mount::loop() { LOG(DEBUG_MOUNT | DEBUG_STEPPERS, "[MOUNT]: Loop: Arrived at park position..."); noInterrupts(); - _mountStatus = STATUS_PARKED; + setStatus(STATUS_PARKED); interrupts(); _slewingToPark = false; } @@ -3590,8 +3596,7 @@ void Mount::calculateRAandDECSteppers(long &targetRASteps, long &targetDECSteps, LOG(DEBUG_COORD_CALC, "[MOUNT]: CalcSteppersIn: moveRA (target) is %f", moveRA); // Total hours of tracking-to-date - float trackedHours - = (_trackingSpeed > 0) ? ((_stepperTRK->currentPosition() / _trackingSpeed) / 3600.0F) : 0.0F; // steps / steps/s / 3600 = hours + float trackedHours = getTrackedHours(); LOG(DEBUG_COORD_CALC, "[MOUNT]: CalcSteppersIn: Tracked time is %l steps (%f h).", _stepperTRK->currentPosition(), trackedHours); // The current RA of the home position, taking tracking-to-date into account @@ -4279,6 +4284,11 @@ void Mount::testUART_vactual(TMC2209Stepper *driver, int _speed, int _duration) #endif #endif +float Mount::getTrackedHours() +{ + return (_trackingSpeed > 0) ? ((_stepperTRK->currentPosition() / _trackingSpeed) / 3600.0F) : 0.0F; // steps / steps/s / 3600 = hours +} + ///////////////////////////////// // // checkRALimit @@ -4286,8 +4296,7 @@ void Mount::testUART_vactual(TMC2209Stepper *driver, int _speed, int _duration) ///////////////////////////////// float Mount::checkRALimit() { - const float trackedHours - = (_trackingSpeed > 0) ? ((_stepperTRK->currentPosition() / _trackingSpeed) / 3600.0F) : 0.0F; // steps / steps/s / 3600 = hours + const float trackedHours = getTrackedHours(); const float homeRA = _zeroPosRA.getTotalHours() + trackedHours; const float RALimit = RA_TRACKING_LIMIT; LOG(DEBUG_MOUNT_VERBOSE, diff --git a/src/Mount.hpp b/src/Mount.hpp index e5328330..a8594661 100644 --- a/src/Mount.hpp +++ b/src/Mount.hpp @@ -264,6 +264,9 @@ class Mount void setSlewRate(int rate); int getSlewRate(); + // Get the number of hours we've been tracking + float getTrackedHours(); + // Set the HA time (HA is derived from LST, the setter calculates and sets LST) void setHA(const DayTime &haTime); const DayTime HA() const; @@ -391,6 +394,7 @@ class Mount // Returns a comma-delimited string with all the mounts' information String getStatusString(); + void setStatus(int state); void setStatusFlag(int flag); void clearStatusFlag(int flag); @@ -656,14 +660,14 @@ class Mount EndSwitch *_decEndSwitch; #endif - volatile unsigned long _guideRaEndTime; - volatile unsigned long _guideDecEndTime; + unsigned long _guideRaEndTime; + unsigned long _guideDecEndTime; unsigned long _lastMountPrint = 0; float _trackingSpeed; // RA u-steps/sec when in tracking mode float _trackingSpeedCalibration; // Dimensionless, very close to 1.0 unsigned long _lastDisplayUpdate; - volatile unsigned long _trackerStoppedAt; - volatile bool _compensateForTrackerOff; + unsigned long _trackerStoppedAt; + bool _compensateForTrackerOff; volatile int _mountStatus; char scratchBuffer[24];