Suggestions for toggling sketch

Looking for suggestions/additions.
Code to show how toggling multiple LEDs can be achieved.

/*
  Demonstration code to toggle a LED a given number of times.
  A 'class' could be used; however, this example exposes the code to the user.

  ToggleLEDs.ino
  LarryD
  Version YY/MM/DD
  1.00    17/08/07   Running code
  1.01    17/08/08   added comments and examples
  1.02    17/08/09   minor code changes

  Note: in this example, toggling is accomplished with:
  digitalWrite(pinNumber, !digitalRead(pinNumber));
  It should be pointed out using a state variable is faster than digitalRead.
  Also, digitalRead may not be portable to other architectures.

  A call to the serviceLED() function is placed in loop().
  Example: serviceLED(heartLED, heartMillis, heartDelay, heartToggleCount);
  Each time through loop() the call checks to see if the LED/pin should be toggled.

  Example: You have an FSM, you want to flash a LED from 'normally OFF' to ON to OFF
  You set the count parameter to 2 (i.e. 2 toggles), you place the following in code:
  In the FSM >>>--->    test2ToggleCount = 2;                //load the toggle count
           'delayed'    test2Millis = millis();              //for delayed action
    OR   'immediate'    test2Millis = millis() + test2Delay; //for immediate action
  In loop()  >>>--->    serviceLED(test2Led, test2Millis, test2Delay, test2ToggleCount);

*/

//****************
#define OFF      HIGH //level it takes to turn a LED OFF
#define ON       LOW  //level it takes to turn a LED ON
#define Pressed  LOW  //level on a pin when a switch is pushed
#define Released HIGH //level on a pin when a switch is not pushed

//******************
const byte heartLED             = 13;
unsigned long heartMillis;
const unsigned long heartDelay  = 500UL;
unsigned int heartToggleCount   = 0xffff; //constant toggling

//******************
const byte test1Led             = 12;
unsigned long test1Millis;
const unsigned long test1Delay  = 100UL;
unsigned int test1ToggleCount   = 1000;   //toggle 1000 times

//******************
const byte test2Led             = 11;
unsigned long test2Millis;
const unsigned long test2Delay  = 100UL;
unsigned int test2ToggleCount   = 0;      //start out with no toggling

//******************
const byte test3Led             = 10;
unsigned long test3Millis;
const unsigned long test3Delay  = 1000UL;
unsigned int test3ToggleCount   = 0;      //start out with no toggling

//******************
const byte stopSwitch           = 8;      //LOW will stop test1Led toggling


//                           s e t u p ( )
//**********************************************************************
void setup()
{
  pinMode(stopSwitch, INPUT_PULLUP);

  pinMode(heartLED, OUTPUT);
  digitalWrite(heartLED, OFF);

  pinMode(test1Led, OUTPUT);
  digitalWrite(test1Led, OFF);

  pinMode(test2Led, OUTPUT);
  digitalWrite(test2Led, OFF);

  pinMode(test3Led, OUTPUT);
  digitalWrite(test3Led, OFF);

  //immediate action
  test2ToggleCount = 2;                //load the toggle count
  test2Millis = millis() + test2Delay; //immediate action

  //delayed action
  test3ToggleCount = 2;                //load the toggle count
  test3Millis = millis();              //delayed action

} //END                      s e t u p ( )


//                            l o o p ( )
//**********************************************************************
void loop()
{
  //**************************
  //constant toggling
  serviceLED(heartLED, heartMillis, heartDelay, heartToggleCount);
  //could also use:  serviceLED(heartLED, heartMillis, 100ul, heartToggleCount);

  //**************************
  //finite toggling, 1000 times
  serviceLED(test1Led, test1Millis, test1Delay, test1ToggleCount);

  //**************************
  //finite toggling with immediate action
  serviceLED(test2Led, test2Millis, test2Delay, test2ToggleCount);
  //ex: initialize:  xxxToggleCount and xxxMillis in FSM
  //then let a call in loop() handle the toggling

  //**************************
  //finite toggling with delayed action
  serviceLED(test3Led, test3Millis, test3Delay, test3ToggleCount);


  //**************************
  //stop test1Led toggling on switch press
  if (digitalRead(stopSwitch) == Pressed)
  {
    //stop toggling on test1Led
    test1ToggleCount = 0;
    digitalWrite(test1Led, OFF);
  }

} //END                       l o o p ( )



//======================================================================
//                         F U N C T I O N S
//======================================================================


//                      s e r v i c e L E D ( )
//**********************************************************************
//Toggles a LED/pin a given number of times.
//Starting with a even number in xxxToggleCount leaves the LED in the same
//state as it started.
void serviceLED(const byte pinNumber, unsigned long &ledMillis,
                unsigned long ledDelay, unsigned int &toggleCount)
{
  //**************************
  if (toggleCount == 0)
  {
    //nothing to be done here
    return;
  }

  //**************************
  if (millis() - ledMillis >= ledDelay)
  {
    ledMillis = millis();

    //**************************
    if (toggleCount == 0xffff)
    {
      //0xffff toggles the LED forever
      digitalWrite(pinNumber, !digitalRead(pinNumber));

      //no need to go any further
      return;
    }

    //**************************
      toggleCount--;
      digitalWrite(pinNumber, !digitalRead(pinNumber));


  } //END  if (millis() - ledMillis >= ledDelay)

} //END                  s e r v i c e L E D ( )

//**********************************************************************


//======================================================================
//                        E N D  O F  C O D E
//======================================================================

EDIT:
Updated code

.

There was some discussion on that technique of reading the output pin state in the Arduino developers mailing list:
https://groups.google.com/a/arduino.cc/forum/#!msg/developers/PbkMV59u2XQ/HmZZ0kiABwAJ
One of the more useful replies was from WestfW:

As one of my students pointed out,
digitalWrite (13,!digitalRead(13));
works very nicely to change the state of an LED.

  1. In fact, this may not work on other architectures, or depending on port settings. PICs are infamous reading the actual value of the pin, rather than the value that was last written. 8051s usually have “open drain” outputs that may read “randomly” when set to “HIGH”...
  2. calling digitalRead() is much slower than accessing a state variable. (usually not relevant, but… irrelevant cpu cycle efficiency vs irrelevant memory efficiency?)
  3. in some sort of vague, hand-wavy way, I think it’s important that students grasp that the LED state cn shadow a data variable state, rather than just being manipulable as an “LED thing.”

BillW/WestfW

You have quite a memory!

Point taken, a note in the comments was added.