Trying to create a one shot pulse

I want to get audible feedback when a button is pressed, and the processor is going to be busy counting, so delay is not an option.
I have written this code, which I find straighforward, but it functions in a strange way.
It acts just as if the button has a loose connection. Some few times the response is right, sometimes I get response every odd button press, the response is random and not reliable. There is something basically wrong with the code, but I am blind to it, so any help will be appreciated greatly.

Button is connected to pin 3, and the output is monitored on pin 4

I guess I am doing something so silly that I am overlooking the obvious.

here is the code:

const int beepPin = 4;
const byte interruptPin3 = 3;
unsigned long startMillis = 0;
unsigned long stopMillis = 0;
int val = 0;
int pulselength = 10;


void setup() {
  pinMode (beepPin, OUTPUT);
  pinMode(interruptPin3, INPUT_PULLUP);
  attachInterrupt(digitalPinToInterrupt(interruptPin3), triggerISP, FALLING);
  Serial.begin(9600);
}



void loop() {
  if (val == 1)
  {
    stopMillis = millis();
    digitalWrite (beepPin, HIGH);

  }
  if (stopMillis - startMillis > pulselength)
  {
    digitalWrite (beepPin, LOW);
    val = 0;
  }

}


void triggerISP()
{
  val = 1;
  startMillis = millis ();

}

Cheers, Finn Hammer

You need to declare your variable 'val' as volatile so the compiler does the right thing since you access it within your ISR and the main loop. You should also make is a byte or uint8_t type so the access is a single instruction.

You really don't need interrupts for detecting button presses.

Which Arduino board ? The Arduino Uno does not have a normal interrupt on pin 3 yes, it does. Why using a interrupt ? You can check a button in the loop(). When using interrupts, then some variables needs to be 'volatile'. However 'volatile' is not enough, when using a 4-byte variable in a interrupt on a 1-byte microcontroller. Why is the variable for a pin sometimes a byte and sometimes a int ? I can go on with these questions, but I will stop here :wink:

This part is part of a software-millis-timer. It should no longer be executed when the millis-timer is no longer needed. However it is always running. For weeks, months, years, perhaps hundreds times per second.

if (stopMillis - startMillis > pulselength)
{
  digitalWrite (beepPin, LOW);
  val = 0;
}

Suppose the 'val' is set to '1' in the interrupt, then this code immediately turns it to '0'.

Can you check the button in the loop() ? Then turn to basic millis() usage as explained in the Blink Without Delay.
I would add a 'bool' variable to enable and disable the millis-timer. For a one shot pulse, I would make the millis-timer turn itself off. Then you get: millis_single_delay.ino

P.S.: Can you check the link to your website under your avatar ? The link is wrong, and also the website is empty.

[EDIT] The Uno has a normal interrupt on pin 3.

You say that your code is "busy counting". While it is "busy", is it running through the main loop() over and over? Or, is it in some other loop spinning somewhere and not passing through the main loop() while this is going on?

-jim lee

jimLee:
You say that your code is "busy counting". While it is "busy", is it running through the main loop() over and over? Or, is it in some other loop spinning somewhere and not passing through the main loop() while this is going on?

-jim lee

No, I said" it will be". For the time being I am only trying to get the single pulse to work.

Koepel:
Which Arduino board ? The Arduino Uno does not have a normal interrupt on pin 3. Why using a interrupt ? You can check a button in the loop(). When using interrupts, then some variables needs to be 'volatile'. However 'volatile' is not enough, when using a 4-byte variable in a interrupt on a 1-byte microcontroller. Why is the variable for a pin sometimes a byte and sometimes a int ? I can go on with these questions, but I will stop here :wink:

This part is part of a software-millis-timer. It should no longer be executed when the millis-timer is no longer needed. However it is always running. For weeks, months, years, perhaps hundreds times per second.

if (stopMillis - startMillis > pulselength)

{
 digitalWrite (beepPin, LOW);
 val = 0;
}



Suppose the 'val' is set to '1' in the interrupt, then this code immediately turns it to '0'.

Can you check the button in the loop() ? Then turn to basic millis() usage as explained in the [Blink Without Delay](https://www.arduino.cc/en/Tutorial/BuiltInExamples/BlinkWithoutDelay).
I would add a 'bool' variable to enable and disable the millis-timer. For a one shot pulse, I would make the millis-timer turn itself off. Then you get: [millis_single_delay.ino](https://github.com/Koepel/Fun_with_millis/blob/master/millis_single_delay.ino)

P.S.: Can you check the link to your website under your avatar ? The link is wrong, and also the website is empty.

It is an UNO, the webpage URL is correct in my profile, but the site has been hacked, is closed by the ISP, and I have not had time to upload the site untarnished by malware.
And why am I using Interrupt? It is a cool way to be sure I never miss a count. When the pulsecode works, I am going to record up to several thousand pulses per second on the interrupt.
I tried your millis_single_delay.ino, contrary to what you write, it starts the 4sec. timer the moment the button goes low, and the led stays on beyond the 4 sec, untill the button is released. So the led is on as long as the button is pressed, plus 4 seconds.
Any help with changing the code with state change will be appreciated
Cheers, Finn Hammer

hammertone:
contrary to what you write, it starts the 4sec. timer the moment the button goes low, and the led stays on beyond the 4 sec, untill the button is released. What am I missing?

No, over here it is working just fine, and so does it for everyone else.
The button is between GND and pin 2. Nothing else is connected. When the button is released, the led stays on for 4 seconds and then the led turns off. I don't know what you did, but somehow you did something else, or something is broken.
Can you try once more ? Did you change something ? Can you show the changed sketch ?
If you can not make that work, how can we help when adding an interrupt ?

The link under your avatar is not okay. It is this:

https://forum.arduino.cc/www.hammertone.com

Oops :-[ Pin 2 and pin 3 are normal interrupts for the Arduino Uno. What I wrote about pin 3 was wrong. Sorry.

When a pulse or pulses are detected at a pin, then a output pin should be high for some time ?
This is a good tutorial by Nick Gammon about interrupts: https://gammon.com.au/interrupts.

What about setting a 8-bit variable in the ISR, and do all the millis() things in the loop() ? That avoids a few problems (such as using a 4-byte variable on a 1-byte microcontroller).
Then I can replace the button by the ISR and do the same thing.

const int pulseInputPin = 3;
const int ledPin = LED_BUILTIN;

unsigned long previousMillis;
const unsigned long interval = 1000;
bool enabled = false;
volatile bool pulseDetected = false;

void setup()
{
  pinMode( pulseInputPin, INPUT_PULLUP);
  pinMode( ledPin, OUTPUT);

  attachInterrupt( digitalPinToInterrupt( pulseInputPin), triggerISP, FALLING);
}

void loop()
{
  unsigned long currentMillis = millis();

  if( pulseDetected)
  {
    digitalWrite( ledPin, HIGH);  // turn on led
    previousMillis = currentMillis;
    enabled = true;               // turn on software timer
    pulseDetected = false;        // reset this variable
  }

  if( enabled)                    // software timer is active ?
  {
    if( currentMillis - previousMillis >= interval)
    {
      digitalWrite( ledPin, LOW); // turn off led
      enabled = false;            // stop software timer
    }
  }
}

void triggerISP()
{
  pulseDetected = true;
}

I have changed the pin for the button to pin 3.
Should the led stay on as long as pin 3 is low ? Then the sketch must be changed, because now it is triggered by a interrupt on a falling edge. It is allowed to use a digitalRead() for a interrupt input.

Then I would change it to:

if( pulseDetected or digitalRead( pulseInputPin) == LOW)  // detect a low level and a fast pulse both

P.S.: You don't have to quote my whole post, what I wrote is already there :wink:

@blh64, rub it in some more ! I have corrected the original post at 00:09:19, and your post is also from 00:09:19 :o

Koepel:
@blh64, rub it in some more ! I have corrected the original post at 00:09:19, and your post is also from 00:09:19 :o

Removed. No rubbing it in

Koepel:
This part is part of a software-millis-timer. It should no longer be executed when the millis-timer is no longer needed. However it is always running. For weeks, months, years, perhaps hundreds times per second.

if (stopMillis - startMillis > pulselength)

{
 digitalWrite (beepPin, LOW);
 val = 0;
}



Suppose the 'val' is set to '1' in the interrupt, then this code immediately turns it to '0'.

Can you check the button in the loop() ? Then turn to basic millis() usage as explained in the [Blink Without Delay](https://www.arduino.cc/en/Tutorial/BuiltInExamples/BlinkWithoutDelay).
I would add a 'bool' variable to enable and disable the millis-timer. For a one shot pulse, I would make the millis-timer turn itself off. Then you get: [millis_single_delay.ino](https://github.com/Koepel/Fun_with_millis/blob/master/millis_single_delay.ino)

Koepel,
I modified your code to make the pulse start the moment the button is pressed.
Thankyou for helping me to think. Those boolean enablers/disablers are strong helpers, I will not forget them soon. I have learned something usefull about how loop executes the code

const int switchPin = 2;
const int ledPin = LED_BUILTIN;
const int interruptPin2 = 2;
unsigned long previousMillis;
const unsigned long interval = 10000;
bool enabled = false;
bool pulsestart = false;

void setup()
{
  pinMode( switchPin, INPUT_PULLUP);
  pinMode( ledPin, OUTPUT);
  Serial.begin(9600);
  pinMode(interruptPin2, INPUT_PULLUP);
  attachInterrupt(digitalPinToInterrupt(interruptPin2), pulserISP, FALLING);
}

void loop()
{
  unsigned long currentMillis = micros();
  
  if(pulsestart)  
  {
    digitalWrite( ledPin, HIGH);  
    previousMillis = currentMillis;
    enabled = true;
 
    pulsestart = false;
  }
  
  
  if( enabled)      
  {
    if( currentMillis - previousMillis >= interval)
    {
      digitalWrite( ledPin, LOW);  
      enabled = false;     
    }
  }
}

void pulserISP ()

{
  pulsestart = true;
 
}

Ok, I just read the comments written while I was coding, Koepel, you beat me to it, of course.

Thanks for the help,

Cheers, Finn Hammer

A beep of 10000 microseconds ? That is the same as 10 milliseconds.
If millis() is possible then I prefer to use millis().

The 10 milliseconds with millis() is just as accurate. The accuracy depends on the other code that will be in the sketch. Suppose there is other code that takes 2 milliseconds, then the beep can start 2 ms later or be 2ms longer.

I think the sketch is more clear if you just use the 'switchPin' for pin 2 and not the 'interruptPin2'.

Your 'pulsestart' should be 'volatile'.
The compiler could optimize the code and keep the variable 'pulsestart' in registers in the loop() and ignore the changes that the interrupt makes to the real variable 'pulsestart' in memory.
The 'volatile' keyword tells the compiler that a variable can be changed by an interrupt and then the compiler uses the actual value of 'pulsestart' that is in memory.
Although this is almost never a problem and is mainly theoretical, you should still add the keyword 'volatile' :wink:

Koepel,

10000 seems silly, no doubt about it. But it is a reminisence from testing at 10µS, to see how accurate the relationship between interruptPin2 and pin13 is. Not that it matters here, but always nice to know.
If it should matter, I could disable interrupts briefly, but not here.
I guess You are right about that interruptPin2, I will change that.
The volatile prefix did not change the length of the compiled code, it is still 14584 bytes long, with the led-display stuff I have now added.

Ultimately, this is going to become a Neutron counter readout. The raw data comes from a neutron detector tube, and this data, is being detected and sorted by an opamp and a comparator, to deliver 2µS long pulses fed into what is still (but not for long, I promise!) interruptPin2.
It is then displayed as total counts and counts per minute.
There will be a stop/start switch and a reset switch. That is all, and a needed ascessory to operating a Farnsworth/Hirsch fusion reactor, which I am currently building.

I just tested it with the function generator, and it keeps count up to somewhere between 100kHz and 130kHz, where it starts to hiccup. This is satisfactory, and there is always the teensy's if I should face higher counts, which is unlikely.
The audio does not keep up with that count rate, but then, my ears are shot and fall off at 10K anyway.

Thanks for your help, Sir! You are a good professor.

Cheers, Finn Hammer

About 100…130kHz ? But if you need to count the pulses, then you need more code in the ISR.
The ISR for an external interrupt takes 5.125 µs on a Arduino Uno.
https://gammon.com.au/interrupts

The microcontroller of a Arduino Uno has hardware timers, for example the T1 input signal for Timer1. I don’t know a good library that uses that :frowning:
I found this, but I did not check or test it: https://forum.arduino.cc/index.php?topic=660418.msg4470461#msg4470461

You mentioned Teensy boards, but there are also Arduino boards with SAMD21G processor or a ESP32.

What kind of buzzer is it ? Does it buzz when applying 5V to it ? Or is it a piezo element that needs a certain frequency with the tone() function ?

You need to declare pulsestart as volatile since you use it both in the main loop and your ISR.

volatile bool pulsestart = false;

I added a variable in the ISP,

neutronsCounted,

which I increment every time the ISP is executed. Seem to work, but:

After adding the OLED display, the pulselength of the audio increased to 250mS, not very usefull.

I can get usefull audio from a pulse as short as 10µS.

Should I try to limit the update of the display, and what should I use the hardware timers for, that you suggested-

recent code:

#include <Wire.h>
#include <Adafruit_GFX.h>
#include <Adafruit_SSD1306.h>
#define OLED_RESET 4
Adafruit_SSD1306 display(OLED_RESET);
const int switchPin = 2;
const int ledPin = LED_BUILTIN;
const int interruptPin3 = 3;
const int interruptPin2 = 2;
long neutronsCounted = 0;
unsigned long previousMillis;
const unsigned long interval = 10;
bool enabled = false;
volatile bool pulsestart = false;

void setup()
{
  pinMode( switchPin, INPUT_PULLUP);
  pinMode( ledPin, OUTPUT);
  Serial.begin(9600);
  pinMode(interruptPin3, INPUT_PULLUP);
  pinMode(interruptPin2, INPUT_PULLUP);
  attachInterrupt(digitalPinToInterrupt(interruptPin3), neutcountedISP, FALLING);
  attachInterrupt(digitalPinToInterrupt(interruptPin2), pauseISP, FALLING);
  display.begin(SSD1306_SWITCHCAPVCC, 0x3C);  // initialize with the I2C addr 0x3C
}

void loop()
{
  displayview ();
  display.display();

  unsigned long currentMillis = millis();

  if (pulsestart)
  {
    digitalWrite( ledPin, HIGH);
    previousMillis = currentMillis;
    enabled = true;

    pulsestart = false;
  }


  if ( enabled)
  {
    if ( currentMillis - previousMillis >= interval)
    {
      digitalWrite( ledPin, LOW);
      enabled = false;
    }
  }
}

void neutcountedISP ()

{
  pulsestart = true;

  neutronsCounted ++;


}

void pauseISP ()
{
}

void displayview () {


   display.clearDisplay();
   display.setTextColor(WHITE);
   display.setTextSize(1);
   display.setCursor(0, 0);
   display.print("Neutron Counter");
  display.setTextSize(2);
  display.setCursor(0, 9);
  display.print(neutronsCounted);
   display.setTextSize(1);
   display.setCursor(85, 25);
   display.print("CPS");
}

Cheers, Finn Hammer

Think of the loop() as if it is running hundreds times per second, maybe even faster.
Updating the display that often is not needed.
You could add a millis-timer for the display.

Are you using the 10 ms for the buzzer to get a audible sound from a piezo buzzer ?
Then I would prefer a certain tone() to a buzzer for a longer time (100 ms or more). It would be possible to increase the frequency of the tone when more pulses are detected.

The OLED uses the I2C bus, which is interrupt driven. That will influence your interrupt to detect the pulses. The Adafruit libraries use a lot of ram and take time.
Can you replace it with a non-I2C LCD ? Arduino - LiquidCrystal.
If you want to keep the OLED, then there is the U8G2 library. It can use software I2C without interrupt. The U8G2 is hard to understand, but there is a simple and fast U8x8 library.

Your Arduino board should do the interrupt. That is the main thing. Anything you add can disturb that. There are libraries that turn off interrupts for some time, or libraries that take some time.

When you are about to display the number of 'neutronsCounted', then you are displaying a variable of 4 bytes. A interrupt could happen while only 1, 2 or 3 bytes of that variable are read. That interrupt could alter the variable and you would have 4 bytes which are part old and part new.

There are two options:

  • Ignore it. It is just a short flash of a wrong number on the display which will almost never happen.
  • Make a copy with interrupts turned off, and use the copy in the loop().
volatile unsigned long neutronsCounted = 0;

void loop()
{
  noInterrupts();
  unsigned long copy_neutronsCounted = neutronsCounted;
  interrupts();

  Serial.println( copy_neutronsCounted);
}

This topic was automatically closed 120 days after the last reply. New replies are no longer allowed.