Code works, but not with a button

[LarryD]

Hardware wise, you might prevent accidental shorts between 5v and GND.

[SteveThackery]

@LarryD

You still have this:

if (currentTime = millis()) ←

Do you understand why this is wrong, @smacks999 ?

[smacks999]

I know you mean
if(currentTime == millis())

It makes no difference in the behaviour. Unless I am just being dense. With this code the button never works.

If i understand correctly (and I really appreciate you folks taking the time to help me straighten out), the == means if a and b are the same, then TRUE. the single = assigns right to left (?).

[SteveThackery]

Yes, that is correct. I read '==' as "is equal to", thus "if (x is equal to y)".

I need to study your code to work out what it does before I can help further. I think in your position I'd start with the bare minimum functionality, as @DrAzzy suggests, get it working and then add features to it. It's a bit complicated in its current form to glean what it's supposed to do.

[LarryD]

Yes.

---

To repeat:

Do you know what the difference is in these two statements ?

Do something when a switch becomes HIGH.

Do something when a switch is HIGH.

Which one did you want ?

[SteveThackery]

@smacks999

currentTime = millis();
  if (currentTime <= redLength)
  {
    digitalWrite(redPin1, HIGH);
    digitalWrite(redPin2, HIGH);
    digitalWrite(relayPin, HIGH);
    digitalWrite(dryDock1, LOW);
    digitalWrite(dryDock2, LOW);
    digitalWrite(dryDock3, LOW);
    digitalWrite(dryDock4, LOW);
    digitalWrite(dryDock5, LOW);
    digitalWrite(dryDock6, LOW);
    digitalWrite(dryDock7, LOW);
  }


  if (currentTime = millis())

I'm puzzled. You make currentTime equal to millis(). Then you do an if statement around redLength. Then you go

if (currentTime == millis())  //corrected as discussed

Why are you doing that test, bearing in mind you assigned millis() to currentTime just a few lines previously?

It might or might not test true, depending on whether the millisecond counter has moved on whilst it was doing the previous test (the redLength test).

I don't think this is what you intended. What did you actually want?

[SteveThackery]

@smacks999

currentTime =- millis();

What is that line supposed to do?

[SteveThackery]

@smacks999

if (currentTime  >= eventInterval7) {
    /* Event code */
    Serial.println("Ice Ice Baby7");
    digitalWrite(dryDock7, HIGH);
    digitalWrite(redPin1, LOW);
    digitalWrite(redPin2, LOW);
    digitalWrite(relayPin, LOW);
    Serial.println("end");
    for (;;) {}
  } else
  { }

I'm probably being thick here, but after that final event, isn't it just going to sit in that for loop forever? Or have I missed something?

[wildbill]

That's the point. The sequence should run once then end. I'll guess that that will change later to be run on a button press, but hey, that's later

[LarryD]

for (;;) {}

The OP said this was temporary.

Oh, yes, the “for” statement at the end is just so I can stop the print. I don’t intend it to be used in the final sketch.

---

What’s your background @smacks999 ?

[SteveThackery]

You know what? I'm about to commit blasphemy. I think this might be one of those occasions where it's actually better to use delay().

As I understand it, all that this code does in turn on and off a set of LEDs in a preset sequence, and then wait until the button is pressed, at which point it all repeats.

In pseudocode I would write it something like this:

void loop() 
{
   do event0
   delay() for interval0 milliseconds
   do event1
   delay() for interval1 milliseconds

   ...

   do event7
   delay() for interval7 milliseconds

   while (not keypressed)
   {  //do nothing 
   }
}

Wouldn't that do the job?

[SteveThackery]

@wildbill

That’s the point. The sequence should run once then end. I’ll guess that that will change later to be run on a button press, but hey, that’s later

Sorry, I'm obviously confused. I thought you wanted it to run each time you pressed the button?

Obviously I'm causing confusion by coming in part way through. Sorry! But, have a look at my previous post about using delay(). Won't that work for you?

[LarryD]

@SteveThackery

I’m about to commit blasphemy.

[wingman2]

I see 2 "if" statements without squiggly brackets. Jes' sayin'. (Newbie observation)

[wildbill]

You are indeed confused - I'm not the OP

[SteveThackery]

I know. I just got mixed up because you answered for the OP. So it's all your fault.

[SteveThackery]

@wingman2

I see 2 “if” statements without squiggly brackets. Jes’ sayin’. (Newbie observation)

You can have if statements without the squiggles, but you are right - there appears to be at least one instance where he should have used squiggles and didn't.

Personally I'd start again and write it with delay() statements, as outlined above. It's way too messy and complicated at the moment.

[jimLee]

I have a button library that may help you. But you have to loose the eternal resistors. You just want them pin to ground. In fact, just hook one up from pin to ground and we can see if it'll help.

-jim lee

[alto777]

@wingman2

I see 2 “if” statements without squiggly brackets. Jes’ sayin’. (Newbie observation)

Good eye.

For next time:

{}       braces
[]       brackets
()       parentheses

I kinda like "squiggly", tho.

a7

[smacks999]

There will be other things going on, thus delay will not be appropriate. I am going to study this for a day or two. Thank y'all for your help and suggestions and patience.