Trying to count down in a bar graph in 5 min increments (switch case?)

So I want to have a timer that I can increase by 5 min increments with a button... easy enough, set a counter and add to it when the button is pressed.

But then the 10 segment bar led needs to be lit according to time left. 50 mins + equal all lit up and when it's down to under 5 mins it should only have the last led lit up.

Obviously I could do this with an obnoxious nested if than else if stuff but I'm sure there is a cleaner way.

I thought switch case with case 1 ... 499999 but then when I add the 2nd case of case 500000 ... 999999 it says they overlap.

Am I barking up the right tree but missing something or is there a better way to do this?

Please post that troubeling code.
How do You think anybody could tell what's wrong?
Read the topic "How to get the best from forum"!

It sounds like you are just doing millis timing?

Set an unsigned long variable “period” to whatever the timer is required to be. When you press a button add 5 mins to it.

Do standard millis timing and compare it to this variable. If millis - startTime >= period, time is up

LEDs are similarly just an equation. PeriodLeft = period- (millis-startTime)

Divide by 5mins and use this to determine the switch case for LEDs

1 Like

The map() function may be what you're looking for.

1 Like

I'm not particularly looking for help on troubling code and I posted the code in which it's suggesting that it's "overlapping"... but I'm wondering CONCEPTUALLY what is the best method to handling this.

I could 100% code this and have it work right out of the box... I'm trying to learn best practices on accomplishing what I would like.

I will spell the concept out more clearly.

10 LED's (label them 1-10)

At 5 mins light up 1, at 10 mins light up 1 and 2. At 15 mins add 3, at 20 mins add 4 and so one.

Then when coming back down they will take away number 10 then number 9 and so on.

One approach would be to code a lookup table with three values per row - min, max, and # of LEDs. I'd expect a single loop. For simplicity, I'd follow the upward sequence with the downward sequence, though it eats twice as much room.
You'll need to catch the end condition, where you run out of table, and need to reset your millis start value.
Example:
500 1000 0x001
1001 2000 0x011
2001 3000 0x111
3001 4000 0x011
4001 5000 0x001
capture new millis() value here, and restart loop
advantage of a table is, you can change the contents without searching through your code looking for magic numbers.

I thought that was it... but it is going to "case 0" while there is still several minutes left.

countTimer = map(countTimerMillis, 0, 6000000, 0, 21);

This creates the case variable (countTimer) and uses how many millis are left in the count down.

So I start by adding 300,000 to the count down but once it gets to 285681 it drops to case 0.

I originally had it map to 20 and it would count up the cases each time I added 300,000 but that was before I implemented the actual count-down part... once it was counting down I noticed it was going to case 0.

i want the final LED to stay lit for the last 5 mins and everything going off when it reaches 0.

pretty hard to help with undisclosed code. Show us what almost worked, please.

I mean, posting all of the code would just be tedious and take up space as I've literally put serial.println's in there that show the code is counting down in countTimerMillis just like it should and countTimer that holds the map value is all that is important which is defined via the map function in my post above.

The problem is that it is going to 0 while there is still time left. Either map isn't the right function to use to achieve what I'm trying to achieve or I need to change how I have it mapped or put some other clause in there that I'm missing.

Again conceptually, I need to keep an output on until the timer (countTimerMillis) reaches 0 and it needs to step down the bargraph led (another 9 outputs as the timer passes each 5 min mark).

From your first post, your problem was "switch case", which is limited to int and char input types. Not long unsigned ints, apparently. Later problems point to similar issues(e.g. the mysterious drop to 0).
From your posts you seem very focused on doing it your way, with map, and not attempting anything outside that box. Come up with an MRE, then, if you're unwilling to expose your code. If you don't know what an MRE is, then ask, but we'll probably point you to "how to get the most out of this forum".

Assuming your 10 LEDs are connected to pins 2 thru 11, something like

unsigned long compare = 0;
for (byte led = 2; led <= 11; led++) {
  if (targetTime - millis() > compare) {
    digitalWrite(led, HIGH);
  }
  else {
    digitalWrite(led, LOW);
  }
  compare += 300000UL;
}

Sorry if I gave off that impression, I'm certainly open to outside the box. I originally stated that I could do it with a bunch of nested if garbage but figured there was probably a better way.

As you see, I said I THOUGHT switch case might be the way but was running into problems. Thanks to your post I understand that I was asking too much with the switch case and using my raw millis. Though I have map'd it down to ints.

I also said, "am i barking up the right tree or is there a better way" thus asking for outside the box things I have not thought of.

I didn't see much else that stood out, but someone suggested using map() so I attempted that and then ran into my new problem. But I have never been above striking all of that code and going a different route.

I again feel like I attempted to demonstrate that i was open to new/better ideas, because frankly I had ZERO preconceived notion at how I should accomplish this and was going down any rabbit hole that I could follow.

I am more than happy to dump my mediocre code of boring counting down... but it's literally just a lot of work to get to a long countTimerMillis that could be replicated by:

long countTimerMilis = 600000;
countTimerMillis = countTimerMillis - 1;

Make 10 led's start dropping off every 300000 ending with that last one being on for that last 300000 and going off at 0.

I just feel like 150+ lines of misc code to focus on that part of the code is merely a distraction because I'm looking more at learning how to solve the problem not correct a particular problem in my code.

This is more of a theoretical/educate me post than a "please help me make my code work!!!" post.

Does that make sense?

Interesting.

So the compare grows by 5 mins every iteration through the for loop and ergo comparing each time through to a higher and higher interval.

Very interesting... I'll give this a go, that's very clean and simple to code but kind of higher level to follow the logic at a glance... very outside the box!

I don't just need all of them to turn off at 0 I need them to drop off, one every 300000 millis.

Yes, but I'll not be responding again until about 5-6 hours from now, when I get back(home just to grab lunch). Sorry. The answer, to me, is "table driven", because it's simple, modifiable,, flexible, and robust. But to work up an example, I really need to sit down at my dev machine, not here on my browse machine.
C

For the record, I ended up using PaulRB's approach, just needed to tweak it a little as his example didn't account for how long the arduino had been running.

Just needed to use some "prevMillis" to take that into account and do math with that.

But his for loop with incrementing a comparator by 5 mins was exactly what needed to be done to neatly cycle through the bar graph and turn on or off the led's depending on the state of the counter.

Here is the relevant bit of code:

if (countTimerMillis <= 0) { //if Time is up or has been stopped turn off everything
    countTimerMillis = 0;  //reset the timer to 0 as it might be negative as a result of math
    digitalWrite(solenoid, LOW); //turn off solenoid
    for (int b=0; b<=10; b++) { //turn off all bargraph leds
    digitalWrite(bg[b], HIGH);
    }
  } else { //if the timer isn't done do the work
        countTimer = 0; //reset the comparator for the bargraph
        for (byte led = 2; led <= 11; led++) { //this loop steps through each led and checks it against a quasi 5 min table
        if (countTimerMillis - (prevMillis - millis()) > countTimer) { //if the timer is higher than the current spot in the table
        digitalWrite(led, LOW); //turn on that corresponding LED in the bargraph
        } else { //once it's below that threshold turn it off
        digitalWrite(led, HIGH);
      }
      // Serial.print(countTimer);
      // Serial.print(",");
      countTimer += 300000UL; //increment through the table by 5 mins
      digitalWrite(solenoid, HIGH); // we are currently running so the solenoid needs to be open/on

    }
      }

It doesn't need to do that. The idea is that targetTime is the value millis() will have at the end of the countdown. For example if, at the moment when millis() == 1000000, the user sets a 15 min countdown, then targetTime would be set to millis() + 15 * 60 * 1000 == 1900000

you may have made things more complicated than they need to be. It's hard to tell without seeing the "irrelavent" parts of your code, but judging by the part you did post, it kinda looks like it. My code would not need extra code to switch off all the LEDs when time is up, it would just happen anyway.

I see... yes, that wasn't how I figured target time.

I was just adding 300,000 (5 mins) to a counter and using millis to subtract it.

Your way again would have been more simple and elegant than mine. :slight_smile:

if (countTimerMillis != 0) { //if the timer is running do the subtraction to the countTimermillies
  countTimerMillis = countTimerMillis - (millis() - prevMillis);
  prevMillis = millis();
}

I also have the ability to add 5 mins to my counter at any given time to extend the on time. So not sure if what you are purposing would account for that... I'm sure it would with a bit of better code... just part of what I was trying to account for that painted me into the corner that I had to work my way out of. :slight_smile:

Something like

if (add_five_minutes) {
  if (targetTime < millis()) targetTime = millis();
  targetTime += 300000UL;
}
1 Like

Tomorrow's request will be to make each interval unique. Decide how you'll accommodate that! :grinning:

1 Like