Harden safety-critical paths from Hamilton review
- Fix lock hierarchy: stopAll() cancels pulse before touching axes - Add configASSERT bounds checks on axis index in move/pulseGuide - Enforce ST4Pulse singleton with configASSERT - Check esp_timer_start_once return, rollback hardware on failure - Validate SYNC coordinates (reject garbage → silent 0.0) - Discard truncated serial commands on buffer overflow - Guard WiFi update()/broadcastState() against null ws_ pointer - Report connection errors to WebSocket clients on move/pulse - Remove redundant Serial.begin() from pulse_guide example
This commit is contained in:
parent
4c91fd4811
commit
35ec6dfdb5
@ -14,15 +14,13 @@ ST4Controller controller;
|
|||||||
ST4Serial st4Serial;
|
ST4Serial st4Serial;
|
||||||
|
|
||||||
void setup() {
|
void setup() {
|
||||||
Serial.begin(115200);
|
|
||||||
Serial.println("ST4 Pulse Guide Example");
|
|
||||||
|
|
||||||
controller.begin(
|
controller.begin(
|
||||||
ST4_PIN_RA_PLUS, ST4_PIN_RA_MINUS,
|
ST4_PIN_RA_PLUS, ST4_PIN_RA_MINUS,
|
||||||
ST4_PIN_DEC_PLUS, ST4_PIN_DEC_MINUS,
|
ST4_PIN_DEC_PLUS, ST4_PIN_DEC_MINUS,
|
||||||
ST4_PIN_LED
|
ST4_PIN_LED
|
||||||
);
|
);
|
||||||
|
|
||||||
|
// st4Serial.begin() opens Serial at 57600 baud and prints INITIALIZED#
|
||||||
st4Serial.begin(controller, Serial);
|
st4Serial.begin(controller, Serial);
|
||||||
controller.connect();
|
controller.connect();
|
||||||
|
|
||||||
|
|||||||
@ -13,6 +13,7 @@ class ST4Serial {
|
|||||||
HardwareSerial* serial_;
|
HardwareSerial* serial_;
|
||||||
String buffer_;
|
String buffer_;
|
||||||
bool extendedMode_;
|
bool extendedMode_;
|
||||||
|
bool bufferOverflow_;
|
||||||
|
|
||||||
void processCommand(const String& cmd);
|
void processCommand(const String& cmd);
|
||||||
void processExtendedCommand(const String& cmd);
|
void processExtendedCommand(const String& cmd);
|
||||||
|
|||||||
@ -90,6 +90,7 @@ bool ST4Controller::isConnected() const {
|
|||||||
|
|
||||||
void ST4Controller::move(ST4AxisId axis, ST4Direction dir) {
|
void ST4Controller::move(ST4AxisId axis, ST4Direction dir) {
|
||||||
if (!connected_) return;
|
if (!connected_) return;
|
||||||
|
configASSERT(static_cast<int>(axis) < 2);
|
||||||
|
|
||||||
xSemaphoreTake(mutex_, portMAX_DELAY);
|
xSemaphoreTake(mutex_, portMAX_DELAY);
|
||||||
int idx = static_cast<int>(axis);
|
int idx = static_cast<int>(axis);
|
||||||
@ -108,6 +109,8 @@ void ST4Controller::stopAxis(ST4AxisId axis) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
void ST4Controller::stopAll() {
|
void ST4Controller::stopAll() {
|
||||||
|
// Cancel pulse first -- consistent lock hierarchy (Pulse before Axis)
|
||||||
|
pulse_.cancel();
|
||||||
xSemaphoreTake(mutex_, portMAX_DELAY);
|
xSemaphoreTake(mutex_, portMAX_DELAY);
|
||||||
int raIdx = static_cast<int>(ST4AxisId::RA);
|
int raIdx = static_cast<int>(ST4AxisId::RA);
|
||||||
int decIdx = static_cast<int>(ST4AxisId::DECLINATION);
|
int decIdx = static_cast<int>(ST4AxisId::DECLINATION);
|
||||||
@ -116,11 +119,11 @@ void ST4Controller::stopAll() {
|
|||||||
axes_[decIdx].stop();
|
axes_[decIdx].stop();
|
||||||
trackers_[decIdx].stop();
|
trackers_[decIdx].stop();
|
||||||
xSemaphoreGive(mutex_);
|
xSemaphoreGive(mutex_);
|
||||||
if (pulse_.isActive()) pulse_.cancel();
|
|
||||||
}
|
}
|
||||||
|
|
||||||
bool ST4Controller::pulseGuide(ST4AxisId axis, ST4Direction dir, uint32_t ms) {
|
bool ST4Controller::pulseGuide(ST4AxisId axis, ST4Direction dir, uint32_t ms) {
|
||||||
if (!connected_) return false;
|
if (!connected_) return false;
|
||||||
|
configASSERT(static_cast<int>(axis) < 2);
|
||||||
|
|
||||||
xSemaphoreTake(mutex_, portMAX_DELAY);
|
xSemaphoreTake(mutex_, portMAX_DELAY);
|
||||||
int idx = static_cast<int>(axis);
|
int idx = static_cast<int>(axis);
|
||||||
|
|||||||
@ -66,6 +66,7 @@ void ST4Pulse::pulseTaskFunc(void* arg) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
void ST4Pulse::begin() {
|
void ST4Pulse::begin() {
|
||||||
|
configASSERT(instance_ == nullptr);
|
||||||
instance_ = this;
|
instance_ = this;
|
||||||
|
|
||||||
mutex_ = xSemaphoreCreateMutex();
|
mutex_ = xSemaphoreCreateMutex();
|
||||||
@ -122,7 +123,17 @@ bool ST4Pulse::pulse(ST4Axis& axis, ST4Tracker& tracker,
|
|||||||
tracker.start(slewRate);
|
tracker.start(slewRate);
|
||||||
|
|
||||||
// Start one-shot timer (microseconds)
|
// Start one-shot timer (microseconds)
|
||||||
esp_timer_start_once(timer_, static_cast<uint64_t>(ms) * 1000);
|
esp_err_t timerErr = esp_timer_start_once(timer_, static_cast<uint64_t>(ms) * 1000);
|
||||||
|
if (timerErr != ESP_OK) {
|
||||||
|
// Timer failed -- rollback hardware activation
|
||||||
|
axis.stop();
|
||||||
|
tracker.stop();
|
||||||
|
active_ = false;
|
||||||
|
activeAxis_ = nullptr;
|
||||||
|
activeTracker_ = nullptr;
|
||||||
|
xSemaphoreGive(mutex_);
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
|
||||||
xSemaphoreGive(mutex_);
|
xSemaphoreGive(mutex_);
|
||||||
return true;
|
return true;
|
||||||
|
|||||||
@ -5,7 +5,8 @@
|
|||||||
#include "ST4Serial.h"
|
#include "ST4Serial.h"
|
||||||
|
|
||||||
ST4Serial::ST4Serial()
|
ST4Serial::ST4Serial()
|
||||||
: controller_(nullptr), serial_(nullptr), extendedMode_(true) {}
|
: controller_(nullptr), serial_(nullptr), extendedMode_(true),
|
||||||
|
bufferOverflow_(false) {}
|
||||||
|
|
||||||
void ST4Serial::begin(ST4Controller& controller, HardwareSerial& serial,
|
void ST4Serial::begin(ST4Controller& controller, HardwareSerial& serial,
|
||||||
bool extendedMode) {
|
bool extendedMode) {
|
||||||
@ -105,10 +106,22 @@ void ST4Serial::processExtendedCommand(const String& cmd) {
|
|||||||
String rest = cmd.substring(5);
|
String rest = cmd.substring(5);
|
||||||
int spaceIdx = rest.indexOf(' ');
|
int spaceIdx = rest.indexOf(' ');
|
||||||
if (spaceIdx > 0) {
|
if (spaceIdx > 0) {
|
||||||
double ra = rest.substring(0, spaceIdx).toDouble();
|
String raStr = rest.substring(0, spaceIdx);
|
||||||
double dec = rest.substring(spaceIdx + 1).toDouble();
|
String decStr = rest.substring(spaceIdx + 1);
|
||||||
controller_->sync(ra, dec);
|
decStr.trim();
|
||||||
serial_->println("OK#");
|
// Validate: must start with digit, sign, or dot (reject garbage → 0.0)
|
||||||
|
bool raValid = raStr.length() > 0 &&
|
||||||
|
(isDigit(raStr[0]) || raStr[0] == '-' || raStr[0] == '+' || raStr[0] == '.');
|
||||||
|
bool decValid = decStr.length() > 0 &&
|
||||||
|
(isDigit(decStr[0]) || decStr[0] == '-' || decStr[0] == '+' || decStr[0] == '.');
|
||||||
|
if (raValid && decValid) {
|
||||||
|
double ra = raStr.toDouble();
|
||||||
|
double dec = decStr.toDouble();
|
||||||
|
controller_->sync(ra, dec);
|
||||||
|
serial_->println("OK#");
|
||||||
|
} else {
|
||||||
|
serial_->println("ERR:INVALID_COORDS#");
|
||||||
|
}
|
||||||
} else {
|
} else {
|
||||||
serial_->println("ERR:INVALID_COORDS#");
|
serial_->println("ERR:INVALID_COORDS#");
|
||||||
}
|
}
|
||||||
@ -137,14 +150,21 @@ void ST4Serial::update() {
|
|||||||
while (serial_->available()) {
|
while (serial_->available()) {
|
||||||
char c = serial_->read();
|
char c = serial_->read();
|
||||||
if (c == '#') {
|
if (c == '#') {
|
||||||
buffer_.trim();
|
if (bufferOverflow_) {
|
||||||
if (buffer_.length() > 0) {
|
// Discard the truncated command entirely
|
||||||
processCommand(buffer_);
|
bufferOverflow_ = false;
|
||||||
|
} else {
|
||||||
|
buffer_.trim();
|
||||||
|
if (buffer_.length() > 0) {
|
||||||
|
processCommand(buffer_);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
buffer_ = "";
|
buffer_ = "";
|
||||||
} else if (c != '\r' && c != '\n') {
|
} else if (c != '\r' && c != '\n') {
|
||||||
if (buffer_.length() < 64) {
|
if (buffer_.length() < 64) {
|
||||||
buffer_ += c;
|
buffer_ += c;
|
||||||
|
} else {
|
||||||
|
bufferOverflow_ = true;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@ -76,6 +76,10 @@ void ST4WiFi::processCommand(AsyncWebSocketClient* client,
|
|||||||
if (!cmd) return;
|
if (!cmd) return;
|
||||||
|
|
||||||
if (strcmp(cmd, "move") == 0) {
|
if (strcmp(cmd, "move") == 0) {
|
||||||
|
if (!controller_->isConnected()) {
|
||||||
|
client->text("{\"error\":\"not connected\"}");
|
||||||
|
return;
|
||||||
|
}
|
||||||
const char* axisStr = doc["axis"];
|
const char* axisStr = doc["axis"];
|
||||||
const char* dirStr = doc["dir"];
|
const char* dirStr = doc["dir"];
|
||||||
if (!axisStr || !dirStr) return;
|
if (!axisStr || !dirStr) return;
|
||||||
@ -94,6 +98,10 @@ void ST4WiFi::processCommand(AsyncWebSocketClient* client,
|
|||||||
broadcastState();
|
broadcastState();
|
||||||
|
|
||||||
} else if (strcmp(cmd, "pulse") == 0) {
|
} else if (strcmp(cmd, "pulse") == 0) {
|
||||||
|
if (!controller_->isConnected()) {
|
||||||
|
client->text("{\"error\":\"not connected\"}");
|
||||||
|
return;
|
||||||
|
}
|
||||||
const char* axisStr = doc["axis"];
|
const char* axisStr = doc["axis"];
|
||||||
const char* dirStr = doc["dir"];
|
const char* dirStr = doc["dir"];
|
||||||
uint32_t ms = doc["ms"] | 0;
|
uint32_t ms = doc["ms"] | 0;
|
||||||
@ -128,6 +136,7 @@ void ST4WiFi::processCommand(AsyncWebSocketClient* client,
|
|||||||
}
|
}
|
||||||
|
|
||||||
void ST4WiFi::broadcastState() {
|
void ST4WiFi::broadcastState() {
|
||||||
|
if (!ws_) return;
|
||||||
if (ws_->count() > 0) {
|
if (ws_->count() > 0) {
|
||||||
ws_->textAll(stateJson());
|
ws_->textAll(stateJson());
|
||||||
}
|
}
|
||||||
@ -159,6 +168,7 @@ String ST4WiFi::stateJson() const {
|
|||||||
}
|
}
|
||||||
|
|
||||||
void ST4WiFi::update() {
|
void ST4WiFi::update() {
|
||||||
|
if (!ws_) return;
|
||||||
ws_->cleanupClients();
|
ws_->cleanupClients();
|
||||||
|
|
||||||
ST4Direction currentRa = controller_->axisDirection(ST4AxisId::RA);
|
ST4Direction currentRa = controller_->axisDirection(ST4AxisId::RA);
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user