Problem with Delay

Hi all, I was wondering if you could give me a hand. I'm trying to write a program here. I want input 1 to trigger output 2 then when done shut off output 2 and then delay 5 seconds. Then do the same thing with input 2 and output 1. I have included the program I have already wrote. The only issue is the delay runs every time the loop comes around. I want the delay to only happen the once when else happens. If anyone can help that would be great. Thank you.

const int Radio1COSPin = 2;
const int Radio1PTTPin = 3;
const int Radio2PTTPin = 4;
const int Radio2COSPin = 5;
int Radio1COSState = LOW;
int Radio2COSState = LOW;
unsigned long interval = 5000;

void setup() 
{
    pinMode(Radio1PTTPin, OUTPUT);
    pinMode(Radio1COSPin, INPUT);
    pinMode(Radio2PTTPin, OUTPUT);
    pinMode(Radio2COSPin, INPUT);
}

void loop()
{
    {
    Radio1COSState = digitalRead(Radio1COSPin);
        if (Radio1COSState == HIGH)
        {
        digitalWrite(Radio2PTTPin, HIGH);
        }
        else 
        {
        digitalWrite(Radio2PTTPin, LOW);
        delay(interval);
        }
    }
    {
    Radio2COSState = digitalRead(Radio2COSPin);
        if (Radio2COSState == HIGH)
        {
        digitalWrite(Radio1PTTPin, LOW);
        }
        else 
        {
        digitalWrite(Radio1PTTPin, HIGH);
        delay(interval);
        }
    }
}

where is input 1 and why (from a naming perspective) it drives output 2 and vice versa? seems weird

where is time management in your code ?

Ok so input 1 in Radio 1 COS it drives a second radio Radio 2 PTT. What is happening when radio 1 receives it makes radio 2 transmit and vice versa. The issue is when a radio is done transmitting it has a tail that I don’t want to trigger the other radio. So I want it to ignore the cos for a period of time after PTT. Hopefully this isn’t too confusing.

I don't think I understand what you want, or whether your program logic is appropriate. Can you write down the requirement in this style

check pin1
  if pin1 is HIGH
     make OUTpin2 HIGH
check if something is done? (what is the something)
  if something is done
    make OUTpin2 LOW
    wait 5 seconds
check pin2
  if pin2 is HIGH
     make OUTpin1 HIGH
check if something is done? (what is the something)
  if something is done
    make OUTpin1 LOW
    wait 5 seconds
repeat

By the way I am assuming that what I have written is incorrect, so please post the correct version.

...R

are you saying you want something like this?

|500x285

if you want to do that you need to remember last time your output went low and before putting it high again check if the 5 seconds are gone

J-M-L you have exactly what I want. The way I have it now is every time the loop runs it delays 5 seconds. I only want it to delay 5 seconds if it is after a PTT event like you have illustrated

look at the Blink Without Delay example - you'll see how to record the time of an event and use that to decide when it's OK to trigger the next one. give it a try and post your code, we'll help fine tune if needed

J-M-L I looked up your example and I tried to wrap my head around how to use it but am having trouble trying to apply it to my program. The issue with your example is its great for a blink circuit. The issue is my triggers can be continuous. So if I have my COS input held for any amount of time it will update the previous millis and when it re-evaluates the if it will shut it self down based on the previous millis and not based on the input as I would like. Maybe I'm misunderstanding. But here is what I have tried to do with your example, the only issue is I don't know where to put the comman to update the previous millis. If I make it update the previous millis on the if statement when the cos is triggered it will update the previous millis and if it stays high the next time around the previous millis will disable the ptt. If I update the previous millis on the else command it will work the first time but then once the cos goes low it will continuously update the previous millis and never trigger again. Sorry if I misunderstand the code, I'm pretty new at this. Thanks again for your help.

const int Radio1COSPin = 2;
const int Radio1PTTPin = 3;
const int Radio2PTTPin = 4;
const int Radio2COSPin = 5;
int Radio1COSState = LOW;
int Radio2COSState = LOW;
unsigned long previousMillis = 0;
const long interval = 1000; 

void setup() 
{
    pinMode(Radio1PTTPin, OUTPUT);
    pinMode(Radio1COSPin, INPUT);
    pinMode(Radio2PTTPin, OUTPUT);
    pinMode(Radio2COSPin, INPUT);
}

void loop()
{
    {
    Radio1COSState = digitalRead(Radio1COSPin);
        if ((Radio1COSState == HIGH) && (millis() - previousMillis >= interval))
        {
        digitalWrite(Radio2PTTPin, HIGH);
        }
        else 
        {
        digitalWrite(Radio2PTTPin, LOW);
        }
    }
    {
    Radio2COSState = digitalRead(Radio2COSPin);
        if ((Radio2COSState == HIGH) && (millis() - previousMillis >= interval))
        {
        digitalWrite(Radio1PTTPin, LOW);
        }
        else 
        {
        digitalWrite(Radio1PTTPin, HIGH);
        }
    }
}

the only issue is I don't know where to put the comman to update the previous millis.

The first thing I would do is to use a more meaningful name for the variable, perhaps startMillis. A simple change like that can help visualise what is happening, when it should happen and why.

What is it that starts the timing ? That is where you need to copy millis() to startMillis.

As to

So if I have my COS input held for any amount of time it will update the previous millis

Only if you save the start time when the COS input [u]is[/u] held rather than when it [u]becomes[/u] held. The StateChangeDetection example will show you how to detect the change of state from not held to held and to ignore the current state unless it has just become significant.

I have not tested the following code, but this should give you ideas

const byte inputPin  = 2;
const byte outputPin = 3;

unsigned long lastHighToLowTransitionTime = 0;
const unsigned long waitingTime = 5000ul;
boolean waitMode;
byte previousInputState;

void setup()
{
  waitMode = false;

  pinMode(inputPin, INPUT);
  previousInputState = digitalRead(inputPin);

  pinMode(outputPin, OUTPUT);
  digitalWrite(outputPin, LOW);
}

void loop()
{
  byte inputState = digitalRead(inputPin);

  if (waitMode) // if we are in the waiting mode,
    if (millis() - lastHighToLowTransitionTime >= waitingTime) // check if delay has elapsed
      waitMode = false; // in which case mark the end of the waiting period


  if ((inputState == HIGH) && (previousInputState == LOW)) {
    // LOW TO HIGH TRANSITION
    if (!waitMode) { // if we are not in the "NO UPDATE" mode
      digitalWrite(outputPin, HIGH); ; // we change the output status
    }
  } else if ((inputState == LOW) && (previousInputState == HIGH)) {
    // HIGH TO LOW TRANSITION
    digitalWrite(outputPin, LOW); ; // we change the output status
    waitMode = true; // we start the waiting period
    lastHighToLowTransitionTime = millis(); // record when we had the HIGH to LOW transition
  }
  previousInputState = inputState;
}

Ok. This is what I cam up with. I kind of took bits from everyone suggestions. I have a feeling I'm going to be way out to lunch here because I'm a newbie. But I'm trying to learn. See if you can find a flaw with this program. I wish there was a virtual machine I could place the program in and run it in a virtual environment. So basically what has to happen is this...

If the south radio COS input goes high I need the north radio ptt to go low. But when the south radio COS goes low, the north ptt goes high and ignore the North COS input for 5 seconds.

Then do the exact same thing the other way around

If the North radio COS input goes high, the south ptt goes low then when the north cos goes low, south ptt goes high and ignores the south cos for 5 seconds.

Hopefully that isn't to confusing to understand. Here is what I got so far, thanks for looking it over.

const int NorthCOSPin = 2;
const int NorthPTTPin = 3;
const int SouthPTTPin = 4;
const int SouthCOSPin = 5;
int NorthCOSState = LOW;
int SouthCOSState = LOW;
int NorthLastState = LOW;
int SouthLastState = LOW;
int NorthStop = false;
int SouthStop = false;
unsigned long Northtimer = 0;
unsigned long Southtimer = 0;

void setup() 
{
    pinMode(NorthPTTPin, OUTPUT);
    pinMode(NorthCOSPin, INPUT);
    pinMode(SouthPTTPin, OUTPUT);
    pinMode(SouthCOSPin, INPUT);
}

void loop()
{
    {
       if ((SouthStop == true) && (millis() - Southtimer >= 5000))
       {
        digitalWrite(SouthStop, false);
       }
    }
    {
       if ((NorthStop == true) && (millis() - Northtimer >= 5000))
       {
        digitalWrite(NorthStop, false);
       }
    }
    {
    NorthCOSState = digitalRead(NorthCOSPin);
        if ((NorthCOSState == HIGH) && (NorthStop == false))
        {
        digitalWrite(NorthLastState, LOW);
        digitalWrite(SouthPTTPin, HIGH);
        }
        else 
        {
        digitalWrite(SouthPTTPin, LOW);
            {
              if ((NorthCOSState == LOW) && (NorthLastState == LOW))
              Southtimer = millis();
              digitalWrite(SouthStop, true);
              digitalWrite(NorthLastState, HIGH);
            }
        }
    }
    {
    SouthCOSState = digitalRead(SouthCOSPin);
        if ((SouthCOSState == HIGH) && (SouthStop == false))
        {
        digitalWrite(SouthLastState, LOW);
        digitalWrite(NorthPTTPin, LOW);
        }
        else 
        {
        digitalWrite(NorthPTTPin, HIGH);
            {
              if ((SouthCOSState == LOW) && (SouthLastState == LOW))
              Northtimer = millis();
              digitalWrite(NorthStop, true);
              digitalWrite(SouthLastState, HIGH);
            }
        }
    }
}

Corymacs: So basically what has to happen is this...

If the south radio COS input goes high I need the north radio ptt to go low. But when the south radio COS goes low, the north ptt goes high and ignore the North COS input for 5 seconds.

Then do the exact same thing the other way around

If the North radio COS input goes high, the south ptt goes low then when the north cos goes low, south ptt goes high and ignores the south cos for 5 seconds.

Maybe this pseudo code will do what you want

start with southEnabled = true

loop()
   read the southCOS
   read the northCOS
   if southEnabled == true && southCOS == HIGH
      make northPTT LOW

   if the northPTT == LOW && southCOS == LOW
      make northPTT HIGH
      waitStart = millis()

   if southEnabled == false &&  northCOS == HIGH
      make southPTT LOW

   if the southPTT == LOW && northCOS == LOW
      make southPTT HIGH
      waitStart = millis()
      
   if millis() - waitStart >= 5000
      southEnabled = ! southEnabled // toggle

...R

Robin2: Maybe this pseudo code will do what you want

what's it gonna take to get you newbies to use code tags?

;)

BulldogLowell: what's it gonna take to get you newbies to use code tags?

;)

Not a lot.

...R

So I tested the code I have now. The NorthCOS toggles the SouthPTT but the SouthCOS doesn't toggle the NorthPTT. Maybe someone can look at my code and see what I have done wrong?

const int NorthCOSPin = 2;
const int NorthPTTPin = 3;
const int SouthPTTPin = 4;
const int SouthCOSPin = 5;
int NorthCOSState = LOW;
int SouthCOSState = LOW;
int NorthLastState = LOW;
int SouthLastState = LOW;
int NorthStop = false;
int SouthStop = false;
unsigned long Northtimer = 0;
unsigned long Southtimer = 0;

void setup() 
{
    pinMode(NorthPTTPin, OUTPUT);
    pinMode(NorthCOSPin, INPUT);
    pinMode(SouthPTTPin, OUTPUT);
    pinMode(SouthCOSPin, INPUT);
}

void loop()
{
    {
       if ((SouthStop == true) && (millis() - Southtimer >= 5000))
       {
       SouthStop = false;
       }
    }
    {
       if ((NorthStop == true) && (millis() - Northtimer >= 5000))
       {
       NorthStop = false;
       }
    }
    {
    NorthCOSState = digitalRead(NorthCOSPin);
        if ((NorthCOSState == HIGH) && (NorthStop == false))
        {
        NorthLastState = LOW;
        digitalWrite(SouthPTTPin, HIGH);
        }
        else 
        {
        digitalWrite(SouthPTTPin, LOW);
        }
    }
    {
    NorthCOSState = digitalRead(NorthCOSPin);
        if ((NorthCOSState == LOW) && (NorthLastState == LOW))
        Southtimer = millis();
        SouthStop = true;
        NorthLastState = HIGH;
    }
    {
    SouthCOSState = digitalRead(SouthCOSPin);
        if ((SouthCOSState == HIGH) && (SouthStop == false))
        {
        SouthLastState = LOW;
        digitalWrite(NorthPTTPin, LOW);
        }
        else 
        {
        digitalWrite(NorthPTTPin, HIGH);
        }
    }
    {
    SouthCOSState = digitalRead(SouthCOSPin);
        if ((SouthCOSState == LOW) && (SouthLastState == LOW))
        Northtimer = millis();
        NorthStop = true;
        SouthLastState = HIGH;
    }
}

Corymacs: So I tested the code I have now.

Did you read Reply #11 ?

...R

Ok Robin2, I modified your program a little bit. I would like to have separate south and north Enables. Let me know what you think. No two issues I have and I'm hoping you can shed some light on them. One is that where do I put the ";" after toggle is the last code? I put it after toggle but I keep getting the error status 1 expected ';' before '}'. The other is I'm trying to understand how the toggle works? I see it has SouthEnabled = ! SouthEnabled // toggle. How does that make it go true and false? Sorry for the newbie questions. I'm just trying to understand how it works so I can create a similar code by myself the next time. I have copied the code I have so far with the ';' in the location that gives me the error...

const int NorthCOSPin = 2;
const int NorthPTTPin = 3;
const int SouthPTTPin = 4;
const int SouthCOSPin = 5;
int NorthCOSState = LOW;
int SouthCOSState = LOW;
int SouthEnabled = true;
int NorthEnabled = true;
unsigned long NorthWaitStart = 0;
unsigned long SouthWaitStart = 0;

void setup() 
{
    pinMode(NorthPTTPin, OUTPUT);
    pinMode(NorthCOSPin, INPUT);
    pinMode(SouthPTTPin, OUTPUT);
    pinMode(SouthCOSPin, INPUT);
}

void loop() 
{
   NorthCOSState = digitalRead(NorthCOSPin);
   SouthCOSState = digitalRead(SouthCOSPin);
     {
      if ((SouthEnabled == true) && (SouthCOSState == HIGH))
           {
            digitalWrite(NorthPTTPin, LOW);
           }
     }
     {
      if ((NorthPTTPin == LOW) && (SouthCOSState == LOW))
           {
            digitalWrite(NorthPTTPin, HIGH);
            NorthWaitStart = millis();
           }
     }
     {
      if ((NorthEnabled == true) && (NorthCOSState == HIGH))
           {
            digitalWrite(SouthPTTPin, LOW);
           }
     } 
     {
      if ((SouthPTTPin == LOW) && (NorthCOSState == LOW))
           {
            digitalWrite(SouthPTTPin, HIGH);
            SouthWaitStart = millis();
           }
     }
     {
      if (millis() - NorthWaitStart >= 5000)
          {
           NorthEnabled = ! NorthEnabled // toggle;
          }
     }
     {
      if (millis() - SouthWaitStart >= 5000)
         {
          SouthEnabled = ! SouthEnabled // toggle;
         }
     }
}

Corymacs: One is that where do I put the ";" after toggle is the last code? I put it after toggle but I keep getting the error status 1 expected ';' before '}'. The other is I'm trying to understand how the toggle works? I see it has SouthEnabled = ! SouthEnabled // toggle. How does that make it go true and false?

The semi-colon must go before the "// toggle" - that is just a comment I added to explain what is happening.

A boolean variable can only have two states true or false. The exclamation mark is a NOT operator. Changing from true to "not true" is the same as changing from true to false. Etc.

These

int SouthEnabled = true;
int NorthEnabled = true;

should be

boolean SouthEnabled = true;
boolean NorthEnabled = true;

You have a lot of unnecessary pairs of {} in your code which makes it hard to read even if it works fine. For example

    {  // here
      if ((SouthEnabled == true) && (SouthCOSState == HIGH))
           {
            digitalWrite(NorthPTTPin, LOW);
           }
     } // and here

I think the way you have the code should work.

The reason I was using just one variable southEnabled was to avoid the possibility of both north and south being true at the same time. But I don't know enough about your project.

...R

Ok. I made some improvements. So this is what I have so far. I added your suggestions, removed some pairs. The issue is the program doesn't work as expected. The NorthPTTPin stays LOW %100 of the time and never changes. I can't test the rest of the program since NorthPTTPin stays low. I cycle the SouthCOSState and the NorthPTTPin just stays low. One last dumb question. When should I be using pairs "{" & "}", I was under the understanding it was to separate different parts of code that's why I put each command in their own pair. Thanks for your patients Robin...

const int NorthCOSPin = 2;
const int NorthPTTPin = 3;
const int SouthPTTPin = 4;
const int SouthCOSPin = 5;
int NorthCOSState = LOW;
int SouthCOSState = LOW;
boolean SouthEnabled = true;
boolean NorthEnabled = true;
unsigned long NorthWaitStart = 0;
unsigned long SouthWaitStart = 0;

void setup() 
{
    pinMode(NorthPTTPin, OUTPUT);
    pinMode(NorthCOSPin, INPUT);
    pinMode(SouthPTTPin, OUTPUT);
    pinMode(SouthCOSPin, INPUT);
}

void loop() 
{
      SouthCOSState = digitalRead(SouthCOSPin);
      if ((SouthEnabled == true) && (SouthCOSState == LOW))
           {
            digitalWrite(NorthPTTPin, LOW);
           }
           
      SouthCOSState = digitalRead(SouthCOSPin);
      if ((NorthPTTPin == LOW) && (SouthCOSState == HIGH))
           {
            digitalWrite(NorthPTTPin, HIGH);
            NorthWaitStart = millis();
           }

      NorthCOSState = digitalRead(NorthCOSPin);
      if ((NorthEnabled == true) && (NorthCOSState == HIGH))
           {
            digitalWrite(SouthPTTPin, LOW);
           }

      NorthCOSState = digitalRead(NorthCOSPin);
      if ((SouthPTTPin == LOW) && (NorthCOSState == LOW))
           {
            digitalWrite(SouthPTTPin, HIGH);
            SouthWaitStart = millis();
           }

      if (millis() - NorthWaitStart >= 5000)
          {
           NorthEnabled = ! NorthEnabled;
          }
          
      if (millis() - SouthWaitStart >= 5000)
         {
          SouthEnabled = ! SouthEnabled;
         }
}

In your code you have these two statements. They can never be true because NorthPTTPin and SouthPTTPin are defined as const int. They are not = 0 can never be low.

if ((NorthPTTPin == LOW) && (SouthCOSState == HIGH))
 if ((SouthPTTPin == LOW) && (NorthCOSState == LOW))

You need to be looking at the digitalRead() of the pin, or assign the digitalRead() value to a state and use the state value.