1
\$\begingroup\$

I want to know if what I have coded is good enough or if there is any simpler/faster/better/less size taking substitute.

I am trying to take input from push buttons and I also want to remove repeated input so I coded like it and saved in a variable key. I used internal pullup to eradicated random input from digital pins. I am in pre-graduation in computer applications and a beginner in electronics.

byte key=0; unsigned int st=millis(),ct=millis(); //st-starttime , ct-currenttime void setup() { delay(5000); //start prog after 5 sec pinMode(7, INPUT); pinMode(8, INPUT); pinMode(9, INPUT); byte i; for(i=7;i<=9;i++) digitalWrite(i, HIGH); //7,8,9 pins HIGH for internal pullup Serial.begin(9600); } void loop() { byte i; for(i=7;i<=9;i++) if(checkswitch(i)==1) //check which switch was last pressed { key=i; st=millis(); while(st>65000) //controlling limits of unsigned int st=st-65000; } ct=millis(); while(ct>65000) ct=ct-65000; //controlling limits of unsigned int if(ct>st+200) //Triggers only if 200 ms passed { //to not fill the Serial input if(key!=0) { Serial.println(key); key=0; //clear saved key } } } byte checkswitch(byte n) //Check button pressed with internal pullup { byte p=0; if(digitalRead(n)==LOW) p=1; //my best algo for if-else statement return p; } 
\$\endgroup\$

    2 Answers 2

    2
    \$\begingroup\$

    I don't know the Arduino language but I've recently been learning, from what I can gather on the website unsigned ints will "roll-back" to zero if they reach their maximum (which will be reached in a huge "fifty days" so no need to worry about that.

    You can clean up your syntax for your statements by putting the braces at the end of the statement (on the same line), also remove the byte i from the function visibility into only the loop by declaring it in the for.

    I also noticed that your function checkswitch is only used for boolean operations yet it's returning a byte - this can be fixed by returning the result of the comparison of the if you used directly.

    Here is my suggestion:

    byte key=0; unsigned int st=millis(),ct=millis(); //st-starttime , ct-currenttime void setup() { delay(5000); //start prog after 5 sec pinMode(7, INPUT); pinMode(8, INPUT); pinMode(9, INPUT); for(byte i=7;i<=9;i++) digitalWrite(i, HIGH); //7,8,9 pins HIGH for internal pullup Serial.begin(9600); } void loop() { for(byte i=7;i<=9;i++) if(isHigh(i)) { //check which switch was last pressed key=i; st=millis(); } if(ct>st+200) { //triggers only if 200 ms passed to not fill the Serial input if(key!=0) { Serial.println(key); key=0; //clear saved key } } } boolean isHigh(byte n) { //Check button pressed with internal pullup return digitalRead(n)==LOW; } 
    \$\endgroup\$
    1
    • \$\begingroup\$You can combine if(ct>st+200) and if(key!=0) into one condition like if(ct > st + 200 && key). That will make it even smaller. Other than it this looks good to me.\$\endgroup\$CommentedJul 21, 2013 at 10:52
    0
    \$\begingroup\$

    For Arduino there is a nice Button library that is already de-bounced, that is very simple to implement, with desired methods already.

    Where it would be argumentative to compare it versus your code against the different aspects of simpler/faster/better/less, in that you have successfully implemented the minimum of what you need. Being there are different criteria to balance ones opinion. Where as I would believe it to be much simpler to implement, read and nearly same size of code as your, but with additional features ready to use.

    \$\endgroup\$

      Start asking to get answers

      Find the answer to your question by asking.

      Ask question

      Explore related questions

      See similar questions with these tags.