Millis timing

I have a program which sends a bunch of integers from a Tx module to a Rx module.

Once sent from the Tx I have a LED which blinks on and off to indicate the message has been sent.

The Rx units receives the message and "does stuff" and sends back the integers received.

When the TX unit confirms that the intergers received are the same as those originally sent

the LED on the Tx module stops blinking and indicates a "solid" LED colour to confirm.

The attached sketch I have composed using my limited knowledge of Arduino and its coding.

The sketch works fine and does exactly what I want it to do.

I would appreciate some constructive critique and advice please on how I could improve

and maybe reduce the amount of code I have used.

Thanks

Farticus:
The attached sketch I have composed using my limited knowledge of Arduino and its coding.

Unfortunately the attaching didn't work. You can edit your post to correct that. If the code is small enough the forum prefers that you post the code inside code tags - see the pinned thread 'How to use this forum' has instructions on how to use code tags.

OOPS

      const byte greenLED = 11;
      const byte redLED = 12;
      unsigned long int a =200;
      unsigned long int b =200;
      unsigned long int c = 0;
      unsigned long int d = 0;
  
  
      const unsigned long greenLEDinterval = a;
      const unsigned long redLEDinterval = b;
      unsigned long greenLEDtimer;
      unsigned long redLEDtimer;

void setup () 
{

      Serial.begin (9600);
      pinMode (greenLED, OUTPUT);
      pinMode (redLED, OUTPUT);
      greenLEDtimer = millis ();
      redLEDtimer = millis ();
}  

void toggleGreenLED ()
{
      if (digitalRead (greenLED) == LOW)
         digitalWrite (greenLED, HIGH);
      else
         digitalWrite (greenLED, LOW);

  
      greenLEDtimer = millis ();  
}  

void toggleRedLED ()
{
     if (digitalRead (redLED) == LOW)
        digitalWrite (redLED, HIGH);
     else
        digitalWrite (redLED, LOW);

  
     redLEDtimer = millis ();  
}  

void loop ()
{
      c++;
      d++;
     
      Serial.println (a);
      if ((c<1500)&& ((millis () - greenLEDtimer) >= a))
      toggleGreenLED ();
      if (c>1500){digitalWrite(greenLED,HIGH);}

    
      if ((d<3000) && ((millis () - redLEDtimer) >=b)) 
      toggleRedLED ();
      if (d>3000){digitalWrite(redLED,HIGH);}

      if (c>3000){(c = 0);}
      if (d>6000){(d = 0);}


}

Please give your variables decent names (a, b, c and d mean nothing). The variables a and b are also reduncdant; this works just as well

const unsigned long greenLEDinterval = 200;

The below is a self contained function to blink the green led; it handles all the timing inside the function

void toggleGreenLED ()
{
  static unsigned long lastUpdateTime = 0;

  if (millis() - lastUpdataTime >= greenLEDinterval)
  {
    lastUpdateTime = millis();

    if (digitalRead (greenLED) == LOW)
      digitalWrite (greenLED, HIGH);
    else
      digitalWrite (greenLED, LOW);
  }
}

Only disadvantage is that the first duration might not be the specified time; this can be polished if needed.

You can use it like

void loop ()
{
  static bool waitForReplay = false;

  if(waitForReply == false)
  {
    // send data
    ...
    ...

    waitForReply = true;
  }
  else
  {
    toggleGreenLed();

    // receive data a byte at a time (non-blocking)
    ...
    ...

    // if data complete
    waitForReply = false;
  }
}

To receive the data in a non-blocking way, check Robin's [Serial Input Basics - updated - Introductory Tutorials - Arduino Forum]serial input basics thread[/url] to get ideas.

Thank you for your input.

Appreciated :slight_smile: