Code slows down with simple led on/off added

I have a working piece of code that measures force with an AD converter and movement with an encoder strip. Data is transported to a PC by USB to an VB6 application. I need to add a motor control from VB6 (simple motor on motor off) to the arduino UNO. So I started with the basics of that but got stuck.

I have piece of code that switch on and off a led with processing through serial (before I implement it in VB6) . This also works fine and when I change the state in processing the led goes off imediatly and vise versa. So I took this code and pasted it in the force/motion code. The behaviour I get is that the led switch on and off in a random eratic delay. I know this is a basic problem but I am overlooking something very simple I think.

Any guidance would be appriciated.

Code for LED on off

 const int ledPin = 8; // the pin that the LED is attached to
int incomingByte;      // a variable to read incoming serial data into

void setup() {
   // initialize serial communication:
   Serial.begin(9600);
   // initialize the LED pin as an output:
   pinMode(ledPin, OUTPUT);
}

void loop() {
   // see if there's incoming serial data:
   if (Serial.available() > 0) {
     // read the oldest byte in the serial buffer:
     incomingByte = Serial.read();
     // if it's a capital H (ASCII 72), turn on the LED:
     if (incomingByte == 'H') {
       digitalWrite(ledPin, HIGH);
     } 
     // if it's an L (ASCII 76) turn off the LED:
     if (incomingByte == 'L') {
       digitalWrite(ledPin, LOW);
     }
   }
}

Code for force and movement with led on off added.

// Interrupt information optical encoder
// 0 on pin 2
// 1 on pin 3

//default connection arduino NANO/UNO TM7709 24 bit ADC
//DRDY = pin 11 >MOSI
//ADIO = pin 12 >MISO
//SCLK = pin 13 >SCK

//default connection arduino MEGA TM7709 24 bit ADC
//DRDY = pin 21 >MOSI
//ADIO = pin 22 >MISO
//SCLK = pin 28 >SCK

#define TM7710_DRDY        3          
#define TM7710_ADIO        4          
#define TM7710_ADIO_OUT()  DDRB|=1<<4     
#define TM7710_ADIO_IN()   DDRB&=~(1<<4)  
#define Set_TM7710_SCLK()  PORTB|=1<<5
#define Set_TM7710_ADIO()  PORTB|=1<<4
#define Clr_TM7710_SCLK()  PORTB&=~(1<<5)
#define Clr_TM7710_ADIO()  PORTB&=~(1<<4)

unsigned char x[3];
long Result;
float vref=4.57;
int motorValue ;
const int ledPin6 = 6; // the pin that the LED is attached to
 const int ledPin7 = 7; // the pin that the LED is attached to
int led = 8;
int incomingByte;      // a variable to read incoming serial data into

enum PinAssignments 
{
  encoderPinA = 2,   // rigth
  encoderPinB = 3,   // left
};

volatile unsigned int encoderPos = 5000;  // a starting counterposition for the encoder
unsigned int lastReportedPos = 1;   // change management
static boolean rotating=false;      // debounce management

// interrupt service routine vars
boolean A_set = false;              
boolean B_set = false;

void setup() 
{
  DDRB|=1<<5 ;
  Serial.begin(9600);// set baudrate for PC too!
  TM7710_Init();

  pinMode(encoderPinA, INPUT); 
  pinMode(encoderPinB, INPUT); 
  
 // turn on pullup resistors
  digitalWrite(encoderPinA, HIGH);
  digitalWrite(encoderPinB, HIGH);
  

// encoder pin on interrupt 0 (pin 2)
  attachInterrupt(0, doEncoderA, CHANGE);
// encoder pin on interrupt 1 (pin 3)
  attachInterrupt(1, doEncoderB, CHANGE);
  
   pinMode(ledPin6, OUTPUT);
   pinMode(ledPin7, OUTPUT);
   pinMode(led,OUTPUT);

}

void loop()
{  
  /------------------------------------------ problem part ----------------------------------------
   if (Serial.available() > 0) 
   {
     incomingByte = Serial.read();
     if (incomingByte == '1') 
     {
       digitalWrite(ledPin6, HIGH);
       digitalWrite(ledPin7, LOW);
     } 
     if (incomingByte == '0') 
     {
       digitalWrite(ledPin6, LOW);
       digitalWrite(ledPin7, HIGH);
     }
   }
   /------------------------------------------------------------------------------------  
  double volt = Result * vref /16 / 6.912; 

  while((PINB&(1<<TM7710_DRDY))==(1<<TM7710_DRDY)); 
  TM7710_start();
  TM7710_write(0x7F);        
  TM7710_ADIO_IN();          
  for(unsigned char j=0;j<3;j++)
  
  {
    x[j]=TM7710_read();
  }
  
  TM7710_ADIO_OUT();         
  TM7710_stop();
  Result=x[0];
  Result = Result * 256;
  Result = Result + x[1];
  Result = Result * 256;
  Result = Result + x[2];
  Result = Result - 6912000;

  Serial.print ("A,");
  Serial.println (volt,0);
  Serial.print("B,");
  Serial.println(encoderPos, DEC);
  Serial.print("C,");
  Serial.println (motorValue);
  
  
    rotating = true;  // reset the debouncer

  if (lastReportedPos != encoderPos) {
    
    lastReportedPos = encoderPos;
  }
}

void TM7710_start(void)   
{
  Clr_TM7710_ADIO();
  delayMicroseconds(1);
  Clr_TM7710_SCLK();
  delayMicroseconds(1);
}

void TM7710_stop(void)    
{
  Clr_TM7710_ADIO();
  delayMicroseconds(1);
  Set_TM7710_SCLK();
  delayMicroseconds(1);
  Set_TM7710_ADIO();
  delayMicroseconds(1);
}

void TM7710_write(unsigned char dd)
{
  unsigned char i; 

  for(i=8;i>0;i--)
  {
    if(dd&0x80)
      Set_TM7710_ADIO();   
    else
      Clr_TM7710_ADIO();   

    delayMicroseconds(1);
    Set_TM7710_SCLK();       
    delayMicroseconds(1);
    Clr_TM7710_SCLK();      
    dd<<=1;                  
  }
}

unsigned char TM7710_read(void)
{
  unsigned char data=0,i; 

  for(i=0;i<8;i++)
  {
    Set_TM7710_SCLK();                   

    data=data<<1;                                  
    if((PINB&(1<<TM7710_ADIO))==(1<<TM7710_ADIO))  
    {
      data=data+1;
    }
    delayMicroseconds(1);
    Clr_TM7710_SCLK();
    delayMicroseconds(1);
  }
  return data;
}

void TM7710_Init()
{
  TM7710_ADIO_OUT();
  delay(100);
  TM7710_stop();
  TM7710_start();
  TM7710_write(0xBF);        
  TM7710_write(0x20); //ADC preamp Gain=128
  TM7710_stop();      
}

// Interrupt on A changing state
void doEncoderA(){
  // debounce
  if ( rotating ) delay (1);  // wait a little until the bouncing is done

  // Test transition, did things really change? 
  if( digitalRead(encoderPinA) != A_set ) {  // debounce once more
    A_set = !A_set;

    // adjust counter + if A leads B
    if ( A_set && !B_set ) 
      encoderPos += 1;

    rotating = false;  // no more debouncing until loop() hits again
  }
}

// Interrupt on B changing state, same as A above
void doEncoderB(){
  if ( rotating ) delay (1);
  if( digitalRead(encoderPinB) != B_set ) {
    B_set = !B_set;
    //  adjust counter - 1 if B leads A
    if( B_set && !A_set ) 
      encoderPos -= 1;

    rotating = false;
  }
}

Well, for starters, you are declaring Result, which will be initialized to 0, then the first thing you do with it is:

  double volt = Result * vref /16 / 6.912;

So, what is the value of volt, after assigning it 0 * 4.57 / 16 / 6.912 ?

static boolean rotating=false;      // debounce management

global variables dont need to be static.

while((PINB&(1<<TM7710_DRDY))==(1<<TM7710_DRDY));

Is this basicly a delay(x) ? How long do you expect it to delay ?

// interrupt service routine vars
boolean A_set = false;              
boolean B_set = false;

While you don't needed in the case of these booleans ALL var's which can be modified by the ISR should be declared as volatile.

Mark

  pinMode(encoderPinA, INPUT); 
  pinMode(encoderPinB, INPUT); 
  
 // turn on pullup resistors
  digitalWrite(encoderPinA, HIGH);
  digitalWrite(encoderPinB, HIGH);

Out of date and old fashioned -lookup the current pinMode() in the reference section.

Mark

void doEncoderA(){
  // debounce
  if ( rotating ) delay (1);  // wait a little until the bouncing is done

  // Test transition, did things really change? 
  if( digitalRead(encoderPinA) != A_set ) {  // debounce once more
    A_set = !A_set;

    // adjust counter + if A leads B
    if ( A_set && !B_set ) 
      encoderPos += 1;

    rotating = false;  // no more debouncing until loop() hits again
  }
}

You can't use delay inside an ISR! nor should you use Serial.print etc.

Mark

@Mark,
What is "ISR" ?
And Why can you not use a delay() inside one?

Thanks, Jack

Interrupt Service Routine

From the attachInterrupt() function...

Inside the attached function, delay() won't work and the value returned by millis() will not increment. Serial data received while in the function may be lost. You should declare as volatile any variables that you modify within the attached function.

backbone:
The behaviour I get is that the led switch on and off in a random eratic delay.

I think you're saying that the LED is switching to the correct state but there is an unexpected erratic delay between sending the command, and the LED changing state.

The code handling the serial input looks fine so it makes sense that it is switching the LED correctly. I suspect that the problem is that the other code you are executing in loop() is taking a long time to complete so the serial port handling code is only running occasionally.

You could either read through the rest of the code carefully and look for places which could take a long time to execute, or add timing code to measure how long each part of the code takes to complete which would lead you to the hotspots.

Thanks for all responses and thinking with me,
Got me offset more then I expected. :slight_smile:

@Pete": Yes you correct about the behaviour. The LED switches but not directly.

For the others the code for the ADC TM7709 came with it and if it contains wrong declaration I was not aware of. I am not that far in programming to detect such things when looking at the code. For my first purpose of using the ADC the code gave me the results I need. If it is not optimized for speed in combination with other snippets of code I was not aware of until....... The code for the linear encoder and the interrupts I took it somewhere else and pasted it in the ADC code.

I will look with all things said at the code and implement the suggestions.
Hope that cures the problem and brings me to a higher programming level.

Paco

Let you know the progress.

OK, some steps further.
I add the changes some mentioned.
Used other code without delay for the encoder.
I commented out all delays in the code for the ADC.
Same problem.
Then to troubleshoot I took out all code for the ADC!
That cured the LED problem.
So problem is inside the ADC code.

I have no idea where to look more in this ADC code then switching off the delays in microseconds.
I tested the ADC code without the micorseconds delay and the code works as the results I expect for measuring the force.

Well, for starters, you are declaring Result, which will be initialized to 0, then the first thing you do with it is:

Code:

double volt = Result * vref /16 / 6.912;

So, what is the value of volt, after assigning it 0 * 4.57 / 16 / 6.912 ?

@Lar3ry
That would give 0 at that moment, dont shoot me but I am missing your point.

Thanks, Paco

While you don't needed in the case of these booleans ALL var's which can be modified by the ISR should be declared as volatile.

So, what you're saying (correctly, but in a very round-about way) is that not every variable that can be modified in an ISR should be qualified "volatile"?

Never did yet get an answer about this:

Code:
while((PINB&(1<<TM7710_DRDY))==(1<<TM7710_DRDY));
Is this basicly a delay(x) ? How long do you expect it to delay ?

It may or may not be a problem, but what do you think?

Jack, if I look at it with my knowledge nothing happens as long as the value stays the same, if this cause a delay I do not know. You might be correct this causes the delay. I did not made the code for the ADC and most of it for me it is "adacadabra". I will check this evening what happens if I constant change the value of the force if the led will switch quicker. Let you know.

Thanks, Paco

I add the manual of the TM7709 in case this might shine more light to it then it already does.

BTW there two lines of code that say " while > A==A"

This is the Original undressed code that belongs to the ADC.
I undressed it from parts I did not needed.

As far I understand it is 8 bits and the length of the word is send when all 8 bits are received.
If there is a delay in sending when there is no value change I can not figure out this cause the loop to wait.

//TM7709 24bit ADC module arduino sketch
//coldtears electronics

//default connection arduino UNO
//DRDY = 11
//ADIO = 12
//SCLK = 13


#define TM7710_DRDY        3          
#define TM7710_ADIO        4          
#define TM7710_ADIO_OUT()  DDRB|=1<<4     
#define TM7710_ADIO_IN()   DDRB&=~(1<<4)  
#define Set_TM7710_SCLK()  PORTB|=1<<5
#define Set_TM7710_ADIO()  PORTB|=1<<4
#define Clr_TM7710_SCLK()  PORTB&=~(1<<5)
#define Clr_TM7710_ADIO()  PORTB&=~(1<<4)

unsigned char x[3];
long Result;
float vref=4.89;

void TM7710_start(void)   
{
    Clr_TM7710_ADIO();
    delayMicroseconds(1);
    Clr_TM7710_SCLK();
    delayMicroseconds(1);
}

void TM7710_stop(void)    
{
    Clr_TM7710_ADIO();
    delayMicroseconds(1);
    Set_TM7710_SCLK();
    delayMicroseconds(1);
    Set_TM7710_ADIO();
    delayMicroseconds(1);
}

void TM7710_write(unsigned char dd)
{
    unsigned char i; 

    for(i=8;i>0;i--)
    {
        if(dd&0x80)
            Set_TM7710_ADIO();   
        else
            Clr_TM7710_ADIO();   
        
        delayMicroseconds(1);
        Set_TM7710_SCLK();       
        delayMicroseconds(1);
        Clr_TM7710_SCLK();      
        dd<<=1;                  
    }
}

unsigned char TM7710_read(void)
{
    unsigned char data=0,i; 

    for(i=0;i<8;i++)
    {
        Set_TM7710_SCLK();                   
        
        data=data<<1;                                  
        if((PINB&(1<<TM7710_ADIO))==(1<<TM7710_ADIO))  
        {
          data=data+1;
        }
        delayMicroseconds(1);
        Clr_TM7710_SCLK();
         delayMicroseconds(1);
    }
    return data;
}
void TM7710_Init()
{
   TM7710_ADIO_OUT();
   delay(100);
   TM7710_stop();
   TM7710_start();
   TM7710_write(0xBF);        
   TM7710_write(0x20);      //Gain=128
   //TM7710_write(0x00); //Gain=16
   TM7710_stop();      
}
void setup() 
{
DDRB|=1<<5 ;
delay(1000);
Serial.begin(9600);
Serial.println("TM7709 24bit ADC Module");
TM7710_Init();
}
    
void loop()
{  
   while((PINB&(1<<TM7710_DRDY))==(1<<TM7710_DRDY)); 
   TM7710_start();
   TM7710_write(0x7F);        
   TM7710_ADIO_IN();          
   for(unsigned char j=0;j<3;j++)
   {
   x[j]=TM7710_read();
   }
   TM7710_ADIO_OUT();         
   TM7710_stop();
   Result=x[0];
   Result = Result * 256;
   Result = Result + x[1];
   Result = Result * 256;
   Result = Result + x[2];
   Result = Result - 6912000;
   
   double volt = Result * vref /16 / 6912000; 
   printFloat(volt,6); 
   Serial.println(" V  ");

}


 void printFloat(float value, int places) {
 // this is used to cast digits
 int digit;
 float tens = 0.1;
 int tenscount = 0;
 int i;
 float tempfloat = value;

 // if value is negative, set tempfloat to the abs value

   // make sure we round properly. this could use pow from
 //<math.h>, but doesn't seem worth the import
 // if this rounding step isn't here, the value  54.321 prints as

 // calculate rounding term d:   0.5/pow(10,places)
 float d = 0.5;
 if (value < 0)
   d *= -1.0;
 // divide by ten for each decimal place
 for (i = 0; i < places; i++)
   d/= 10.0;
 // this small addition, combined with truncation will round our

 tempfloat +=  d;

 if (value < 0)
   tempfloat *= -1.0;
 while ((tens * 10.0) <= tempfloat) {
   tens *= 10.0;
   tenscount += 1;
 }

 // write out the negative if needed
 if (value < 0)
   Serial.print('-');

 if (tenscount == 0)
   Serial.print(0, DEC);

 for (i=0; i< tenscount; i++) {
   digit = (int) (tempfloat/tens);
   Serial.print(digit, DEC);
   tempfloat = tempfloat - ((float)digit * tens);
   tens /= 10.0;
 }

 // if no places after decimal, stop now and return
 if (places <= 0)
   return;

 // otherwise, write the point and continue on
 Serial.print('.');

 for (i = 0; i < places; i++) {
   tempfloat *= 10.0;
   digit = (int) tempfloat;
   Serial.print(digit,DEC);
   // once written, subtract off that digit
   tempfloat = tempfloat - (float) digit;
 }
 
  }

TM7709EN_V1.0.pdf (470 KB)

I did a test and found this behaviour.

When I do not change the force measurement of the ADC and perform 4 times in a row a quick on/off led activation there is nothing happening. Then after 3 seconds the led will go on/off four times. If I do the same for 10 times the same behaviour appears. So it looks the on/off switching is stored and then perfomed.

Anyone with some advice on this?

Thanks in advance, Paco