[SOLVED] Nesting millis() timed events

I will drive 11 solenoid mallets striking Japanese standing bells at random intervals, timed with millis() to have the whole shebang non-blocking (only two in the trial code below). So far, so good.

However, the solenoids need a 100ms delay per strike in between HIGH and LOW, so I sneaked in the nasty blocking delay() again, right through the back door. Doubleplusungood, I guess.

How could I "nest" (if that's the right way to describe it) another millis() based timer to get rid of these delays?

Cheers!

const byte solenoidSmall = 5;
const byte solenoidLarge = 6;

int bellSmallInterval = 3000;
int bellLargeInterval = 4000;
unsigned long timeNowBellSmall = 0;
unsigned long timeNowBellLarge = 0;

void setup()
{
  pinMode(solenoidSmall, OUTPUT);
  pinMode(solenoidLarge, OUTPUT);
  randomSeed(analogRead(5));
}

void loop() {
  strikeBellSmall();
  strikeBellLarge();
}

void strikeBellSmall()
{
  if (millis() - timeNowBellSmall >= bellSmallInterval) // Check if it is time to strike
  {
    timeNowBellSmall = millis(); // Record the current time
    digitalWrite(solenoidSmall, HIGH);
    delay(100);
    digitalWrite(solenoidSmall, LOW);
    bellSmallInterval = random(3000, 17000); // Generate a new random strike interval
  }
}

void strikeBellLarge()
{
  if (millis() - timeNowBellLarge >= bellLargeInterval) // Check if it is time to strike
  {
    timeNowBellLarge = millis(); // Record the current time
    digitalWrite(solenoidLarge, HIGH);
    delay(100);
    digitalWrite(solenoidLarge, LOW);
    bellLargeInterval = random(2000, 16000); // Generate a new random strike interval
  }
}

However, the solenoids need a 100ms delay per strike in between HIGH and LOW, so I sneaked in the nasty blocking delay() again, right through the back door. Doubleplusungood, I guess.

How could I "nest" (if that's the right way to describe it) another millis() based timer to get rid of these delays?

The strike functions could look like this

void strikeBellSmall()
{
  static unsigned long startTime;
  static boolean timing = false;
  
  if (!timing && millis() - timeNowBellSmall >= bellSmallInterval) // Check if it is time to strike
  {
    timeNowBellSmall = millis(); // Record the current time
    digitalWrite(solenoidSmall, HIGH);
    timing = true;
    startTime = millis();
    //delay(100);
    //digitalWrite(solenoidSmall, LOW);
    //bellSmallInterval = random(3000, 17000); // Generate a new random strike interval
  }
  if(timing && millis()- startTime >100)
  {
    timing = false;
    digitalWrite(solenoidSmall, LOW);
    bellSmallInterval = random(3000, 17000);   
  }
}
void strikeBellSmall()
{
  static bool isStarted = false;
  static unsigned long startedTime = 0;

  if (isStarted && millis() - startedTime > 100) {
    digitalWrite(solenoidLarge, LOW);
    isStarted = false;
    return;
  }

  if (millis() - timeNowBellSmall >= bellSmallInterval) // Check if it is time to strike
  {
    timeNowBellSmall = millis(); // Record the current time
    digitalWrite(solenoidSmall, HIGH);
    isStarted = true;
    startedTime = millis();
    bellSmallInterval = random(3000, 17000); // Generate a new random strike interval
  }
}

remove "return;" if you want to allow the solenoidSmall to be turned on even if it is currently on.

I can't get this tested in less time than a forum post timeout so this is untested but, I've done it before.

void strikeBellLarge()
{
  static byte state = 0;  

  switch ( state )
  {
    case 0 :
      if (millis() - timeNowBellLarge >= bellLargeInterval) // Check if it is time to strike
      {
        digitalWrite(solenoidLarge, HIGH);
        timeNowBellLarge = millis(); // Record the current time
        state = 1;
      }
      break;

    case 1 :
      if (millis() - timeNowBellLarge >= 100)
      {
        digitalWrite(solenoidLarge, LOW);
        bellLargeInterval = random(2000, 16000); // Generate a new random strike interval
        timeNowBellLarge = millis(); // Record the current time
        state = 0;
      }
  }
}

This function must run every time void loop() runs.

Attached sketch is a way to get rid of many delays with only 1 timing section.

addasketch_undelay.ino (3.18 KB)

Is there anything useful you need the Arduino to do during the 100ms that it's holding down one solenoid? Does it need to initiate a strike on another bell during that period? If not, then using delay(100) seems appropriate.

Now you just need to read up on arrays so you can use an array of bells instead of naming them large and not-so-large and almost-as-large...

Random strikes with multiple bells, never have two strike at once?

The striking of a bell needn't differentiate large or small bells except, in your case, for the range used in random(). Consider the use of a class for your bells and instantiating as many objects as you have bells. Then just loop through each as you cycle through loop. (Compiles, tested only a bit...)

//defines
#define NUM_BELLS       11          //#     number of bells to control
#define STRIKE_DWELL_TIME   100ul   //mS    mS solenoid strike time

//enumerate the bell sizes (small or large)
enum BellSizes
{
    SMALL_BELL,
    LARGE_BELL
};

//define the pins for each bell (use whatever pins you want)
//pins shown assume Mega2560
const byte pinBell_1 = 5;
const byte pinBell_2 = 6;
const byte pinBell_3 = LED_BUILTIN;     //debug pin to visualize strike time
const byte pinBell_4 = 31;
const byte pinBell_5 = 32;
const byte pinBell_6 = 33;
const byte pinBell_7 = 34;
const byte pinBell_8 = 35;
const byte pinBell_9 = 36;
const byte pinBell_A = 37;
const byte pinBell_B = 38;

//arrays of pins and types for easier initialization
const byte pinBells[] =
{
    pinBell_1, pinBell_2, pinBell_3, 
    pinBell_4, pinBell_5, pinBell_6, 
    pinBell_7, pinBell_8, pinBell_9, 
    pinBell_A, pinBell_B
};
const byte typeBells[] = 
{
    SMALL_BELL, SMALL_BELL, SMALL_BELL, 
    SMALL_BELL, SMALL_BELL, SMALL_BELL,
    LARGE_BELL, LARGE_BELL, LARGE_BELL, 
    LARGE_BELL, LARGE_BELL
}; 

//the Bell class
class Bell
{
    private:    
        enum States
        {
            ST_WAIT,
            ST_STRIKE
        };
        
        //
        bool            _bType;
        byte            _bPin;
        byte            _bState;
        unsigned long   _timeBell;
        unsigned long   _timeStrike;
        unsigned long   _timeDelay;
        //
        unsigned long   _tNow;

    public:
        Bell::Bell()
        {          
        }//Bell

        void Bell::begin( bool type, byte pin, unsigned long strikeTime )
        {
            //initialize the instance
            _bType = type;
            _bPin = pin;
            _timeStrike = strikeTime;
            //
            _timeBell = millis();
            _bState = ST_WAIT;
            //
            if( _bType == SMALL_BELL )
                _timeDelay = random( 3000, 17000 );
            else
                _timeDelay = random( 2000, 16000 );

            pinMode( _bPin, OUTPUT );
             
        }//begin

        //state machine for the instance
        void Bell::spin( void )
        {
            //get time
            _tNow = millis();
            //is it time for a state change?
            if( (_tNow - _timeBell) >= _timeDelay )
            {
                //yes, save this time for next state change
                _timeBell = _tNow;
                //run current state
                switch( _bState )
                {
                    //if waiting, time to strike the bell
                    case    ST_WAIT:
                        //strike the bell
                        digitalWrite( _bPin, HIGH );     
                        //set up the bell-strike time delay           
                        _timeDelay = _timeStrike;
                        //and move to the strike state
                        _bState = ST_STRIKE;
                        
                    break;
        
                    case    ST_STRIKE:
                        //we're striking now; time to release the striker
                        digitalWrite( _bPin, LOW );
                        //set up the time for the next strike event
                        //random range will depend on bell size                
                        if( _bType == SMALL_BELL )
                            _timeDelay = random( 3000, 17000 );
                        else
                            _timeDelay = random( 2000, 16000 );

                        //and now go back and wait
                        _bState = ST_WAIT;
                        
                    break;
                    
                }//switch    
                
            }//if
        
        }//Spin
        
};//class Bell

//declare as many instances as you have bells
Bell 
    allBells[NUM_BELLS];

void setup( void )
{    
    //Serial.begin(115200); //debug
    
    //seed the random generator (make sure A5 floats)
    randomSeed( analogRead( A5 ) );    

    //begin all instances; use the arrays we created to simplify this
    for( int i=0; i<NUM_BELLS; i++ )
        allBells[i].begin( typeBells[i], pinBells[i], STRIKE_DWELL_TIME );
    
}//setup

void loop( void )
{
    static byte
        bellnum = 0;

    //each pass through loop we'll process another bell's state machine
    allBells[bellnum].spin();

    //after processing, move to the next bell 
    bellnum++;
    //until all are done, then cycle back to bell zero
    if( bellnum == NUM_BELLS )
        bellnum = 0;
    
}//loop

Thanks so much for the suggestions; there'll be other code later, reading a particle sensor, so that's why I want to remove the 100ms delays, and some bells could strike together.

The suggested state variable approach from post #3 does work, even though this ain't no true multitasking system. So, this is solved.

I try out the class route also though, seems the thing to do for a larger tsurigane and ōgane forest ; )

C++ Classes allow you to package data and code together in an easier to use form, sometimes incredibly easier.

Thanks, yes; although I'm chuffed to bits that it all works as intended, I'll look into packaging in classes now, particularly as there are many bells, and thus too much repetitive (redundant) code.

Lagom:
Thanks, yes; although I'm chuffed to bits that it all works as intended, I'll look into packaging in classes now, particularly as there are many bells, and thus too much repetitive (redundant) code.

You -could- get around that mainly using arrays since what changes? Pin number, interval, ... ?

As neat as classes are, it's better to cover the C basics before those.