I everyone. I' m a computer science student and I'm working for my bachelor thesis with particle Argon. I' m writing a program to collect data from an anemometer, but i have some concurrency issue with the ISR :
#include <ThingSpeak.h>
// client for the communication with thingspeak
TCPClient client;
unsigned long myChannelNumber = XXXXXXX; // change this to your channel number
const char * myWriteAPIKey = "XXXXXXXX"; // change this to your channel write API key
#define REQUEST_RATE 10000 // in milliseconds - Thingspeak update rate
unsigned long last_update; //last Thingspeak update in ms
unsigned long current_time; // current time in ms
// ===========================
// -- Anemometer section
// ===========================
unsigned long PulseTimeNow = 0; // Time stamp (in millisecons) for pulse triggering the interrupt
// Davis anemometer
// ================
const float WindTo_mps = 1.00584;
const float WindTo_kph = 3.62102;
const float WindTo_mph = 2.25;
const float WindTo_knt = 1.95515;
float WindSpeed_mps, WindSpeed_kph, WindSpeed_mph, WindSpeed_knt;
volatile unsigned long PulseTimeLast = 0; // Time stamp of the previous pulse
volatile unsigned long PulseTimeInterval = 0;; // Time interval since last pulse
float WindSpeed_mpsx;
volatile unsigned long PulsesCumulatedTime = 0; // Time Interval since last wind speed computation
volatile unsigned long PulsesNbr = 0; // Number of pulses since last wind speed computation
volatile unsigned long LastPulseTimeInterval = 1000000000;
volatile unsigned long MinPulseTimeInterval = 1000000000;
float MaxWind = 0.0; // Max wind speed
float WindGust = 0.0;
// Davis vane
unsigned int WindDirection = 0; // value from the ADC
float DirectionVolt = 0.0; // voltage read from the vane output
float VaneOffset = 0.0; // Enter here the offset for you vane to get a calibrated wind direction
// Misc.
// -----
int FirstLoop = 0;
// END -- Anemometer section
// **********************
// -- SETUP
// **********************
void setup() {
ThingSpeak.begin(client);
// Anemometer Interrupt setup
// Davis anemometer is assigned to interrupt 1 on digital pin D3
// when an interrupt on pin D3 occurs then run AnemometerPulse function
pinMode(D3, INPUT);
attachInterrupt(D3, AnemometerPulse, FALLING);
// Note: analogPin pin does not require pinMode()
PulseTimeLast = micros();
current_time = millis();
last_update = current_time;
}
// **********************
// -- MAIN LOOP
// **********************
void loop() {
current_time = millis();
if ( ( current_time-last_update ) >= REQUEST_RATE ){
// Get Anemometer data
AnemometerLoop();
if (FirstLoop <= 2) { return; } // Discard first sets of data to make sure you get clean data
// set fields one at a time
ThingSpeak.setField(1,WindSpeed_mps);
ThingSpeak.setField(2,WindSpeed_kph);
ThingSpeak.setField(3,WindSpeed_mph);
ThingSpeak.setField(4,WindSpeed_knt);
// Write the fields that you've set all at once.
ThingSpeak.writeFields(myChannelNumber, myWriteAPIKey);
last_update = current_time;
}
}
// **********************
// -- FUNCTIONS
// **********************
void AnemometerLoop () // START Anemometer section
{
// with micros()
WindSpeed_mpsx = 1000000*WindTo_mps/PulseTimeInterval; // calculated on last pulse periode (only 1 pulse)
WindSpeed_mps = (1000000*WindTo_mps/PulsesCumulatedTime)*PulsesNbr; // averaged every XX seconds (on Thingspeak update rate)
WindSpeed_kph = (1000000*WindTo_kph/PulsesCumulatedTime)*PulsesNbr;
WindSpeed_mph = (1000000*WindTo_mph/PulsesCumulatedTime)*PulsesNbr;
WindSpeed_knt = (1000000*WindTo_knt/PulsesCumulatedTime)*PulsesNbr;
MaxWind = 1000000*WindTo_mps/MinPulseTimeInterval; // Determine wind gust (i.e the smallest pulse interval between Pachube updates)t pulse interval between Pachube updates)
PulsesCumulatedTime = 0;
PulsesNbr = 0;
MinPulseTimeInterval = 1000000000;
LastPulseTimeInterval = 1000000000;
WindGust = MaxWind;
MaxWind = 0;
DirectionVolt = analogRead (A1); // Analog input 1 (A1) is Davis Vane #2
WindDirection = (DirectionVolt / 1024.0) * 360.0;
WindDirection = WindDirection + VaneOffset ; // add direction offset to calibrade vane position
if (WindDirection > 360 ) { WindDirection = WindDirection - 360; }
if (FirstLoop <= 2)
{
FirstLoop = FirstLoop + 1;
}
}
void AnemometerPulse()
{
noInterrupts(); // disable global interrupts
PulseTimeNow = micros(); // Micros() is more precise to compute pulse width that millis();
PulseTimeInterval = PulseTimeNow - PulseTimeLast;
PulseTimeLast = PulseTimeNow;
PulsesCumulatedTime = PulsesCumulatedTime + PulseTimeInterval;
PulsesNbr++;
if ( PulseTimeInterval < LastPulseTimeInterval ) // faster wind speed == shortest pulse interval
{
MinPulseTimeInterval = PulseTimeInterval;
LastPulseTimeInterval = MinPulseTimeInterval;
}
// deglitch algorith needs to added !!!!
interrupts(); // Re-enable Interrupts
}
As you can se I access shared resource in the ISR and loop function. Is there any possible race condition ? The ISR and loop function can be execute in a concurrent way or the situation is thread safe ? Should I disable the interrupts also in the loop function when I access the resource shared with the ISR??
Thanks in advance.
First, I would do the following while not disabling/enabling interrupts. Make the ISR do as little as possible, such as set a volatile boolean variable then let the loop() process the variable:
volatile bool pulseDetected = false;
void isrTriggered() {
pulseDetected = true;
}
void setup() {
...
attachInterrupt(D3, isrTriggered, FALLING);
...
}
void loop() {
if (pulseDetected) {
pulseDetected = false;
// process pulse here
}
//all the rest here
}
Thanks a lot for your suggestion. I'll consider write my code starting from this scratch. Now I have another question, more specific, since I need to keep a counter of the pulse detected :
Do I risk to have some sinchronization issues? I mean the counter is shared between the loop function and the ISR. Thanks in advance for your help!!
volatile int pulseCount;
void isrTriggered() {
pulseCount++;
}
void setup() {
...
attachInterrupt(D3, isrTriggered, FALLING);
pulseCount = 0;
...
}
void loop() {
...
// process pulse here
....
if( need to reset the counter) {
pulseCount = 0;
}
}
I would not implement it that way. You should not disable interrupts from the ISR. And the volatile keyword does not solve this issue either. This causes the compiler to not cache the result in a register, but does not prevent the situation where the ISR modifies the value while another thread is reading or writing it.
The method I use is std::atomic which does atomic operations on integer-like values without disabling interrupts, and is also thread and ISR safe. Can the P2 run two Interrupts concurrently?
Thank you @rickkas7 for your help. Putting together your contribution and the one by @robc I managed to write the following code that seems working. Tell me what do you think and thank you again!
#include <ThingSpeak.h>
SYSTEM_THREAD(ENABLED);
// ===========================
// -- ThingSpeak section
// ===========================
// for the communication with thingspeak
TCPClient client;
unsigned long myChannelNumber = XXXXXXX; // change this to your channel number
const char * myWriteAPIKey = "XXXXXXXXXX"; // change this to your channel write API key
#define REQUEST_RATE 60000 // in milliseconds - Thingspeak update rate
unsigned long last_update; //last Thingspeak update in ms
unsigned long current_time; // current time in ms
// END -- ThingSpeak section
// ===========================
// -- Anemometer section
// ===========================
// Davis anemometer
// ================
const float WindTo_kph = 3.62102;
const float WindTo_mph = 2.25;
float WindSpeed_kph, WindSpeed_mph, PulseFrequency_hz;
volatile std::atomic<uint32_t> PulsesNbr; // Number of pulses since last wind speed computation
// END -- Anemometer section
// **********************
// -- SETUP
// **********************
void setup() {
// communication with ThingSpeak setup
ThingSpeak.begin(client);
current_time = millis();
last_update = current_time;
pinMode(D3, INPUT);
PulsesNbr.store(0, std::memory_order_relaxed);
// Anemometer Interrupt setup
// Davis anemometer is assigned to interrupt 1 on digital pin D3
// when an interrupt on pin D3 occurs then run AnemometerPulse function
attachInterrupt(D3, AnemometerPulse, FALLING);
}
// **********************
// -- MAIN LOOP
// **********************
void loop() {
current_time = millis(); // update current time to millis()
if ( ( current_time-last_update ) >= REQUEST_RATE ){ // It's time to update thingspeak!
// Get Anemometer data
// REQUEST_RATE is expressed in ms --> must convert ms in s dividing by 1000
AnemometerLoop(REQUEST_RATE/1000);
// set fields one at a time
ThingSpeak.setField(1,WindSpeed_mph);
ThingSpeak.setField(2,WindSpeed_kph);
ThingSpeak.setField(3, PulseFrequency_hz);
// Write the fields that you've set all at once.
ThingSpeak.writeFields(myChannelNumber, myWriteAPIKey);
last_update = current_time;
}
}
// **********************
// -- FUNCTIONS
// **********************
//param SamplePeriod must be in second (s)
void AnemometerLoop (unsigned long SamplePeriod) // START Anemometer section
{
//atomically reset the pulse counter and fetch old value
uint32_t PulsesDetected = PulsesNbr.fetch_and(0, std::memory_order_relaxed);
// V = P(2.25/T), V = speed in mph, P = no. of pulses per sample period, T = sample period in seconds
// 2.25 mph equates to 3,62102 kph
WindSpeed_kph = (WindTo_kph/SamplePeriod)*PulsesDetected; // averaged every SamplePeriod seconds (on Thingspeak update rate)
WindSpeed_mph = (WindTo_mph/SamplePeriod)*PulsesDetected;
PulseFrequency_hz = (1.00/SamplePeriod)*PulsesDetected;
Particle.publish("Number of detected pulses:" + String(PulsesDetected));
}
void AnemometerPulse()
{
// This increments the value atomically. Even if the ISR triggers
// while we're resetting the value from loop, the count will
// not be lost.
PulsesNbr.fetch_add(1, std::memory_order_relaxed);
}
I thought I would correct or "make better" the pseudocode I wrote earlier:
// Atomic variables - values are set and get atomically for use with ISR
std::atomic<uint32_t> pulseCount;
void isrTriggered() {
// This increments the value atomically. Even if the ISR triggers
// while we're resetting the value from loop, the count will
// not be lost.
pulseCount.fetch_add(1, std::memory_order_relaxed);
}
void setup() {
...
pulseCount.store(0, std::memory_order_relaxed);
pinMode(D3, INPUT);
attachInterrupt(D3, isrTriggered, FALLING);
...
}
void loop() {
static unsigned long lastCheck = 0;
if (millis() - lastCheck >= 1000) {
lastCheck = millis();
// Take the value from and clear it to 0 atomically. Even if
// the ISR triggers at this exact moment, "pulseCount" will either
// be updated before reading (.fetch_and), or after setting to 0, but never
// in the middle.
uint32_t tempPulseCount = pulseCount.fetch_and(0, std::memory_order_relaxed);
// process pulse count here
...
}
// all the rest here
...
}