Missing pulses in anemometer code

I built an anemometer and wrote the obvious (hence probably not optimal) code. After getting it installed, it's pretty obviously missing pulses from the sensor. I've done a code audit and some poking around, but figured I'd ask if there's anything else I ought to be using before I started rewiring things and reworking the code.

The sensor generates a 1 hz signal for winds at 2.5mph, and is known to work in wind speeds up to 125mph, so we're dealing with a maximum 50 hz signal. Should be easy, right?

I just latched the pulse to an interrupt which increments a counter.

A second timer-driven interrupt grabs the count and resets it, converts from pulses to mph (so it will now fit in a uint8_t), and pushes it onto a circular queue: store, increment counter, wrap counter if it needs to.

The main loop scans the queue, calculating averages, maximum and trend information. It pushes this data out a serial line driving an LCD display, using softserial.

Ok, I've identified two things that should help:

  1. Use the hardware frequency counters.
  2. Switch to the standard serial line (this is on a pro mini)

And now 3) Save raw counts instead of speeds, converting just before display.

I suspect the problem is that the main loop is spending significant time in interrupt code in the SoftSerial library, so either #1 or #2 should solve the problem, but fixing both will result in the most accurate output. #3 will help by moving calculations outside the interrupt handling code.

Does this analysis seem to be on the right track? Is there something else I overlooked, or might have overlooked if you could only see the code? Etc?

Thanks,
<mike

Second post in the Programming Questions section...

Read this before posting a programming question ... - Programming Questions - Arduino Forum
Read this before posting a programming question ...
6. Getting help on the forum
Post your complete sketch (program code)! If you don't you waste time while people ask you to do that.

Please use
</mark> <mark>[code]</mark> <mark>

</mark> <mark>[/code]</mark> <mark>
tags.

Counting 50 Hz should be trivial. Better post your code, not waffle on about it.

Who knows what your code is but one pulse counter is much the same as another, so you might find this basic code useful.

It works very well but I am in the process writing it to collect averages, as the rate display is too erratic.

/*    DON'T FIDDLE WITH THIS virginal Coding Badly
 * Water Flow Gauge
 *
 * Uses a hall-effect flow sensor to measure the rate of water flow and
 * output it via the serial connection once per second.
 * 
 */

const byte sensorInterrupt = 0;  // 0 = pin 2; 1 = pin 3
const byte sensorPin       = 2; //

const float calibrationFactor = 6.7;

const unsigned long UPDATE_INTERVAL = 1000UL;

volatile byte pulseCount;  
static byte OldPulseCount;
static byte CurrentPulseCount;

static float flowRate;
static unsigned int flowMilliLitres;
static unsigned long totalMilliLitres;
static unsigned long oldTime;

void setup()
{
  
  pinMode(sensorPin, INPUT);
  digitalWrite(sensorPin, HIGH);

  pulseCount          = 0;
  flowRate            = 0.0;
  flowMilliLitres          = 0;
  totalMilliLitres         = 0;
  oldTime             = 0;


  attachInterrupt(sensorInterrupt, pulseCounter, FALLING);
}

void loop()
{ 
  unsigned long CurrentTime;

  CurrentTime = millis();

  if((CurrentTime - oldTime) >= UPDATE_INTERVAL)
  { 
    CurrentPulseCount = pulseCount;
    flowRate = ((1000.0 / (CurrentTime - oldTime)) * (byte)(CurrentPulseCount - OldPulseCount)) / calibrationFactor;
    flowMilliLitres = (flowRate / 60) * 1000;
    totalMilliLitres += flowMilliLitres;

    OldPulseCount = CurrentPulseCount;
    oldTime += UPDATE_INTERVAL;
  }
}

void pulseCounter()
{
  pulseCount++;
}

Edit: I posted the wrong example. The previous works but this is what I am, actually using.

Well, I was looking for advice on a project, not programming (hence this is in the project guidance section, not the programming section). Any chance someone can answer the questions about the interrupt behavior of SoftSerial vs. Serial and using the hardware counters instead of a software interrupt?

But...

#include <TimerOne.h>
#include <SoftwareSerial.h>
#include <string.h>

// Values that control display
#define BLUE_SPEED	8	// Minimum speed for blue display
#define RED_SPEED	21	// Minimum speed for red dispaly

/*
 * Values that control recording. Note that DELAY_SECONDS and
 * DELAY_SECONDS * TREND_BUCKETS must both go evenly into RECORD_TIME.
 */
#define RECORD_TIME	300	// Length of the recording
#define DELAY_SECONDS	1	// Sample time length. Max of 8.
#define TREND_BUCKETS	3	// # of buckets to use for trend checking.

// Physical constants
#define SENSOR_INT	1	// Meaning pin #3
#define SERIAL_OUT	2	// Serial line to the display
#define LINE_LENGTH	16	// Length of output line on LCD

// Display (TREND_* maps to first custom char for that trend).
#define TREND_UNKNOWN	0
#define TREND_DOWN	2
#define TREND_EVEN	4
#define TREND_UP	6

// 1 PULSE/SEC = 2.5 MPH
#define PULSES_TO_SPEED(count) (((count) * 5) / (DELAY_SECONDS * 2))

// The wind sensor interrupt data
volatile static uint16_t sensor_count = 0 ;

// And handler
static void
sensor_tick() {
  sensor_count += 1 ;
}

// The recording instrument data
#define SPEED_LENGTH (RECORD_TIME / DELAY_SECONDS)

// Length of trend buckets in the record
#define BUCKET_LENGTH	(SPEED_LENGTH / TREND_BUCKETS)

volatile static uint8_t current_speed ;
volatile static uint8_t speeds[SPEED_LENGTH] ;
volatile static uint16_t next_speed = 0 ;
volatile static uint8_t wrapped = 0 ;

void
recorder_tick() {
  uint16_t raw ;

  raw = sensor_count ;	// Warning - could lose a tick here...
  sensor_count = 0 ;
  current_speed = PULSES_TO_SPEED(raw) ;
  speeds[next_speed++] = current_speed ;
  if (next_speed >= SPEED_LENGTH) {
    wrapped = 1 ;
    next_speed = 0 ;
  }
}

// Select a color with a very clever plan
static uint8_t
color_of(uint8_t speed) {
  if (speed >= RED_SPEED) return 0 ;
  if (speed >= BLUE_SPEED) return 2 ;
  return 1 ;
}

SoftwareSerial lcd = SoftwareSerial(0, SERIAL_OUT) ;

void
do_command(SoftwareSerial lcd, uint8_t len, const char *data) {
  lcd.write(0xFE) ;
  while (len--)
    lcd.write(*data++) ;
  delay(10) ;
}


void
setup() {
#ifdef MAKE_CHARS
  char chars[][11] = {
    {0xC1, 1, 0, B01001, B00101, B00011, B11111, B11111, B00011, B00101, B01001},
    {0xC1, 1, 1, B10010, B10100, B11000, B11111, B11111, B11000, B10100, B10010},
    {0xC1, 1, 2, 4, 2, 1, 0, 0, 0, 0, 0},
    {0xC1, 1, 3, 0, 0, 0, B10001, B01001, B00101, B00011, B11111},
    {0xC1, 1, 4, 0, 0, 0, B11111, B11111, 0, 0, 0},
    {0xC1, 1, 5, B01000, B00100, B00010, B11111, B11111, B00010, B00100, B01000},
    {0xC1, 1, 6, 0, 0, 0, 0, 0, 1, 2, 4},
    {0xC1, 1, 7, B11111, B00011, B00101, B01001, B10001, 0, 0, 0},
    } ;
#endif
  
  pinMode(3, INPUT_PULLUP) ;
  attachInterrupt(SENSOR_INT, sensor_tick, RISING) ;
  Timer1.initialize(1000000 * DELAY_SECONDS) ;
  Timer1.attachInterrupt(recorder_tick) ;

  lcd.begin(57600) ;
  delay(10) ;

#ifdef MAKE_CHARS
  do_command(lcd, 33, "\x40 Meyer Heliport Wind Conditions ") ;
  for (uint8_t i = 0; i < 8; i += 1)
    do_command(lcd, 11, chars[i]) ;
#endif
  do_command(lcd, 1, "\x4B") ;		// Turn off underline cursor
  do_command(lcd, 1, "\x54") ;		// blinking cursor
  do_command(lcd, 1, "\x52") ;		// and autoscroll
  do_command(lcd, 2, "\xC0\01") ;	// Get arrows
}

void
loop() {
  int8_t trend ;
  int16_t end, start ;
  uint16_t i, now, cur, avg, max, bucket_counter, bucket_count ;
  uint32_t sum, bucket_sum ;
  int32_t buckets[TREND_BUCKETS] ;
  char bhold[LINE_LENGTH + 1], colors[4] = {0xD0} ;

  end = next_speed ;
  start = wrapped ? end : 0 ;

  bucket_count = 0 ;
  trend = TREND_UNKNOWN ;
  if (!wrapped && end == 0) {	// Haven't gotten a sample yet.
    avg = max = cur = current_speed ;
  } else {			// Do some stats...
    cur = current_speed ;
    i = start ;
    bucket_sum = bucket_counter = max = sum = 0 ;
    do {
      now = speeds[i] ;
      sum += now ;
      if (now > max)
	max = now ;
      bucket_sum += now ;
      bucket_counter += 1 ;
      if (bucket_counter == BUCKET_LENGTH) {
	buckets[bucket_count++] = bucket_sum ;
	bucket_counter = 0 ;
	bucket_sum = 0 ;
      }
      i += 1 ;
      if (i == SPEED_LENGTH)
	i = 0 ;
    } while (i != end) ;
    avg = sum / (wrapped ? SPEED_LENGTH : end) ;

    if (bucket_count == TREND_BUCKETS) {
      for (i = 0; i < TREND_BUCKETS - 1; i += 1) {
	if (buckets[i] == buckets[i + 1]) {
	  trend = TREND_EVEN ;
	  break ;
	} else if (buckets[i] < buckets[i + 1]) {
	  if (trend == TREND_UNKNOWN)
	    trend = TREND_UP ;
	  else if (trend == TREND_DOWN) {
	    trend = TREND_UNKNOWN ;
	    break ;
	  }
	} else if (buckets[i] > buckets[i + 1]) {
	  if (trend == TREND_UNKNOWN)
	    trend = TREND_DOWN ;
	  else if (trend == TREND_UP) {
	    trend = TREND_UNKNOWN ;
	    break ;
	  } else if (trend == TREND_UNKNOWN) {
	    trend = TREND_EVEN ;
	  }
	}
      }
    }
  }

  // Now, display the data
  do_command(lcd, 1, "\x58") ;	// clear screen
  do_command(lcd, 1, "\x48") ;	// go home

  // Pick a color...
  memset(colors + 1, 0, 3) ;
  colors[color_of(avg) + 1] = 255 ;
  colors[color_of(max) + 1] = 255 ;
  do_command(lcd, 4, colors) ;

  lcd.println("Cur Avg Max Tnd") ;
  snprintf(bhold, LINE_LENGTH + 1, "%3d %3d %3d  ", cur, avg, max) ;
  lcd.print(bhold) ;
  lcd.write(trend) ;
  lcd.write(trend + 1) ;
  delay(1000) ;
}
do_command(SoftwareSerial lcd, uint8_t len, const char *data)

...makes a copy of lcd each time it's called. Make the first parameter pass-by-reference...

do_command(SoftwareSerial & lcd, uint8_t len, const char *data)

Even better would be to eliminate the first parameter. It is not serving any purpose.

mwmeyer:
Ok, I've identified two things that should help:

You need to identify the problem before tossing out solutions.

mwmeyer:
I suspect the problem is that the main loop is spending significant time in interrupt code in the SoftSerial library

I suspect you are wrong. SoftwareSerial only disables interrupts while transmitting each byte. At 57600 baud that's 174 microseconds.

There's nothing obviously wrong with the approach you outlined, although personally I wouldn't have used an interrupt for the timed processing (since it is very low frequency and not time-critical) and I probably wouldn't have bothered with interrupts for capturing the input signal either since it is extremely low frequency. Interrupts introduce considerable design complexity and should IMO be used only where necessary - and they aren't necessary here.

However, the devil's in the detail and without seeing your code it's impossible to guess whether you've made any gotchas.

Have you tested the pulse detection/counting code, and if so how?

What is the evidence that brings you to conclude that you are missing pulses?

What are the characteristics of the signal you are trying to capture - is it a TTL square wave at the same nominal voltage as the Arduino?

How is the input connected?

It may be project guidance but basically it is a programming question.

In loop you call do_command three times. Each one introduces a 10 mS delay. So you can basically do that 33 times a second before you spend all your time in delays. And you said you wanted to count 50 Hz.

I don't see the point of the timer interrupt. Every second or so (which you can determine by calling millis() ) you can get the count, and divide by the exact interval (current time - previous time) to find a rate.

Oh wait, I missed this:

  delay(1000) ;

When you are missing pulses, I would get rid of delays.

I have a sketch here that counts up to 5 kHz:

There's another one on that page that counts up to 100 kHz.

Does delay() block interrupts? Because it clearly means that I'm not losing pulses to being in SerialSoftware.

No it doesn't, so perhaps I was wrong about that. However SoftwareSerial does block interrupts.

So your suggestion of using hardware serial sounds sensible.