Skip to content

Commit de50bbe

Browse files
committed
FIX: Crashes when writing to flash
The TOF-sensor isp handler contained code that was executed from flash (not tagged with `IRAM_ATTR`) which could lead to crashes, especially when dealing with the configuration file also stored on the flash. Typical Guru Meditation messages had included: "Guru Meditation Error: Core 1 panic'ed (Cache disabled but cached memory region accessed)" "Core 1 was running in ISR context"
1 parent 456f51a commit de50bbe

File tree

2 files changed

+45
-17
lines changed

2 files changed

+45
-17
lines changed

src/sensor.cpp

Lines changed: 45 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,10 @@ static const uint32_t MEASUREMENT_IN_PROGRESS = 0;
9898
* close to the needed 148ms
9999
*/
100100

101+
static int isrPort[2];
102+
static volatile uint32_t * isrStart[2];
103+
static volatile uint32_t * isrEnd[2];
104+
101105
void HCSR04SensorManager::registerSensor(HCSR04SensorInfo sensorInfo) {
102106
m_sensors.push_back(sensorInfo);
103107
pinMode(sensorInfo.triggerPin, OUTPUT);
@@ -107,12 +111,51 @@ void HCSR04SensorManager::registerSensor(HCSR04SensorInfo sensorInfo) {
107111
if (m_sensors[m_sensors.size() - 1].median == nullptr) {
108112
m_sensors[m_sensors.size() - 1].median = new Median<uint16_t>(5, MAX_SENSOR_VALUE);
109113
}
114+
115+
// Make sure pointers are current todo: replace m_sensors vector with array.
116+
for (size_t idx = 0; idx < m_sensors.size(); ++idx) {
117+
HCSR04SensorInfo* const sensor = &m_sensors[idx];
118+
isrPort[idx] = sensor->echoPin;
119+
isrStart[idx] = &sensor->start;
120+
isrEnd[idx] = &sensor->end;
121+
}
110122
attachSensorInterrupt(sensorInfo);
111123
}
112124

125+
static void IRAM_ATTR isr(int pin, volatile uint32_t *start, volatile uint32_t *end) {
126+
// since the measurement of start and stop use the same interrupt
127+
// mechanism we should see a similar delay.
128+
if (*end == MEASUREMENT_IN_PROGRESS) {
129+
const uint32_t now = micros();
130+
if (HIGH == digitalRead(pin)) {
131+
*start = now;
132+
} else { // LOW
133+
*end = now;
134+
}
135+
}
136+
}
137+
138+
static void IRAM_ATTR isr0() {
139+
isr(isrPort[0], isrStart[0], isrEnd[0]);
140+
}
141+
142+
static void IRAM_ATTR isr1() {
143+
isr(isrPort[1], isrStart[1], isrEnd[1]);
144+
}
145+
113146
void HCSR04SensorManager::attachSensorInterrupt(HCSR04SensorInfo &sensorInfo) {
114-
// only one interrupt per pin, can not split RISING/FALLING here
115-
attachInterrupt(sensorInfo.echoPin, std::bind(&HCSR04SensorManager::isr, this, m_sensors.size() - 1), CHANGE);
147+
// bad bad bad ....
148+
if (sensorInfo.echoPin == isrPort[0]) {
149+
attachInterrupt(sensorInfo.echoPin, isr0, CHANGE);
150+
} else {
151+
attachInterrupt(sensorInfo.echoPin, isr1, CHANGE);
152+
}
153+
154+
// a solution like below leads to crashes with:
155+
// Guru Meditation Error: Core 1 panic'ed (Cache disabled but cached memory region accessed)
156+
// Core 1 was running in ISR context:
157+
// attachInterrupt(sensorInfo.echoPin,
158+
// std::bind(&isr, sensorInfo.echoPin, &sensorInfo.start, &sensorInfo.end), CHANGE);
116159
}
117160

118161
void HCSR04SensorManager::detachInterrupts() {
@@ -489,17 +532,3 @@ uint16_t HCSR04SensorManager::median(uint16_t a, uint16_t b, uint16_t c) {
489532
}
490533
return c;
491534
}
492-
493-
void IRAM_ATTR HCSR04SensorManager::isr(int idx) {
494-
// since the measurement of start and stop use the same interrupt
495-
// mechanism we should see a similar delay.
496-
HCSR04SensorInfo* const sensor = &m_sensors[idx];
497-
if (sensor->end == MEASUREMENT_IN_PROGRESS) {
498-
const uint32_t now = micros();
499-
if (HIGH == digitalRead(sensor->echoPin)) {
500-
sensor->start = now;
501-
} else { // LOW
502-
sensor->end = now;
503-
}
504-
}
505-
}

src/sensor.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,6 @@ class HCSR04SensorManager {
122122
void collectSensorResults();
123123
void sendTriggerToReadySensor();
124124
void attachSensorInterrupt(HCSR04SensorInfo &sensorInfo);
125-
void IRAM_ATTR isr(int idx);
126125
uint32_t getFixedStart(size_t idx, const HCSR04SensorInfo *sensor);
127126
static uint16_t medianMeasure(HCSR04SensorInfo* const sensor, uint16_t value);
128127
static uint16_t median(uint16_t a, uint16_t b, uint16_t c);

0 commit comments

Comments
 (0)