Faster Atmega328

[scenario] There was this arcade machine at the mall where anyone could potentially win a cellphone if he had perfect timing in releasing a button so a metal arm could go and push out the prize.

[my solution]
I had my shot using an Arduino project I made that had a timer (in milliseconds), a small menu, and a servo, then once the timer ends, it rotates a servo then releases the button. It really only takes about 6 seconds for the correct button press - to be exact. it was between 5883 Milliseconds and 5863 milliseconds. I was less than an inch close!

[the problem on my solution]
The controls on my project could get the target numbers, but when the timer started, it could not hit the right millisecond timing that i set, its always more than the target. I later realize that my code takes 20 milliseconds to do one whole loop. So if the target time is 5863 milliseconds, it will stop at 5883 milliseconds, Then any target number between 5863 milliseconds to 5883, the timer will still stop at 5883 milliseconds.

[my hypothesis]
i'm using the standard Arduino IDE and the code it outputs. I have read that there is a way of coding that would make Arduino about 20 times faster, but I do not know how to do this yet.

[my question]

Is there somewhere I can read where i can learn this?. I am hoping it would be a step by step learning. Can I use the Arduino IDE for this?

I looked at avr-libc: Modules and this site has been pointed out to be a good resource - but this is seriously complicated and not for a newbie like me.

[summary]
my project works, it just needs to be faster.

Maybe the code can be fixed. Post it, using code tags ("</>" button).

I later realize that my code takes 20 milliseconds to do one whole loop.

That is a very long time even for a 328

What was it doing that took so long ?

ag273n:
I later realize that my code takes 20 milliseconds to do one whole loop.

Then you did probably a whole lot of things wrong, 20 milliseconds are about 320000 instructions.

Yes, the IDE can be used to write very efficient code.

jremington:
Maybe the code can be fixed. Post it, using code tags ("</>" button).

In most, but not all, code, you can speed up your response by eliminating "Arduino" from your code. Arduino's Style Guide states "Efficiency is not paramount; readability is." While most areas will not see 20% gain, the area of Port Manipulation will very likely see this improvement.

Perehama:
the area of Port Manipulation will very likely see this improvement.

If loop() takes 20 millisecs to repeat then there is a lot that can be fixed without needing the complexity of port manipulation.

...R

wow. replies here are quick. I love that. :slight_smile:
I will have to post it in the next 24 hrs, as I have it on another computer.

yes, there's a lot going on the code, its got a simple menu, and its built around the debounce code using two buttons and two potentiometers.

ok here’s my entire code…
the project works like this.
Theres two modes. The default mode is where the user sets the Target millisecond time and where the user can start the timer. In this mode, there’s two potentiometers, one has the coarse adjustment and the other has the fine adjustment.
The second mode is where the angle of rotation of the servo can be set using one of the potentiometers.

There’s two buttons, one button toggles the mode, and the other starts the timer.
So when the timer starts, the servo rotates first, and milliseconds keeps counting and only stops when the target millisecond becomes smaller than the current millisecond count - then thats when the servo rotates again towards the other direction.

the circuitry is using the atmega328 with a 16mhz crystal powered by two 18650 cells in series through an LM338 voltage regulator (no heatsink). There’s a 16X2 LCD with I2C backpack and the case is 3d printed. The servo is an SG-5010. The circuit was soldered like a ratsnest because i was in a hurry and didn’t bother making a good PCB.

const int buttonPin = 11;   //timer start button
const int buttonPin_2 = 10;   //change mode button
const int ledPin = 13;     
const int BuzzerPin = 12;    

int ledState = LOW;      
int ledState_2 = LOW;    
int buttonState;       
int buttonState_2;     
int lastButtonState = LOW; 
int lastButtonState_2 = LOW; 

unsigned long lastDebounceTime = 0;  
unsigned long lastDebounceTime_2 = 0;
unsigned long debounceDelay = 50;    // the debounce time; increase if the output flickers

#include <Wire.h>
#include <LiquidCrystal_I2C.h>
LiquidCrystal_I2C lcd(0x27,16,2);  // 16X2 LCD

unsigned long Target_Millis = 0;
unsigned long Myloop = 0;
int ClearScreenTime = 100;
int Minutes = 0;
long SmallerPot = 0;
unsigned long LargerPot = 0;
boolean StartCountDown = false;
unsigned long StartMillis = 0;
unsigned long StopWatch = 0;

boolean SetServoReach = false; //toggle for servo mode
boolean DefaultMode = true; // toggle for default mode - timer
#include <Servo.h>
Servo myservo;  
int pos = 0;    // Servo Position
int finalpos = 69; // = 90 degrees - default Servo rotation angle
int ServoPin = 9;
int AngleAdjust = 21; //angle adjustment of Servo - due to inaccuracy


void setup() {
  pinMode(buttonPin, INPUT);
  pinMode(buttonPin_2, INPUT);
  pinMode(ledPin, OUTPUT);
  pinMode(BuzzerPin, OUTPUT);

  digitalWrite(ledPin, ledState);
  digitalWrite(BuzzerPin, ledState_2);

  // ---------------------------------------------------
  lcd.init();  lcd.backlight();
  lcd.setCursor(0,0);   lcd.print(" Timed Actuator ");
  lcd.setCursor(0,1);   lcd.print("  Firmware 4.0  ");
  delay(1500);
  lcd.setCursor(0,0);   lcd.print("                ");
  lcd.setCursor(0,1);   lcd.print("                ");
  
  lcd.setCursor(0,0);   lcd.print("T mSec: ");
  myservo.attach(ServoPin); 
  //myservo.write(158); //180 degrees
  myservo.write(0); //0 degrees
  //myservo.write(70); //90 degrees
  Serial.begin(115200);
  BlinkLED();
  Buzzer();
}

void loop() {
  
  int reading = digitalRead(buttonPin);
  int reading_2 = digitalRead(buttonPin_2);

  if (reading != lastButtonState) {lastDebounceTime = millis();}
  if (reading_2 != lastButtonState_2) {lastDebounceTime_2 = millis();}

  if ((millis() - lastDebounceTime) > debounceDelay) {
    if (reading != buttonState) {
      buttonState = reading;
      if (buttonState == HIGH && DefaultMode == true) {
          BlinkLED();
          StartCountDown = true; //start millis timer -----
          ledState = !ledState;
          StartMillis = millis();
          lcd.setCursor(8,1);  lcd.print("          ");
          MoveArmDown();
      }
    }
  }
  if ((millis() - lastDebounceTime_2) > debounceDelay) {
    if (reading_2 != buttonState_2) {
      buttonState_2 = reading_2;
      if (buttonState_2 == HIGH) {
        if (StartCountDown == true){goto SkipClick;} // skip click if timer is running
        
        if (DefaultMode == true && SetServoReach == false){  // set servo rotation angle mode
          DefaultMode = false;
          SetServoReach = true;
          lcd.clear();
          goto ExitSetMode;}

          //last mode - returning to default
        if (DefaultMode == false && SetServoReach == true){ //set back to timer - default mode
          DefaultMode = true; 
          SetServoReach = false;
          lcd.clear();
          lcd.setCursor(0,0);   lcd.print("T mSec: ");
          lcd.setCursor(0,1);   lcd.print("minute: ");
          goto ExitSetMode;}
          
        ExitSetMode:
          Buzzer();// Blink twice
        
      }
    }
  }
SkipClick:
  lastButtonState = reading;
  lastButtonState_2 = reading_2;
  
  if (StartCountDown == true){
    StopWatch = millis() - StartMillis;
    lcd.setCursor(0,1);   lcd.print("C mSec: ");
    lcd.setCursor(8,1);  lcd.print(StopWatch);    }
    
   //stop countdown----------------------------------
   if (Target_Millis < StopWatch){StartCountDown = false;
    MoveArmUp();  
    StopWatch = 0;
    lcd.setCursor(8,1);  lcd.print("---       ");
    delay(1500); 
    lcd.setCursor(8,1);  lcd.print("       ");}
    
    if (StartCountDown == false){
          if (DefaultMode == true){ //default timer mode
              SmallerPot = map(analogRead(A1), 0, 1023, 0, 100);
              LargerPot = map(analogRead(A0), 0, 1023, 0, 100);
              LargerPot = (LargerPot *100);
              Target_Millis = LargerPot + SmallerPot;
              lcd.setCursor(8,0);  lcd.print(Target_Millis);
              
              Minutes = Target_Millis/1000/60;
              if (Minutes < 1){
                lcd.setCursor(0,1);   lcd.print("   Sec: "); lcd.setCursor(9,1);  
                lcd.print(Target_Millis/1000);}
              if (Minutes >= 1){
                lcd.setCursor(0,1);   lcd.print("   Min: "); lcd.setCursor(9,1);  
                lcd.print(Minutes);} 
          }
          if (SetServoReach == true){
              //finalpos = 90 (default angle)
              SmallerPot = map( analogRead(A1), 0, 1000, 0, 180)/10;
              SmallerPot = (SmallerPot*10);
              lcd.setCursor(0,0);   lcd.print("*Rotation Angle* "); 
              lcd.setCursor(0,1);   lcd.print("Degrees: "); 
              lcd.setCursor(9,1);   lcd.print(SmallerPot); 
              finalpos = SmallerPot - AngleAdjust;
              myservo.write(finalpos); 
          }
          
          Myloop ++; //to clear screen of any hanging digits
          if (Myloop> ClearScreenTime){
          Myloop = 0; lcd.setCursor(8,0); lcd.print("          ");  
          lcd.setCursor(8,1); lcd.print("          "); }
    
      }
}

void Buzzer(){
  digitalWrite(BuzzerPin, HIGH);
  delay(100);
  digitalWrite(BuzzerPin, LOW);
  delay(50);
  digitalWrite(BuzzerPin, HIGH);
  delay(100);
  digitalWrite(BuzzerPin, LOW);
}

void BlinkLED(){
    digitalWrite(ledPin, HIGH);
    delay(100); 
    digitalWrite(ledPin, LOW);
    delay(100); 
    digitalWrite(ledPin, HIGH);
    delay(100); 
    digitalWrite(ledPin, LOW);
}

void MoveArmUp(){
    for (pos = 0; pos <= (finalpos); pos += 1) { //move to finalpos (variable, default = 0)
    myservo.write(pos);
    delay(6);
    }
}

void MoveArmDown(){
    for (pos = (finalpos); pos >= 0; pos -= 1) { //move back to 0 degrees
    myservo.write(pos); 
    delay(6); 
    }
}

i understand my code could probably be better. i hope someone can help me optimize it.

I think your button code is a little fishy, how about using a trusted library for that, like bounce2?

Replace all delays by millis code, replace any gotos by proper constructs.

Most of your variables (basically all that are not times or booleans) could use better fitting data types.

It could be that you write too much and too often to the lcd while waiting for the timeout to happen.

ag273n:
ok here's my entire code...

I suggest you reorganize your code so that different parts are in separate functions and loop() is just a series of calls to those functions. That way you can test each part separately and identify which parts are slow. Have a look at Planning and Implementing a Program

Also, by organizing your program into functions there will be no need to use GOTO. At best, GOTO makes it very hard to keep track of what is happening.

Printing to the LCD is slow so you should minimize that.

You have two functions with large delay()s - why are they not using millis() ?

...R

Your use of millis( ) for debounce amounts to nothing more than just a delay. And mixing debounce with the logic for the button function is really confusing.
The whole idea of debounce is to detect if the button is.... bouncing! That implies that you need to read the button to see if it is.... bouncing!
The minimal requirement for debouncing would be to debounce the button release only. In this case the idea would be to delay between button readings long enough so that you are pretty sure that you are not reading bounce on the initial press and accidentally detecting that the button is released when it's not. As I said, this is a minimal test for bounce:

void debounce( byte PIN ) {
    // debounce a button release
    byte db = 0;

    while( !db ) {
        if( digitalRead( PIN ) == HIGH ) {          // button released -- start timing
            delay( 25 );
            if( digitalRead( PIN ) == HIGH ) {      // detect button released
                delay( 25 );
                if( digitalRead( PIN ) == HIGH )    // detect button released
                    db = 1;                         // not bouncing
            }
        }
    }
}

So in your code you would just detect if the button is pressed (LOW) and do your function. When the function is complete, call debounce( ) with that button pin number as an argument.

As stated before, write your code so that gotos are not required.
Don't bury your "}" where you can't tell where your blocks end.

Format your code to make it more readable. Here is a short section of your code the way I like it:

    if( (millis( ) - lastDebounceTime_2) > debounceDelay ) {
        if( reading_2 != buttonState_2 ) {
            buttonState_2 = reading_2;
            if( buttonState_2 == HIGH ) {
                if( StartCountDown == true )
                    goto SkipClick;                                     // skip click if timer is running
                if( DefaultMode == true && SetServoReach == false ) {   // set servo rotation angle mode
                    DefaultMode = false;
                    SetServoReach = true;
                    lcd.clear( );
                    goto ExitSetMode;
                }

                //last mode - returning to default
                if( DefaultMode == false && SetServoReach == true ) {   // set back to timer - default mode
                    DefaultMode = true;
                    SetServoReach = false;
                    lcd.clear( );
                    lcd.setCursor( 0, 0 );   
                    lcd.print( "T mSec: " );
                    lcd.setCursor( 0, 1 );   
                    lcd.print( "minute: " );
                    goto ExitSetMode;
                }

                ExitSetMode:
                Buzzer( );                                      // Blink twice
            }
        }
    }

Don't bury your "}" where you can't tell where your blocks end.

and don't hide the "{" either

Here is the same portion of code formatted the way that I like it

if ( (millis( ) - lastDebounceTime_2) > debounceDelay )
{
  if ( reading_2 != buttonState_2 )
  {
    buttonState_2 = reading_2;
    if ( buttonState_2 == HIGH )
    {
      if ( StartCountDown == true )
        goto SkipClick;                                     // skip click if timer is running
      if ( DefaultMode == true && SetServoReach == false )    // set servo rotation angle mode
      {
        DefaultMode = false;
        SetServoReach = true;
        lcd.clear( );
        goto ExitSetMode;
      }
      //last mode - returning to default
      if ( DefaultMode == false && SetServoReach == true )    // set back to timer - default mode
      {
        DefaultMode = true;
        SetServoReach = false;
        lcd.clear( );
        lcd.setCursor( 0, 0 );
        lcd.print( "T mSec: " );
        lcd.setCursor( 0, 1 );
        lcd.print( "minute: " );
        goto ExitSetMode;
      }
ExitSetMode:
      Buzzer( );                                      // Blink twice
    }
  }
}

Note how all of the "{ and "}" are on their own line and pairs of them are vertically aligned, which makes it easy to see the extent of the code blocks. You can customise Auto Format to do this for you automatically and it is usually the first thing I do when looking at code copied from here.

You can customise Auto Format to do this for you automatically and it is usually the first thing I do when looking at code copied from here.

That sounds intriguing - are you talking about the Arduino IDE and if so, how do you do it?

For me the indent is enough to visually indicate the start of a block. Then you can see at a glance where the block end is. You don't have to differentiate between the two '{', '}'. To me, it also looks a lot cleaner and saves an unnecessary line.
But, to each his own.

That sounds intriguing - are you talking about the Arduino IDE and if so, how do you do it?

Yes, the Arduino IDE

I could tell you how to do it, but I would have to kill you afterwards ...

If you are willing to take the chance then here's how

In the IDE use File/Preferences then click on the link to the folder where preferences.txt is located. In that folder you will find a file named formatter.conf which you can open with an editor such as Notepad++ If the file is not there then find it on your system and copy it to the preferences.txt folder

At the top of the file you will find a URL that provides details on the options available which you can add to formatter.conf

I have added the following lines

# put a space around operators
 pad-oper

# put a space after if/for/while
 pad-header

# Move opening brackets onto new line
 --style=allman --style=bsd --style=break -A1

# delete empty lines in functions
 --delete-empty-lines 

# Insert space padding around operators. 
 --pad-oper

Have fun

I'm guessing from your userid that you're in the UK, so I'll take my chances :smiley:

It even worked - thanks very much, Whitesmith here I come!

boolrules:
For me the indent is enough to visually indicate the start of a block. Then you can see at a glance where the block end is. You don’t have to differentiate between the two ‘{’, ‘}’. To me, it also looks a lot cleaner and saves an unnecessary line.
But, to each his own.

I agree. Its personally harder for me to see the overall outline the more lines there are so my code is condensed that way.

In excel VBA, we had a way to exit IF statements or loop statements using an exit function. this function i didn’t
find an equivalent in Arduino’s reference. that’s why I had to use goto Statements. The goto statements don’t really go too far from where it jumps. it’s only my way of skipping a portion of the code, because if I didn’t add this, the next line would return a true and still execute.

notice how in this set of codes, the second IF statement would return true because that’s what the first IF statement does - toggle the Booleans.

        if (DefaultMode == true && SetServoReach == false){  // set servo rotation angle mode
          DefaultMode = false;
          SetServoReach = true;
          lcd.clear();
          goto ExitSetMode;}

          //last mode - returning to default
        if (DefaultMode == false && SetServoReach == true){ //set back to timer - default mode
          DefaultMode = true; 
          SetServoReach = false;
          lcd.clear();
          lcd.setCursor(0,0);   lcd.print("T mSec: ");
          lcd.setCursor(0,1);   lcd.print("minute: ");
          goto ExitSetMode;}
          
        ExitSetMode:
          Buzzer();// Blink twice

Whandall:
I think your button code is a little fishy, how about using a trusted library for that, like bounce2?

the sketch was built around the debounce sketch from the Arduino examples - notice how my variables are named the same as in the examples. Is there another debounce sketch?..

Whandall:
Most of your variables (basically all that are not times or booleans) could use better fitting data types.

It could be that you write too much and too often to the lcd while waiting for the timeout to happen.

I think these does make a point.
the only part in my code that needs to speed up is when the timer is running, truly a huge part of this is projecting on the screen the running millisecond time. I will change it later today when I can and will report back.
About the Data types though, the ones I declared as unsigned long does need to contain numbers that are insanely huge. we are dealing with milliseconds. Depending on how the potentiometer control is mapped, the target milliseconds could reach minutes.

I was thinking this project could be used on some other applications so I considered that.

boolrules:
Your use of millis( ) for debounce amounts to nothing more than just a delay. And mixing debounce with the logic for the button function is really confusing.
The whole idea of debounce is to detect if the button is.... bouncing! That implies that you need to read the button to see if it is.... bouncing!
The minimal requirement for debouncing would be to debounce the button release only. In this case the idea would be to delay between button readings long enough so that you are pretty sure that you are not reading bounce on the initial press and accidentally detecting that the button is released when it's not. As I said, this is a minimal test for bounce:

void debounce( byte PIN ) {

// debounce a button release
   byte db = 0;

while( !db ) {
       if( digitalRead( PIN ) == HIGH ) {          // button released -- start timing
           delay( 25 );
           if( digitalRead( PIN ) == HIGH ) {      // detect button released
               delay( 25 );
               if( digitalRead( PIN ) == HIGH )    // detect button released
                   db = 1;                         // not bouncing
           }
       }
   }
}



So in your code you would just detect if the button is pressed (LOW) and do your function. When the function is complete, call debounce( ) with that button pin number as an argument.

hey, i'm not sure yet how to use that "Byte" is that only a way to store values as good as either zero or one?..

Also, this function you wrote, if I understand it correctly, so I can assign here the pin number of my button then does it return something?.. i guess i can use this to toggle the booleans that way the next sequence of the codes will catch the change.

ag273n:
hey, i'm not sure yet how to use that "Byte" is that only a way to store values as good as either zero or one?..

Byte is variable type 8-bit unsigned. It stores 0-255. a Bool stores true and false, but is also 8-bits. You can store 1 and 0 in every variable type, but why go out to 16 bits for an int or 32 bits for a long?

In excel VBA, we had a way to exit IF statements or loop statements using an exit function. this function i didn't
find an equivalent in Arduino's reference

Arduino is programmed in C++. Do not confuse the Arduino reference with the full range of C++ statements and functions

Have a look at the break; statement