ISR vs Servo's

Forum 2005-2010 (read only) Software Syntax & Programs
1

I am currently clueless.

After struggling with an ISR ft. a series of 74HC595 Shift Registers (which do work now ;D) I am stuck at implementing a piece of code that triggers 4 individual servo's.

The code I have is working when I use it independently;

#include <Servo.h> 
 
Servo rechtsvoor;  
Servo linksvoor;
Servo rechtsachter;
Servo linksachter;
 

int poslinksvoor = 80;    // variable to store the servo position 
int posrechtsvoor = 165;
int poslinksachter = 130;
int posrechtsachter = 10;

 int i = 0;
 int initial = 0;

void setup() { 
  rechtsvoor.attach(3);  //rechtsvoor
  linksvoor.attach(5);  // rechtsachter
  rechtsachter.attach(6);  // rechtsachter
  linksachter.attach(9);  // linksachter
} 
 
void loop() 
{ 
 if(initial==0){
     linksachter.write(poslinksachter); // lager is de goede richting op
     rechtsvoor.write(posrechtsvoor);  // lager is de goede richting op
     linksvoor.write(poslinksvoor); // hoger is de goede richting op
     rechtsachter.write(posrechtsachter);  // hoger is de goede richting
     delay(10);
    }
    initial = 1;
 }
}

The code basically 'normalizes' the servo's for the application I am building (I have some functions ready that simply rotate the servo between certain values (80-150 for instance), but I left these away in the code above to keep it more clear and to have a better overview for you!

So this works, but when I implement this in my code for my leds (a long boring ISR / shift routine code) it simply doesn't work. The code verifies and works (the leds still fade etc.) but the servo's don't act at all.

It seems that the code/arduino simply skips the whole servo-piece at all.

Anyone got any idea's? I'm kinda out. Is it possible that because all of my variables and pins are '#define'-ed in my code that the simple Servo linksachter; doesn't work anymore?

How do you '#define' a servo anyway?

Hope that you can help me! I have an Duemilanove /w atmel 168

2

Anyone got any idea's?

Yes. I have two ideas but you're probably not going to like them...

  • The list of what you're trying to do in the interrupt service routine is long. I suspect too long. Interrupt service routines are meant to be short and to the point. Service the interrupt and return. The phrase "a long boring ISR" is usually a sign of trouble.
  • It's very likely that the Servo library doesn't work in the context of an interrupt.

It's very likely that you can accomplish what you're trying to do without interrupts. In many cases, the solution without using interrupts is simple, elegant, and easy to debug. I suggest describing what you're trying to do and ask for suggestions regarding the implementation.

3

Hey Coding Badly!

Tnx for your reply, I will elaborate more on what I want to do:

I currently have this ISR code (aside from the void loop and some other functions that arrange some color effects for my LEDS):

ISR(TIMER1_OVF_vect) { /* Framebuffer interrupt routine */
  TCNT1 = __TIMER1_MAX - __TIMER1_CNT;
  byte cycle;
  
  digitalWrite(__display_enable,LOW);  // enable display inside ISR
  
  for(cycle = 0; cycle < __max_brightness; cycle++) {
    byte led;
    byte row;
    byte red;  // current sinker, on when (0)

    digitalWrite(__spi_latch,LOW);  // prepare the daisy chained 595s to receive data bytes

    for(row = 0; row <= __max_row; row++) {
      red = B11111111;  // off
      for(led = 0; led <= __max_led; led++) {
        if(cycle < brightness_red[row][led]) {
          red &= ~(1<<led);
        }
      }
      spi_transfer(red); // send 1 byte of data
    }
    digitalWrite(__spi_latch,HIGH); // end data transfer for all bytes
  }
  digitalWrite(__display_enable,HIGH);    // disable display outside ISR
}

where: maxbrightness = 32, max row = 8, and maxled = 8, so yeah it is pretty long I guess :frowning: I have about 66 leds that I need to individually address, thats why I chose for the 74HC959 in combination with this ISR function (for speed and resolution). It works pretty good actually! So basically, this is what the ISR does (other stuff in that code concern color-functions, timers and such).

What I want to do is to combine these codes (so the code of the servo as posted in the OP and the code of the LED's (the ISR with the appropriate functions and timer-setup)). This way I can create matching servo-angles to my color-schemes (which I want to do for my project).

The total idea is to have a few buttons that match certain profiles, and that these profiles trigger the servo's and the LEDS.

I really hope to have this working by next monday, since I will be testing my creation :-[

4

The servo library uses timer1, this conflicts with your code.

You can let a timer2 overflow interrupt handle the leds. I have some old code you can look at. I had timer2 setup routines in there, so you can get the idea of what to change yourself.

these are the chunks:

#define __TIMER2_MAX 0xFF // 8 bit CTR
#define __TIMER2_CNT 0xFF // max 28 levels !

Again, you'll have to tune the settings.

ISR(TIMER2_OVF_vect) {
  TCNT2 = __TIMER2_MAX - __TIMER2_CNT; // precharge TIMER2 to maximize ISR time --> max led brightness
  //..
}

void setup(void) {
  // ...
  setup_timer2_ovf();
  // ...
}

void setup_timer2_ovf(void) {
  // Arduino runs at 16 Mhz...
  // Timer Settings, for the Timer Control Register etc. , thank you internets. ATmega168 !
  // Timer2 (8bit) Settings:
  // prescaler (frequency divider) values:   CS22    CS21   CS20
  //                                           0       0      0    stopped
  //                                           0       0      1      /1      62500 Hz
  //                                           0       1      0      /8       7813 Hz
  //                                           0       1      1      /32      1953 Hz
  //                                           1       0      0      /64       977 Hz
  //                                           1       0      1      /128      488 Hz
  //                                           1       1      0      /256      244 Hz
  //                                           1       1      1      /1024      61 Hz
  // irq_freq = 16MHz / ( 256 * prescaler )
  //
  // set irq to 61 Hz: CS22-bit = 1, CS21-bit = 1, CS20-bit = 1
  TCCR2B |= ( (1<<CS22) | (1<<CS21) | (1<<CS20));      
  //TCCR2B &= ~( (1<<CS20) );       
  // Use normal mode  
  TCCR2A &= ~( (1<<WGM21) | (1<<WGM20) );
  TCCR2B &= ~( (1<<WGM22) );  
  //Timer2 Overflow Interrupt Enable  
  TIMSK2 |= (1<<TOIE2);
  TCNT2 = __TIMER2_MAX - __TIMER2_CNT;                 
  // enable all interrupts
  sei(); 
}
5

DUDE :smiley:

madworm u are the bomb (of course, u2 Coding Badly :wink: )

Tnx for pinpointing the exact problem once again! I wasn't aware of this conflict (hell what do I know, I just found out today that the analog-inputpins run from 0 to 1000-something for a corresponding 0-5V :o)

I maybe come over as the total arduino-noob-idiot, while I've been using this stuff for like three years now (I guess in the wrong fashion then).

Anyways, I will try to reprogram / adapt this into my code! You will probably hear from me again though, but I'll do my best -_-''

I'll definitely post some movie or so of my design online (although this is a NDA-project, I'll let you have a sneak-a-peak madworm, hell you deserve it!)

Tnx Tnx Tnx, kudos all around! ;D

6

Moving the code to timer2 may help but your ISR code will disable interrupts for a relatively long periods (from the servos perspective) so there may still interaction with the servo code because the servo pulses can't be controlled while your ISR is active.

Why not move all your ISR code into loop and add whatever delay you need to get your timing correct.

7

Hey mem! Tnx for you reply!

Is it possible to shift my registers from the loop? Isn't this mandatory in the ISR for the shift registers to function properly (i.e. with a high resolution so there will be no flickering etc.)

8

Putting the LED code into 'loop' works, as long as you don't do much else = loops fast enough. Otherwise it will flicker. Just try it. It should still work if you make a normal function from everything in between /BEGIN/ and /END/ and call that in loop().

To reduce the time spent in the interrupt handler, you could try rewriting it in such a way that it returns after each 'cycle'.

Something like this:

ISR(TIMER2_OVF_vect) { /* Framebuffer interrupt routine */
  TCNT2 = __TIMER2_MAX - __TIMER2_CNT;
  
  /* BEGIN */
  /* initialize once and remember where we are for the next run */
  static byte cycle = 0; 

  digitalWrite(__display_enable,LOW);  // enable display inside ISR
  
    byte led;
    byte row;
    byte red;  // current sinker, on when (0)

    digitalWrite(__spi_latch,LOW);  // prepare the daisy chained 595s to receive data bytes

    for(row = 0; row <= __max_row; row++) {
      red = B11111111;  // off
      for(led = 0; led <= __max_led; led++) {
        if(cycle < brightness_red[row][led]) {
          red &= ~(1<<led);
        }
      }
      spi_transfer(red); // send 1 byte of data
    }
    digitalWrite(__spi_latch,HIGH); // end data transfer for all bytes
    digitalWrite(__display_enable,HIGH);    // disable display outside ISR
 
    /* increase cycle count for next run */
    cycle++;
    
    /* reset cycle to 0 if maxed out */
    if(cycle > __max_brightness) {
      cycle = 0;
    }
    /* END */
}

This code will run about '__max_brightness' times faster and should therefore not block other jobs as much. Tweaking (reduction) of '__TIMER2_CNT' is necessary again (only if you keep the interrupt thing of course).

9

The code can actually run faster in loop than the ISR in the sense that time will no longer be lost saving and restoring registers each time the interrupt handler is called. But as pointed out above your loop code must not have any significant delays in whatever other functionality is handled there. What else does your loop code currently do (perhaps post that code so we can have a look)

10

Hey all!

I tried to quickly built the code for the void loop. As I am still thinking about how to built this all together, this is still WIP of course, but I hope you can get an insight in what I am trying to achieve here :slight_smile:

void loop (void) {
  
  readState(); // function that reads a single variable from Flash
  /* this variable will be used to address the servos and LEDS for their 
  initial state */
 
  // read out of analog inputs for processing
   micValue = analogRead(micPin);
   warmthValue = analogRead(warmthPin);
   lefttopValue = analogRead(ltPin);
   righttopValue = analogRead(rtPin);
   leftbottomvalue = analogRead(lbPin);
   rightbottomValue = analogRead (rbPin);
   
   // trigger functions concerning the inputs
   if (micValue > 450){
      changeLedsCold(); // function for changeing the leds color 
     /* this will be a simple function with a few
      set_led_red(led,value); -rules which are used by the curreny ISR
      */
   }
   
   if (warmthPin > 100){
      changeLedsWarmth(); // function for changeing the leds color
    /* this will be a simple function with a few
      set_led_red(led,value); -rules which are used by the curreny ISR
      */  
   }  
   
   if (lettopValue > 100 && righttopValue > 100) {
      lowerServos(); // function for lowering the servo's values
      /* this will be a function that addresses the values of the servo's
      via a for-loop probably as well as address a few values for the LEDS
      colors through the same set_led_red(led, value); function
      */
   }
   
   if (leftbottomValue > 100 && rightbottomValue > 100) {
      increaseServos(); // function for highering the servo's values
      /* this will be a function that addresses the values of the servo's
      via a for-loop probably, as well as address a few values for the LEDS
      colors through the same set_led_red(led, value); function
      */
   }
   
   centreLoop(); // function that creates a 'rotating' effect with 4 LEDS
   /* addressed through the set_led_red(led, value); function via a for loop */
   
   calculateState(); // function that calculates the current status of the system
   /* the function only uses a set of parametres which are dependant on the state
   of the servo's, LEDS and the centreLoop(); state.
   */

   outputState(); // function that outputs one variable to the serial 9600 port
   /* this funtion will trigger a connection with Flash CS4 to parse a single variable
   which will be used in the Flash application
   */

}

Thanks for your help so far! It is highly appreciated!

11

I had a look at the code you were running to service the LEDs on a logic analyzer and it took well over 3 milliseconds. Not sure exactly how much longer because I removed the wait for SPI and did not include the additional ISR overhead for saving and restoring registers, but even with the 3.5ms delay I measured you could interfere with the interrupt handler in the servo code.

It would be interesting to see how the code performed in the timer2 ISR but don't be surprised if the servos timing was upset. If that is case then one solution would be to change your loop code into a state machine that services the analog inputs and the LED outputs on multiple passes through the loop. Assuming you only need to service the LEDs 50 times per second or so, you could have alternate passes run either your existing loop code or your LED service code.

Have fun!

p.s. here is the test code i ran:

#define __display_enable 4
#define __spi_latch      5  

#define __max_brightness  32
#define __max_row 8
#define __max_led 8

byte brightness_red[__max_row][__max_led];

void setup()
{
     pinMode(__display_enable, OUTPUT);    
     pinMode(__spi_latch  , OUTPUT);    
}

void loop()
{

 byte cycle;

  digitalWrite(__display_enable,LOW);  // enable display 
  
  for(cycle = 0; cycle < __max_brightness; cycle++) {
    byte led;
    byte row;
    byte red;  // current sinker, on when (0)

    digitalWrite(__spi_latch,LOW);  // prepare the daisy chained 595s to receive data bytes

    for(row = 0; row <= __max_row; row++) {
      red = B11111111;  // off
      for(led = 0; led <= __max_led; led++) {
        if(cycle < brightness_red[row][led]) {
          red &= ~(1<<led);
        }
      }
      spi_transfer(red); // send 1 byte of data
    }
    digitalWrite(__spi_latch,HIGH); // end data transfer for all bytes
  }
  digitalWrite(__display_enable,HIGH);    // disable display
}

char spi_transfer(volatile char data)
{  
  SPDR = data;                    // Start the transmission

  while (!(SPSR & (1<<SPIF)))     // Wait for the end of the transmission
  {
      return SPDR;       // RETURN IMMEDIATLY FOR THIS TEST !!!!
  };
  return SPDR;                    // return the received byte
}
12

Tnx for you reply mem.

I've removed all the timer1 functionality and built in the timer2, but still no result though? The only functionality I have at the moment is the addressing of the servos within my LED code (but no LEDS are lit or addressed through a function). The void loop basically is:

servo1.write(80);
servo2.write(160);
servo3.write(70);
servo4.write(20);

I'll have another look at it later today, hopefully it'll be fine though. The LEDS do work in the Timer2! So that's perfec.

Secondly I was wondering what you mean by a 'state machine'. How would this work out in theory?

13

As I mentioned above, if you only need to update the leds 50 times a second or so you can use very simple logic to handle the two high level states

The first high level state (getting input from the sensors) is perhaps the code you posted in reply #9

The other high level state is the code to output values to the LEDs, this is the code you currently are trying in the ISR.

A state machine directs the flow of execution through states that need to be serviced and in your case you may (if your lucky) be able to simply alternate between the two states.

You could start of by creating two functions, perhaps one called something like serviceLEDs that contains the LED code and another function (perhaps named getInputs) that has your input code.

In loop you can try to call each alternately but depending on the timing requirements you may need to add delays, call one more frequently than the other, or break down one or both functions into smaller blocks.

14

So if I'm correct what you mean is this:

I create a main loop with the following (for a simple fader function):

 void loop(){
for (i = 0; i < 32 ; i++){
set_led_red(56,i);
goPWM();
}
for (i=32; i>0; i--) {
set_led_red(56,i);
}

 }

in which goPWM(); is the following function:

void goPWM() {
  byte cycle;

  digitalWrite(__display_enable,LOW);  // enable display
  
  for(cycle = 0; cycle < __max_brightness; cycle++) {
    byte led;
    byte row;
    byte red;  // current sinker, on when (0)

    digitalWrite(__spi_latch,LOW);  // prepare the daisy chained 595s to receive data bytes

    for(row = 0; row <= __max_row; row++) {
      red = B11111111;  // off
      for(led = 0; led <= __max_led; led++) {
        if(cycle < brightness_red[row][led]) {
          red &= ~(1<<led);
        }
      }
      spi_transfer(red); // send 1 byte of data
    }
    digitalWrite(__spi_latch,HIGH); // end data transfer for all bytes
  }
  digitalWrite(__display_enable,HIGH);    // disable display
}

Because I tried this, and it doesn't seem to work :frowning:

(I changed the spi_transfer to this:

char spi_transfer(volatile char data)
{  
  SPDR = data;                    // Start the transmission

  while (!(SPSR & (1<<SPIF)))     // Wait for the end of the transmission
  {
      return SPDR;       // RETURN IMMEDIATLY FOR THIS TEST !!!!
  };
  return SPDR;                    // return the received byte
}

Also I cannot get the Servo's to work (in the other code) :frowning: Even with the leds connected to Timer2.

Geez :frowning: I have the feeling that this is never going to be finished :frowning:

15

Perhaps it is easier to fix a second arduino to serve as the main LED controller and one for the other stuff such as the servos and the input/output stuff?

Is it possible to transfer that much information from one arduino to another? (i.e. change RGB values for leds through one arduino, send them to another and use them within the LED ISR?)

16

I am suggesting you put the LED code (all the code you had in the ISR in reply #2) and put that into one function

Put the code you need in loop (the code you posted in reply#9 ? ) into another function.

In loop call the first function followed by the second function

Change the SPI transfer function back to the way you had it. I only changed it for my timing test because I did not want to take the time to setup SPI.

The feasibility of using two arduinos depends on the data rate (the number of bytes you need to send every second) and this depends on how often the data needs to be updated. I don't think you said what this was.

17

I am really thinking about switching to two arduino's. The level of data that has to be transferred can be reduced I guess. If I have one arduino controlling the LEDS and another controlling the servos and input sounds as an ideal solution.

Although a few problems come into mind: I have to re-solder my whole circuit as well as figure out how I can run 2 arduino's with a single proxy etc etc.

Sounds as a different pain in the ass to me after all :frowning:

I'll have another look at the current code later this morning! Thanks for all the input guys, I hope I will be able to tackle this problem.

18

Before you go to all that trouble, have you done a simple test to see if you can run the LED code in loop.

19

I've been through this 'just use 2 boards for the job' idea, and it is possible, but you'll have to invest some thought into how to organize the communication. Make it error proof, self re-syncing and so on. This may be trivial for a pro, but it can give you a headache nevertheless. Been there, done that.

I should mention that if you run the LED code using the timer interrupts (the code that runs the complete cycle loop), communication using the serial interface still works, but you will have to be careful with the baud rate you chose. The transfer must be completed pretty quickly, so it fits into the time window when the LED code doesn't run. Also just throwing tons of data at the receiving side (with the LEDs) may not work 100% reliable, as you never know if it got through, as the receiver is 'blind and deaf' during a display update. You will definitely need a well defined data packet with start and stop bytes, so the receiver can tell meaningful data from partially transfered junk.

Data reception can be improved a bit by including the 'sei();' instruction inside the ISR() function, but it is a dirty hack. This allows it to be interrupted by other interrupts e.g. those dealing with the serial interface. As far as I remember, this didn't cause many side effects on the display in my RGB gadget. YMMV though.

You can have a look at some code I used to show 'quasi' live video on my device. A serial handler with start/stop byte check and so on is in there too. Just look at the .pde file. The perl scripts are used to send the data from my linux box.

May the force be with you!

[edit]Edit: The state machine approach suggested by mem has the benefit that you can debug it more easily. There is no interrupt that strikes you down from behind and kicks you in the f... at 'apparently' random times. And don't listen to what I say, as "I know nothing!"[/edit]

20

I've been unable to work on my code today unfortunately :frowning:

However I do have two arduino's at the moment and I want to try if I am able to set-up a communication between them (through flash probably, as a communication between the arduino and flash is needed in the first place).

madworm, your post has me a little bit worried about this idea. I had the following idea in mind to solve the whole problem:

arduino1: input readout, servo output and 1 byte output to flash

flash: receives byte, arranges movieclip in order to the byte recieved, sends the byte to the second arduino

arduino2: controlles LEDS through ISR, receives byte and addresses the ISR/LEDS in order to the received byte.

The number of transmissions per second can be reduced to, lets say, 1 or 2, I guess (so my whole project doesn't run AWESOME REAL TIME, but at least it runs right?).

Secondly the communication will only take place when the input changes, so not ALL the time.

Third: with arduino2 only acting as a output (so solely running the right LED-function / colorscheme, will there be syncing problems? Or is this a problem when receiving bytes? So the second arduino doesn't need to send bytes, only receive them. Arduino1 has to be able to send and receive.

Do you think this is possible? Does the ISR of the second arduino interfere with the readout of the byte?

Anyway, I will try the suggestion of mem tomorrow (yesterday, my tryout of the ISR-function within the void loop gave me flickering (50/60ish HZ) LEDS (if you look straight at it, it looks fine, but if you look away and the LEDS are in your eye-corners you see them flicker, it looks ugly), but then I used the wrong spi_transfer.)

I'll keep you posted, and once again, thanks for all your effort!