Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Simplify Arduino code #22

Open
raquelalegre opened this issue May 14, 2015 · 3 comments
Open

Simplify Arduino code #22

raquelalegre opened this issue May 14, 2015 · 3 comments

Comments

@raquelalegre
Copy link
Contributor

I've detected a few things that can be simplified in the Arduino code to make it more readable and/or efficient. I thought I could list some of them in this issue as I run into them, we can then replace them together and hope you keep them in mind when you develop in the future.

@raquelalegre
Copy link
Contributor Author

In CS_comm.ino:

As is now:

int CS_checkresponse(String Str_exp) {

    //compare intput string and string in input buffer
    int respflag = 0;
    /*
      Serial.print("This just in ... ");
      Serial.println(CS_inputBuffer);
      Serial.print("expected ");
      Serial.println(Str_exp);
      */
    if (Str_exp == CS_inputBuffer)
    {
        // Serial.println("they match");
        respflag = 1;
    }
    else
    {
        // Serial.println("they dont match");
        respflag = 0;
    }
    return respflag;
}

Could be something like:

/*
* Compares given string and CS input buffer.
* @param input Whatever this argument represents
* @return Strings match
*/
boolean CS_CheckResponse(String response) {

    return response == CS_InputBuffer;
}
  • That notation in the first comment can be understood by tools like Doxygen that'll build automatically documentation for your C code.
  • In several parts of the code, I see functions returning ints and variables used as flags declared as ints. If they can only take 0 or 1 values, they'd better be booleans: makes things more readable, more safe (if you declare them as ints, nothing stops me from giving them somewhere else in the code a -2342 value and break things), and take less space.
  • In general, I see many variables being global, like CS_inputBuffer. It's good practice to avoid them, because there's side effects that can happen and you might not like. Making them local make it easier to debug, as well as frees space once they are not needed. It doesn't mean you can't use them when reasonable, though.
  • Variable/Function naming may need reviewing.
  • Debug needs to be done differently, as we'll discuss in Arduino code logger #21
  • All this function does is compare two strings, might not need a separate function after all.

@raquelalegre
Copy link
Contributor Author

Some variables might benefit from being declared as volatile. The Arduino documentation explains why. Check variables involved in attachInterrupt calls in Stim.ino and CS_Pmark_Test.ino.

@Jimbles
Copy link
Member

Jimbles commented Aug 25, 2015

Started refactoring in libaries in branch Making Library 50ec7c0 and ran into loads of interdependency problems. Code has too many globals and stuff at the moment. So some refactoring would be needed along with simplification. Focusing on development for now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants