Help improve this code

Other Hardware Robotics
4

s_premkumar999:
Check your Bluetooth modules default baud rate. is it 9600? Check the module using a direct serial interface and and read the received code for each character you sent by the software to verify is there any extra characters coming with each frame.

i have checked and it is 9600 i have successfully connected to the modul and it works fine

robtillaart:
step 1 : remove all the while loops and do the checking in one place

void loop()

{ 
    det = check();
      if (det == 'F')  //if incoming data is a F, move foward
      { 
          desniMotor.run (FORWARD); //right motor moves forward
          desniMotor.setSpeed (velocity); // rotational speed of the motor
          lijeviMotor.run (FORWARD); // -||-
          lijeviMotor.setSpeed (velocity); // -||-
      } 
      else if (det == 'B')  //if incoming data is a B, move back
      {   
          desniMotor.run (BACKWARD); //
          desniMotor.setSpeed (velocity); //
          lijeviMotor.run (BACKWARD); // -||-
          lijeviMotor.setSpeed (velocity); //-||-       
      } 
      else if (det == 'L')  //if incoming data is a L, move wheels left
      {     
          desniMotor.run (FORWARD);
          desniMotor.setSpeed (velocity);
          lijeviMotor.run (BACKWARD);
          lijeviMotor.setSpeed (velocity);               
      } 
      else if (det == 'R')  //if incoming data is a R, move wheels right
      {   
          desniMotor.run (BACKWARD);
          desniMotor.setSpeed (velocity);
          lijeviMotor.run (FORWARD);
          lijeviMotor.setSpeed (velocity);       
      }
      else if (det == 'I')  //if incoming data is a I, turn right forward
      {     
          desniMotor.run (FORWARD);
          desniMotor.setSpeed (velocity);
          lijeviMotor.run (FORWARD);
          lijeviMotor.setSpeed (velocity);       
      } 
      else if (det == 'J')  //if incoming data is a J, turn right back
      {     
          desniMotor.run (BACKWARD);
          desniMotor.setSpeed (velocity);
          lijeviMotor.run (BACKWARD);
          lijeviMotor.setSpeed (velocity);               
      }         
      else if (det == 'G')  //if incoming data is a G, turn left forward
      { 
          desniMotor.run (FORWARD);
          desniMotor.setSpeed (velocity);
          lijeviMotor.run (FORWARD);
          lijeviMotor.setSpeed (velocity);           
      }   
      else if (det == 'H')  //if incoming data is a H, turn left back
      {
          desniMotor.run (BACKWARD);
          desniMotor.setSpeed (velocity);
          lijeviMotor.run (BACKWARD);
          lijeviMotor.setSpeed (velocity);                                                 
      }   
      else if (det == 'S')  //if incoming data is a S, stop
      {
          desniMotor.run (RELEASE);
          lijeviMotor.run (RELEASE);       
      }
}




step 2: replace the motorcalls with one parameterized function

void loop()
{ 
    det = check();
      if (det == 'F')  //if incoming data is a F, move foward
      { 
          action(FORWARD, velocity, FORWARD, velocity);
      } 
      else if (det == 'B')  //if incoming data is a B, move back
      {   
          action(BACKWARD, velocity, BACKWARD, velocity);
      } 
      else if (det == 'L')  //if incoming data is a L, move wheels left
      {     
          action(FORWARD, velocity, BACKWARD, velocity);           
      } 
      else if (det == 'R')  //if incoming data is a R, move wheels right
      {   
          action(BACKWARD, velocity, FORWARD, velocity);   
      }
      else if (det == 'I')  //if incoming data is a I, turn right forward
      {     
          action(FORWARD, velocity, FORWARD, velocity);     
      } 
      else if (det == 'J')  //if incoming data is a J, turn right back
      {     
          action(BACKWARD, velocity, BACKWARD, velocity);             
      }         
      else if (det == 'G')  //if incoming data is a G, turn left forward
      { 
          action(BACKWARD, velocity, BACKWARD, velocity); 
      }   
      else if (det == 'H')  //if incoming data is a H, turn left back
      {
          action(BACKWARD, velocity, BACKWARD, velocity);                                               
      }   
      else if (det == 'S')  //if incoming data is a S, stop
      {
          action(RELEASE, 0, RELEASE, 0);
      }
}

void action( int directionRight, int speedRight, int directionLeft, int speedLeft )
{
          desniMotor.run (directionRight);
          desniMotor.setSpeed (speedRight);
          lijeviMotor.run (directionLeft);
          lijeviMotor.setSpeed (speedLeft );
}



Note not all calls correctly

Step 3: remove unneeded comments and braces

void loop()
{ 
      det = check();
      if (det == 'F')  action(FORWARD, velocity, FORWARD, velocity);
      else if (det == 'B') action(BACKWARD, velocity, BACKWARD, velocity);
      else if (det == 'L') action(FORWARD, velocity, BACKWARD, velocity);           
      else if (det == 'R') action(BACKWARD, velocity, FORWARD, velocity);   
      else if (det == 'I') action(FORWARD, velocity, FORWARD, velocity);     
      else if (det == 'J') action(BACKWARD, velocity, BACKWARD, velocity);             
      else if (det == 'G') action(FORWARD, velocity, BACKWARD, velocity); 
      else if (det == 'H') action(BACKWARD, velocity, FORWARD, velocity);                                               
      else if (det == 'S') action(RELEASE, 0, RELEASE, 0);
}



- you could align the code visually to get nice columns of params (easier to check)
- you could use a "switch case" here instead of the "if else ladder"
- As the parameter velocity is always equal you can refactor one away (but if you want diff speeds -> don't)
- ...
my 2 cents

can you tell me how would i declare action in the scope?