Measuring pulse period

I want to measure the wheel speed on my bicycle. I have:

  • bicycle
  • hall effect position sensor (honeywell 103SR5A-1)
  • arduino nano every

I will make a bracket to mount the sensor to the chain stay such that it will sense the steel spokes passing by. I have some rough code written so far:

#include <util/atomic.h>

// update interval
const byte update_interval = 100;
unsigned long update_time = 0;

// vehicle speed sensor
const byte vss_pin = 2;
volatile unsigned long vss_count_volatile = 0;
volatile unsigned long vss_period_volatile = 0;
unsigned long vss_time = 0;

void setup()
{
  pinMode(vss_pin, INPUT);
  attachInterrupt(digitalPinToInterrupt(vss_pin), vss_isr, RISING);
}

void loop()
{
  unsigned long time_millis = millis();
  if (time_millis - update_time > update_interval)
  {
    update_time += update_interval;
  }
  else
  {
    return;
  }
  
  unsigned long vss_count;
  unsigned long vss_period;
  ATOMIC_BLOCK(ATOMIC_RESTORESTATE)
  {
    vss_count = vss_count_volatile;
    vss_period = vss_period_volatile;
  }

  // todo: calculate distance from vss_count

  // todo: calculate speed from vss_period
}

void vss_isr()
{
  unsigned long time_micros = micros();
  vss_count_volatile++;
  vss_period_volatile = time_micros - vss_time;
  vss_time = time_micros;
}

My concern is that I could get a pulse while inside my atomic block, delaying the reading of micros(). I guess the effect will be very small, but I thought the mega could store a timer value in a register at the moment the interrupt is triggered. However, I haven't been able to find any reference to this feature in the atmega4809 datasheet.

Read this:

Sorry, but I don't think that function is useful in this application. Not only does it measure the duration of a pulse instead of the time between two pulses, it will also not allow me to do anything else while waiting on a pulse.

1 Like

Why? Normally an atomic block is used to poll the interrupt. If it's from a hall rotation sensor, the next pulse is virtually an eternity in the future.

Also, you captured the exact micros() time inside the ISR, how can it change outside it?

Because it could delay the execution of the isr and therefore my call to micros() which will have an effect on the measurement. If the mega could store a timestamp in a register on the same cycle that the interrupt is triggered, this would achieve better accuracy.

Not a chance. It will delay the execution of the ISR, but your next call that an external interrupt from the sensor, is way way way in the future.

The measurement you speak of, occurs inside the ISR. The polling detects the ISR and should be able to copy the measurement a long, long, long, long time before it is called again.

Also, how far does your bicycle wheel travel during the execution of the critical section? :slight_smile:

I'm not sure where you're going with this question. If you're asking why I'm using the atomic blocks, it's because reading a 32-bit variable is a non-atomic operation.

I'm not sure we're on the same page. I'm not worried about the next pulse getting skipped because the wheel is spinning too fast because, although i haven't done the math, I don't think I can go that fast. My concern is that the time between the sensor sending a pulse and my call to micros() inside the isr can be delayed if my loop happens to be inside the atomic block.

Okay, I get it now. Yes there is a few microseconds latency. But as I suggested, for a bike speedometer the error would be miniscule, insignificant. The thing you think you need is provided by a different MCU function, Input Capture, which operates independently from software to freeze the timer value when an input is toggled.

But I'm willing to bet, your application doesn't need that degree of accuracy.

Actually, though, there is absolutely no risk of "skipping" any pulse... you have that one wrong. Unless you have a horrendously slow loop() execution time...

That's ten updates per second, but a scratch on the back of the envelope says the bike wheel only turns 3 times per second, that's a pedal bike at a very good clip.

Seems like your algorithm would need a much bigger window to accumulate data for your calculation.

I would look into edge to edge measurement and some kind of averaging if recent readings.

At 3 Hz there's plenty of time to handle what each interrupt discovers.

a7

Input Capture is exactly what I was thinking of. My tire has a circumference of 2161mm and there are 9 triggers on the wheel. That's a pulse about every 9.5 in. If I'm going some crazy speed like 30 mph, I would get a pulse every 17.9 ms. I don't know how long I'm spending inside my atomic block, but I would imagine it's somewhere in the ballpark of 1 us. So I guess in this application, the error is insignificant. Although I may implement the input capture logic just for fun.

a7, update_interval is for controlling the rate which I read the volatile variables, do some calculations, and update the display. It has nothing to do with the actual measurement of the time between pulses.

Thank you, yes I see.

Looking forward to the to- do part!

a7

Here's what I have so far for anyone interested:

// board: arduino nano every
// registers emulation: none

#include <util/atomic.h>
#include <SPI.h>
#include <Adafruit_GFX.h>
#include <Adafruit_PCD8544.h>

// magic number
// a: 945 inch hundredths per pulse
// b: 3600000000 microseconds in an hour
// c: 6336000 inch hundredths in a mile
const float magic_number_vd = 0.000149148; // a / c
const uint32_t magic_number_vs = 536931; // a * b / c

// vehicle speed sensor
#define VSS_PIN 2
volatile uint32_t vss_time_prev_volatile = 0;
volatile uint32_t vss_period_volatile = 0;
volatile uint32_t vss_count_volatile = 0;

// display
#define DISPLAY_DC_PIN 5 // data/command select
#define DISPLAY_CS_PIN 4 // chip select
#define DISPLAY_RST_PIN 3 // reset
Adafruit_PCD8544 display = Adafruit_PCD8544(DISPLAY_DC_PIN, DISPLAY_CS_PIN, DISPLAY_RST_PIN);

// update interval
const uint32_t update_interval = 100000; // microseconds
uint32_t update_time = -1;

void setup()
{
  // vss initialization
  pinMode(VSS_PIN, INPUT);
  attachInterrupt(digitalPinToInterrupt(VSS_PIN), vss_isr, RISING);

  // display initialization
  display.begin();
  display.clearDisplay();
  display.setContrast(75);
  display.setRotation(0);

  delay(2500);
  Serial.begin(9600);
  Serial.println(magic_number_vd, 10);
  Serial.println(magic_number_vs);
}

void loop()
{
  uint32_t time_us = micros();
  if (time_us - update_time >= update_interval)
  {
    update_time += update_interval;
  }
  else
  {
    return;
  }
  
  uint32_t vss_time_prev;
  uint32_t vss_period;
  uint32_t vss_count;
  ATOMIC_BLOCK(ATOMIC_RESTORESTATE)
  {
    vss_time_prev = vss_time_prev_volatile;
    vss_period = vss_period_volatile;
    vss_count = vss_count_volatile;
  }

  // vehicle distance
  float vehicle_distance = vss_count * magic_number_vd;
  display.setTextSize(2);
  display.setCursor(0,24);
  if (vehicle_distance < 10) display.print("000");
  if (vehicle_distance < 100) display.print("00");
  if (vehicle_distance < 1000) display.print("0");
  display.print(vehicle_distance, 2);

  // vehicle speed
  uint8_t vehicle_speed = 0;
  if (time_us - vss_time_prev < magic_number_vs)
  {
    vehicle_speed = magic_number_vs / vss_period;
  }
  display.setTextSize(3);
  display.setCursor(0,0);
  if (vehicle_speed == 0 || vehicle_speed > 99)
  {
    display.print("--");
  }
  else
  {
    if (vehicle_speed < 10) display.print("0");
    display.print(vehicle_speed);
  }

  // write to display
  display.display();
}

void vss_isr()
{
  uint32_t time_us = micros();
  vss_period_volatile = time_us - vss_time_prev_volatile;
  vss_time_prev_volatile = time_us;
  vss_count_volatile++;
}

I have yet to test anything as I'm waiting for hardware. I will be adding button logic for resetting distance and for controlling the headlight and taillight.

OK, nice.

I placed your code in the wokwi simulator. They don't have a Nano Every, but that should not matter.

I removed the LCD stuff. That also should not matter.

I routed the output of the Arduino tone() function to the sensor input pin to simulate the sensor switch closings.

So far I can't make sense of the outputs and their relation to the tone frequency.

Since

It is not possible to generate tones lower than 31Hz.

you'd have to use another technique to simulate the expected pulse rate from the wheel.

Or a real wheel IRL obvsly, or a signal generator.

But your loop() runs free! so it would be easy to also synthesize the very pulses you are measuring.

I'd start on that now, but the weather is glorious and I my ride to the beach wlll arrive soon.

Check out the wokwi here. You don't need an account, and you will have your copy to tinker with.

It is way easier than the upload/test/edit cycle.

Grrr. Nice day slipping away, have I been Syd Barretted?

Anyway, eliminate the call to tone() in setup() and you can use

// synthesize input pulses
# define PERIOD 300
uint32_t now = millis();
static unsigned long lastEdge;
if (now - lastEdge > (PERIOD / 2)) {
  lastEdge = now;
  digitalWrite(toneOut, !digitalRead(toneOut));
}

At the top of your loop() function to feed known pulses to your mechanism.

Have you figured out the frequency range of the pulses from the wheel sensor?

I just noticed you are looking at the steel spokes, so maybe tone() will work as a proxy for you.

How many spokes on your wheel?

a7

That's a very interesting tool. I actually just did some bench testing and verified that everything is working perfectly. The simulator agrees as well. There's a section of the wheel where two spokes cross and, if I line my sensor up with that, there will be 9 triggers evenly spaced around the wheel. The circumference of the tire is 2161mm. I calculated that to be about 945 hundredths of an inch per pulse. vehicle_speed is in miles per hour. Thanks for taking interest in my project :o)

It is interesting.

This

 uint32_t update_time = -1;

is nonsense - an unsigned integer can not be negative. I don't think you need that initial value at all.

I had to find an odd factor of 10, I think it is mostly the magic number 945 in units 0f 0.01.

I did (in another version) change some of your calculations to floating point, but if you are getting numbers you like I never argue with success.

And I wrote

  if (time_us - update_time < update_interval) // time for update?
      return;

  update_time += update_interval;

It will be interesting to see how the real sensor works. Keep us informed.

a7

-1 should overflow to the highest possible int value. I wrote that thinking that it would guarantee the loop would run on the first iteration, but after looking at it, I don't think it will. Maybe I can do this:

uint32_t update_time = -update_interval;

I wanted to do all the math with integers initially either because I thought it would be simpler or just because I wanted to. But it's starting to look like I should use floating point math now. I just wrote some logic for the speed calculation which prevents bouncing between the edges of two numbers.

// vehicle speed
float vehicle_speed = 0;
// if the last pulse was so long ago that our speed would be less than 1 mile per hour, let vehicle_speed remain 0.
if (time_us - vss_time_ref <= magic_number_vs)
{
  vehicle_speed = magic_number_vs / vss_period;
}
// prevent bouncing of vehicle speed value
if (vehicle_speed < vehicle_speed_ref && vehicle_speed >= vehicle_speed_ref - 0.5)
{
  vehicle_speed = ceil(vehicle_speed);
}
else
{
  vehicle_speed = floor(vehicle_speed);
}
vehicle_speed_ref = vehicle_speed;

I've been told that I should display the tenth of a mile per hour as well, but I'm not sure if I want to.

To force execution first time thru use

do (whatever) while (condition) 

How do you use this idea to solve the OP's problem with this sequence…

uint32_t time_us = micros();
  if (time_us - update_time >= update_interval)
  {
    update_time += update_interval;
  }
  else
  {
    return;
  }

in the loop() function of the code he posted?

Just forget the fact that it won't run for the very first 100 milliseconds. There is no large deal here. You could also test for update_time being 0, but that seems silly for something that happens only once.

You could also just delay() long enough at the end of setup(), if indeed setup() is not already running past the point where time_us will be greater than or equal to update_interval.

Oh, look:

    delay(2500);

Right there in setup(), so.

HTH

a7