I’m publishing distance data (from a HC-SR04 ultrasonic sensor) every 5 minutes which sometimes works, and other times publishes when the value is out of range, which should be prevented by an if clause, but it isn’t. I’m not getting a random value, but one that is either 172091 or 172092 mm (the correct range should be around 320 mm). I thought maybe this was a problem with IFTTT or with Google sheets, but I see the same incorrect values on the Particle Dashboard. Here is the code I’m using:
int trigPin = D0;
int echoRisingPin = D2;
int echoFallingPin = D4;
unsigned long startTime;
unsigned long endTime;
unsigned long distance;
String waterLevel;
bool publishValue; // if true, pusblish the event, "tankMonitor", to IFTTT which adds the waterLevel value to a Google sheet
bool initiatePulse; // if true, causes an ultrasonic pulse to be sent out by the sensor (HC-SR04)
void setup() {
publishValue = false;
initiatePulse = true;
pinMode(trigPin, OUTPUT);
digitalWrite(trigPin, LOW);
attachInterrupt(echoRisingPin, risingDetect, RISING);
attachInterrupt(echoFallingPin, fallingDetect, FALLING);
}
void loop() {
if (publishValue) {
Particle.publish("tankMonitor", waterLevel);
delay(1000);
System.sleep(SLEEP_MODE_DEEP,300);
}
if (initiatePulse) {
initiatePulse = false; // set to false so another trigger pulse is not sent on the next turn of the loop
triggerSensor(); // trigger the sensor to send out an ultrasonic pulse
}
}
void risingDetect() {
startTime = micros();
}
void fallingDetect() {
endTime = micros();
distance = (endTime - startTime)/5.82; // distance in millimeters;
waterLevel = String::format("%lu mm", distance);
if (distance > 310 && distance < 400) {
publishValue = true;
}else{
initiatePulse = true;
}
}
void triggerSensor() {
delay(1000);
digitalWrite(trigPin, HIGH);
delayMicroseconds(15);
digitalWrite(trigPin, LOW);
}
The data I’m publishing is the String waterLevel, but my if clause tests the unsigned long, distance. Could there be something wrong with the conversion from the unsigned long to the string, or am I just missing something obviously wrong in my code?
I wonder if you are trying to accomplish too much in this ISR… Have you tried moving the waterlevel = String::format("%lu mm", distance); function into loop?
Also, any variable that you manipulate in an ISR ought to be declared volatile :
...
volatile unsigned long startTime;
volatile unsigned long endTime;
volatile unsigned long distance;
...
finally, using String class can cause issues… if you can use null terminated array of chars (C string) you may find it more stable over the long term.
What do you mean by “using String class can cause issues”? Is the String class usable or not? Does it have known issues that are being worked on? I’ll try implementing your other suggestions and see if they fix the issue. As far as “doing too much in an ISR”, what is too much? I know you’re not supposed to have anything long running in one, but really, if this is too much, what can you do in one?
In addition to the issues pointed out above that should definitely be fixed, it looks like you’re never clearing the publishValue flag so as soon as one value is within the range, all future values are forever set to be published. You do this for initiatePulse, so it was probably just an oversight.
Have you already tried @BulldogLowell’s suggestions? I think the String class issue he is probably alluding to is that it covertly uses heap memory, which could lead to eventual heap fragmentation. Rather than using String::format, you can try snprintf’ing to a fixed-length buffer to sidestep such issues entirely.
I have a hunch that because you didn’t declare the flags you’re setting as retained variables, you can’t count on them being set to a non-arbitrary value upon waking up.
Edit: Nevermind, I see that those are being initialized in setup as well. Out of ideas for now
@indraastra, I haven’t tried sprintf’ing yet, but I’ve made the other changes. I declared the variables as volatile, and moved the String::format line into loop(). I’m still having the same problem, so I’ll get rid of the format function, and see if the fixes it. I want to take this a step at a time, so I can understand what could cause this problem.
Just to be on the safe side of things you might also want to add pinMode(xxx, INPUT) for your interrupt pins.
I’ve seen an open issue in the open sources about attachInterrupt() not setting the pinMode() implicitly (which I personally don’t see as an issue but rather desirable, to give the considered programmer the freedom of choice of the best suited mode ;-)).
I’d also add a sanity check as startTime != 0 and make sure that startTime gets reset after (or inside) fallingDetect().
Also move the String building out of the ISR into a place where timing is non-critical - as @BulldogLowell already suggested.
I’d also add a publishValue = false into the else block of your range check, since you might actually have another trigger to one of your ISRs before your loop() gets round to acting upon publishValue == true causing the subsequent distance to be published - for that reason @indraastra’s advice is valid and relying on the auto-reset via deep-sleep might not actually work in such a case.
Keeping track of asynchronous tasks is not always that easy
@BulldogLowell and @ScruffR, thanks for all the suggested changes. As far as I can tell, BulldogLowell’s code, with a minor addition, is working well. I had to add pulseComplete = true (and set startTime and endTime equal to 0 to be sure they had defined values the first time through loop()) to setup(), otherwise triggerSensor() is never called.
