Make ISR function from polling function

Hey, im bit stuck with code, can someboady please help me to make this function into ISR function? Main code is pretty long and does different functions, so holdandpeak() function isn't precise thats why it could be cool to make it with ISR. Codes main thing is to read rising edge from sensor and make one large pulse to mosfet, to get more current flowing to open solonoid, after that it goes into pwm mode to hold solonoid open but with less current and after x time it turns off and waits next rising edge input and the cycle repeats.

bool triggered = false;

void setup() {
  DDRB = (1 << PB2);
  pinMode(2, INPUT_PULLUP);
  attachInterrupt(0, trigger, FALLING);
}

void loop() {
  if (triggered) {
    peakAndHold(2000, 20);
    triggered = false;
  }
}

void trigger() {
  triggered = true;
}

void peakAndHold(int timeout, int duration) {
  delayMicroseconds(timeout); // waits x time to start pulse
  PORTB = (1 << PB2); // pin HIGH
  delayMicroseconds(1000); // how long first pulse are. Send higher current to solonoid
  for (int i = 1; i < duration; i++) { // make "PWM" and run for x times. hold solonoid open, but with lower current
    PORTB = (1 << PB2); // pin HIGH
    delayMicroseconds(100); // "PMW" high time
    PORTB &= ~(1 << PB2); // pin LOW
    delayMicroseconds(100); // "PMW" low time
  }
  delayMicroseconds(5000); // "button debounce" time - not so important in main code
}

Pulse looks like this: red line - button press, blue line - output

What is 'precise' in this environment ( µs, ms? ). Maybe your main code is written with blocking functions?
You cannot turn your peakAndHold into an ISR. An ISR cannot have parameters, and it must be short. delay() is not allowed in an ISR. Your peakAndHold runs much too long for an ISR.

Maybe you need timer functions to generate this signal.

Precise environment is µs.
Main code doing math and "talking" with python app to send variables.
I know that delay() is not allowed in an ISR. I meant somehow preloading TCNT1 and OCR1A registers or some other way, make pulse with compare a and then run after that pwm "routine"? Just cant think of logic how to do that

I think that's the correct way to go. I assume you are using an UNO? Let timer 1 do the job when your trigger fires. In the trigger ISR you set the inital values for timer 1 ( the delay time until your pulse. The rest is done in a timer ISR. First for your initial pulse length, and than you create the pulses with OCR1 and count them in the timer ISR. After the last pulse you switch off everything.
I think you have to study the datasheet of the mega328 a little bit :wink:

Yes, im using atmega328
And yes, "Let timer 1 do the job when your trigger fires." thats the plan.

enum ScheduleStatus {OFF, PENDING, RUNNING};
ScheduleStatus isrStatus;
unsigned int startCompare; //The counter value of the timer when this will start
unsigned int endCompare;

void setup() {
  DDRB = (1 << PB2);
  pinMode(2, INPUT_PULLUP);
  attachInterrupt(0, trigger, FALLING);

  //Setup 16 bit timer1
  TCCR1A = 0x00;        //Disable Timer1 while set it up
  TCNT1 = 0;            //Reset Timer Count
  TIFR1 = 0x00;         //Timer1 INT Flag Reg: Clear Timer Overflow Flag
  TCCR1A = 0x00;        //Timer1 Control Reg A: Wave Gen Mode normal
  TCCR1B = (1 << CS12); //Timer1 Control Reg B: Timer Prescaler set to 256
}

void loop() {
}

void setSolonoidSchedule(unsigned long timeout, unsigned long duration) {
  if (isrStatus == RUNNING) {
    return;
  }
  startCompare = TCNT1 + (timeout >> 4);
  endCompare = startCompare + (duration >> 4);
  OCR1A = startCompare;  //Use the A copmare unit of timer 1
  isrStatus = PENDING;  //Turn this schedule on
  TIMSK1 |= (1 << OCIE1A);  //Turn on the A compare unit (ie turn on the interrupt)
}

ISR(TIMER1_COMPA_vect) {  //solonoid interupt service routine
  if (isrStatus == PENDING) { //Check to see if this schedule is turned on
    PORTB = (1 << PB2);
    isrStatus = RUNNING; //Set the status to be in progress (ie The start callback has been called, but not the end callback)
    OCR1A = endCompare;
  }
  else if (isrStatus == RUNNING) {
    PORTB &= ~(1 << PB2);
    isrStatus = OFF; //Turn off the schedule
    TIMSK1 &= ~(1 << OCIE1A);      //Turn off this output compare unit (This simply writes 0 to the OCIE1A bit of TIMSK3)
  }
}

void trigger() {
  setSolonoidSchedule(10000, 2000);
}

But how to make PWM after that pulse?

And yes, i truly need to study more datasheet of the mega328 :smiley:

Ohh, got it now, thanks buddy :handshake:

enum ScheduleStatus {OFF, PENDING, RUNNING, RUNNING2, RUNNING3, RUNNING4, RUNNING5};
ScheduleStatus isrStatus;
unsigned int startCompare; //The counter value of the timer when this will start
unsigned int endCompare;

/*In the trigger ISR you set the inital values for timer 1 ( the delay time until your pulse. 
The rest is done in a timer ISR. 
First for your initial pulse length, and than you create the pulses with OCR1 
and count them in the timer ISR. After the last pulse you switch off everything.*/

void setup() {
  DDRB = (1 << PB2);
  pinMode(2, INPUT_PULLUP);
  attachInterrupt(0, trigger, FALLING);

  //Setup 16 bit timer1
  TCCR1A = 0x00;        //Disable Timer1 while set it up
  TCNT1 = 0;            //Reset Timer Count
  TIFR1 = 0x00;         //Timer1 INT Flag Reg: Clear Timer Overflow Flag
  TCCR1A = 0x00;        //Timer1 Control Reg A: Wave Gen Mode normal
  TCCR1B = (1 << CS12); //Timer1 Control Reg B: Timer Prescaler set to 256
}

void loop() {
}

void setSolonoidSchedule(unsigned long timeout, unsigned long duration) {
  if (isrStatus == RUNNING) {
    return;
  }
  startCompare = TCNT1 + (timeout >> 4);
  endCompare = startCompare + (duration >> 4);
  OCR1A = startCompare;  //Use the A copmare unit of timer 1
  isrStatus = PENDING;  //Turn this schedule on
  TIMSK1 |= (1 << OCIE1A);  //Turn on the A compare unit (ie turn on the interrupt)
}

ISR(TIMER1_COMPA_vect) {  //solonoid interupt service routine
  if (isrStatus == PENDING) { //Check to see if this schedule is turned on
    PORTB = (1 << PB2);
    isrStatus = RUNNING; //Set the status to be in progress
    OCR1A = endCompare;
  }
  else if (isrStatus == RUNNING) {
    PORTB &= ~(1 << PB2);
    OCR1A = endCompare + 20;
    isrStatus = RUNNING2;
  }
  else if (isrStatus == RUNNING2) {
    PORTB = (1 << PB2);
    OCR1A = endCompare + 40;
    isrStatus = RUNNING3;
  }
  else if (isrStatus == RUNNING3) {
    PORTB &= ~(1 << PB2);
    OCR1A = endCompare + 80;
    isrStatus = RUNNING4;
  }
  else if (isrStatus == RUNNING4) {
    PORTB = (1 << PB2);
    OCR1A = endCompare + 100;
    isrStatus = RUNNING5;
  }
  else if (isrStatus == RUNNING5) {
    PORTB &= ~(1 << PB2);
    isrStatus = OFF; //Turn off the schedule
    TIMSK1 &= ~(1 << OCIE1A);      //Turn off this output compare unit (This simply writes 0 to the OCIE1A bit of TIMSK3)
  }
}

void trigger() {
  setSolonoidSchedule(10000, 2000);
}

Just need to make it now with some other logic not with 5x -10x times "same" code lines :smiley:

You could simply use a counter.
And instead of these if .. else if I would use a switch statement.
You need less ISR overhead if you let OCR1B switch your output.

I dont think that switch statement would be the best in my case, because i need 1 initial long pulse, and after that somwhere between 10 to 20 pwm pulses, and it will change, how long i need to make solonoid stay open based on other variables. Need to think of some other logic :smiley:

The switch is only a better alternative to your many if...else if. It has nothing to do with your pulse length and the flexibility to change them.
But I would definitely let the timer switch your output directly. It makes things easier.
Something like this:


const byte clkStart = (1<<CS11);  // with prescaler 8 ( 0.5µs per tic )
const byte clkStop = 0b11111000;  // mask to stop the timer ( bit 0..2 = 0 )
volatile int firstTimeout = 2000;  // timout from edg ISR to first pulse
volatile int duration = 20;      // number of pulses
const int firstPulse = 1000;
const int pwmPulse = 100;
volatile byte isrState;

void initTimer1() {
  // initial settings ( especially PWM mode )
  TCCR1B = 0;   // stop timer
  // use mode 12 ( CTC-mode, TOP is ICR1 ), OCR1B toggles output
  TCCR1A = (0<<COM1B1) | (1<<COM1B0) |(0<<WGM11) | (0<<WGM10);
  TCCR1B = (1<<WGM12) | (1<<WGM13);
  TIMSK1 = 0;
  TCNT1 = 0;
}
void setup() {
  pinMode( 10,OUTPUT);
  pinMode(2, INPUT_PULLUP);
  initTimer1();
  attachInterrupt(digitalPinToInterrupt(2), trigger, FALLING);
}

void loop() {
  // you can change firstTimeout and duration in loop
}

void trigger() {
  if (isrState == 0 ) {
    // edge detected and pulses not running, start timer 1 ,set output at OCR1B
    // set TOP
    ICR1 = (firstTimeout+firstPulse)*2; // *2 because tic is 0.5 microseconds
    OCR1B = firstTimeout*2;
    TIMSK1 = (1<<ICF1);     // Interrupt at TOP ( = ICR1 )
    TCNT1 = 0;
    TCCR1B |= clkStart;     // start with prescaler = 8  
    isrState = 1;
  }
}

ISR(TIMER1_CAPT_vect)  {
  static byte pwmCount;
  // End of pulse reached
  TCCR1C = (1<<FOC1B);
  if ( isrState == 1 ) {
    // end of first pulse, start pwm pulses
    pwmCount = 0;
    ICR1 = 2*pwmPulse *2; // *2 because tic is 0.5 microseconds
    OCR1B = pwmPulse*2;
    isrState = 2;
  } else {
    // counting PWM pulses
    if ( ++pwmCount >= duration ) {
      // we are at the end, stop the timer
      PORTB &= ~(1 << PB2);
      TCCR1B &= clkStop;
      TCNT1 = 0;
      isrState = 0;
    }
  }
}

This works excelent, thank you very much :handshake: :hugs:

You're welcome :slightly_smiling_face:

Is it? You want to control a solenoid, which have switching time in ms-range.
Despite, I think you can do it without any ISR routine (at least for the PWM-part). With the manual of ATmega328 please have a look on the following:

  • When, e,g, configured in Mode 8 (PWM, Phase and Frequency Correct), TOP can be defined through value stored in ICR1

  • You can adjust PWM frequency by both pre-scaler and ICR1

  • Duty Cycle is defined by OCR1A (and OCR1B) while OCR1A = 0...ICR1, i.e. duty cycle OCR1A/ICR1 * 100%

  • With OCR1A = 0 duty cycle is 0% (!) - which equals "off", with OCR1A = ICR1 duty cycle is 100% - which is "on". In both cases no(!) narrow spikes are created (this applies not for all PWM modes).

  • OCR1A are doubled buffered registers. Thus they can be written in your loop()-function - which makes your ISR obsolete to do all all the switching (on, off, some PWM)

Just one remark: this is my theory from reading the data sheet. I have actually done this in my project ... and, well, it works.

So code may look like this (adapted from mine):

  // Duty-Cycle is set by adjusting OCR1A and OCR1B, ranging from 0..ICR1 (ICR1 provides resolution)
  //    OCR1x = 0    results in permantetly off  
  //    OCR1x = ICR1 results in permanently on
  //
  // Timer 1 is also used to trigger ISR on counter overflow.
  
    TCCR1A = 0;                                   // set timer controller register to zero to ...
    TCCR1B = 0;                                   // ... know from what we start (TCCR1A and TCCR1B)
    TCNT1  = 0;                                   // initialize counter value to 0
    // Control Register
    //
    // Digital pins shall be cleared on compare match with OCR1A and OCR1B
    TCCR1A |= (1 << COM1A1);                      // OCR1A: Connect to pin PB1
    TCCR1A |= (1 << COM1B1);                      // OCR1B:                PB2
    // Use Mode 8, with ICR1 as TOP, Phase and Frequency Correct PWM, OCR1x updated at BOTTOM
    TCCR1B |= (1 << WGM13);                      
    TCCR1B |= (0 << WGM12);                      
    TCCR1A |= (0 << WGM11);                      
    TCCR1A |= (0 << WGM10);                      
    // Prescaler = 8, PWM-Freq = 16*10^6/(2*8*TOP)) = 1960Hz, smallest pulse 2*Prescaler/f_clk =1us
    TCCR1B |= (0 << CS12);
    TCCR1B |= (1 << CS11);
    TCCR1B |= (0 << CS10);    
    // Set TOP Register (with TOP 512 1960Hz is obtained)
    ICR1 = 512;               
    // OCR1A and OCR1B will define duty-cycle, ranging from 1 to ICR1
    OCR1A = 0;                                    // Range 0..ICR1,    0:   0% (totally off)
    OCR1B = 0;                                    //              , ICR1: 100% (totally on )
    // Output pins are OC1A (=PB1, HW-Pin 15) and OC1B (=PB2, HW-Pin 16), stupid Arduino pinNo apply
    pinMode(pinPB1, OUTPUT);                      // set Data Direction Register by pinMode()
    pinMode(pinPB2, OUTPUT);                      // dito
    // enable timer compare interrupt via TOV1 (1960Hz) (--> TIMER1_OVF)
    TIMSK1 |= (1 << TOIE1);

You would need to replace pinPB1, and pinPB2 with Arduino-Naming-Konvention. Also not sure if I have all things corrected for your application (mine is using some extra feature) ... so check against data sheet.

As said, OCR1A and OCR1B do all stuff an can be set in loop(): on, off, some (visible) PWM. Just highlighting "visible" because both on and off can be seen as PWM signal as well - as said: 0% or 100%. Simply write, e.g.,

OCR1A = 0;

to turn your solenoid off. And that's it!

(or OCR1A = 384 to obtain a duty cycle of 75% (with ICR1 = 512))