Go Down

Topic: Too many "if" statements? (Read 10894 times) previous topic - next topic

Jim_Socks

I don't know anything about the flash library...  I'll root around and find somewhere I can read up on it!

ALSO, I have attached my sketch here now, in case that is more manageable.

wildbill

Use the F macro to wrap your constant strings e.g.

Code: [Select]
      Udp.write(F("Brantini- Serve with a lemon twist. Enjoy!"));    // Send Message back to iPhone


That should free up RAM in abundance.

PaulS

You could have a lot less code by making flash() take an argument - the number of times to flash. Move the for loops into the flash() function.

The next time that the forum software tells you about the character count limit, attach your code. Reading code spread over half a dozen posts is difficult.

keeper63

Ok - I only looked at the first segment of code, but one thing that stood out to me is the sheer repetitiveness of it; any time your code looks like that, it is a sure sign you need a function (and probably a simple array or two). I'm willing to bet your code could be reduced greatly if you did that.
I will not respond to Arduino help PM's from random forum users; if you have such a question, start a new topic thread.

Jim_Socks

Quote
Use the F macro to wrap your constant strings


Awesome- I will!

Quote
Move the for loops into the flash() function.


I would, but each "for" loop is different.  The number of times to flash changes every time (this corresponds to which bottle is running low- so if bottle number 11 is getting low, then that "for" loop knows to flash the light eleven times) 


if (rumlt >= 180) {
for (int x = 0; x < 11; x++){


and here is the one for bottle #2:

if (rumlt >= 180) {
for (int x = 0; x < 2; x++){


So I wouldn't know how to move that into the flash() function because it is different each time...

Quote
attach your code.


Yeah, sorry about that.  I didn't know I could until afterwards.  I have attached it in an earlier reply in this thread, and I have attached it again here.

Quote
one thing that stood out to me is the sheer repetitiveness of it; any time your code looks like that, it is a sure sign you need a function and maybe an array or two


Two things:

1.  The code definitely looks repetitive.  I don't know how much less repetitive I can make it though.  The reason for this is I have 23 different little pumps, pumping 23 different liquids.  Thus, I need 23 distinctions in code for everything that I do (don't I?).  THEN- I take these pumps, and I make them work together in about 100 different drink "recipes".  These "recipes" use the functions you'll see near the end of the code for different liquids and different lengths of time to run the pumps (this corresponds to measurements, like one once, 1.5 ounces, etc.).  These liquid measurement functions have names like vodka1(); for one ounce of vodka, whiskey5(); for a half ounce of whiskey, or gin15(); for an ounce and a half of gin.  I am not sure how much less repetitive I can make something like this? 

2.  I am so new to this electronics game (actually started 100% fresh at the beginning of this month) that while I have seen arrays in passing on tutorial sites and in a couple examples... I don't know what they actually do, how to use them, or how they could help me out here.  I'll look into them!

marco_c

One way to reduce complexity and size may be to define a class for a pump (assuming they work similarly) and have parameters (variables) that hold the values that make these pumps different from the standard ( such as the name of liquid being pumped, the amount, time to pump, i/o pins, etc).

What you have created is generally know as a recipe handling system in industrial environments and there is a lot of experience in how to do these efficiently. The key is to separate the data from the code and idealize the equipment so that you can reuse the same object multiple times.
Arduino Libraries https://github.com/MajicDesigns?tab=Repositories
Parola for Arduino https://github.com/MajicDesigns/Parola
Arduino++ blog https://arduinoplusplus.wordpress.com

UKHeliBob


Quote

I would, but each "for" loop is different.  The number of times to flash changes every time (this corresponds to which bottle is running low- so if bottle number 11 is getting low, then that "for" loop knows to flash the light eleven times) 


if (rumlt >= 180) {
for (int x = 0; x < 11; x++){


and here is the one for bottle #2:

if (rumlt >= 180) {
for (int x = 0; x < 2; x++){


So I wouldn't know how to move that into the flash() function because it is different each time...


How about something like this :

Code: [Select]


//bottle #11:

if (rumlt >= 180)
{
    flash(11)
}

//bottle #2:

if (rumlt >= 180)
{
flash(2)
}

void flash(int flashCount)
{
for (int aflash=0;aflash<=flashCount;aflash++)
  {
      digitalWrite(LED, 1);
     delay (250);
      digitalWrite(LED, 0);
     delay (250);
  }

}











Please do not send me PMs asking for help.  Post in the forum then everyone will benefit from seeing the questions and answers.

PaulS

Quote
How about something like this :

How about something that at least compiles? C++ statements end with a ;.

But, yeah, that's the general idea. One for loop, in the function, instead of a dozen outside it.

Jim_Socks

Quote
define a class for a pump


Do you mean create a library for the pumps, defining classes and such in it?  If so, I tried this out earlier (with the hole file.h, file.cpp, and keywords.txt mumbo-jumbo) and MAN was it confusing.  Even once I got it doing something, I was completely lost as to how to get it to do anything else...

Or did you mean something else?  Can I define a class right inside my sketch?

Quote
idealize the equipment so that you can reuse the same object multiple times.


Is this the same thing as "defining a class", or is it something else?   I am going to poke around the web for "recipe handling system in industrial environments" and see what I come up with.


UKHeliBob:

Oh man!  I was blind and now I see.  I tried out your elegant solution:


Quote

void flash(byte flashcount)
{
  for (byte x = 0; x <= flashcount; x++)
  {
      digitalWrite(LED, 1);
   delay (250);
      digitalWrite(LED, 0);
   delay (250);
  }




With this in the actual counting operation:

Quote

if (vodka >= 180) {
   
     flash(1);

   delay(warn);
    }



and it works like a champ!  Thanks so much!

UKHeliBob

As well as leaving out the semi-colons I also got the upper limit of the counter in the for loop wrong in my example.

I could try and pretend that both were deliberate as an exercise for the reader, but I won't !

Glad it worked for you.
Please do not send me PMs asking for help.  Post in the forum then everyone will benefit from seeing the questions and answers.

Jim_Socks


Use the F macro to wrap your constant strings e.g.

Code: [Select]
      Udp.write(F("Brantini- Serve with a lemon twist. Enjoy!"));    // Send Message back to iPhone


That should free up RAM in abundance.


I just brought in the latest flash library, and modified my print.h file to accommodate the use of the 'F' thingy, buuut...
it won't work with Udp.write will it?  Only with Serial.print and Serial.prinln right?  My compiler seems to think so...

PaulS

Quote
I just brought in the latest flash library

In a sketch that is already out of memory, I added another resource-intense library. It didn't help. Does anyone have any idea why not?

Jim_Socks


Quote
I just brought in the latest flash library

In a sketch that is already out of memory, I added another resource-intense library. It didn't help. Does anyone have any idea why not?


:smiley-slim:

I had the foresight to comment out a large majority of code in order to experiment with it.  The library works just fine in my sketch when used with Serial.print or Serial.prinln but it doesn't work with Udp.write like the previous gentleman had suggested it would.  Do you know of a way to get it to do so, or were you just being mean?

johncc


Quote
define a class for a pump


Do you mean create a library for the pumps, defining classes and such in it?  If so, I tried this out earlier (with the hole file.h, file.cpp, and keywords.txt mumbo-jumbo) and MAN was it confusing.  Even once I got it doing something, I was completely lost as to how to get it to do anything else...

Or did you mean something else?  Can I define a class right inside my sketch?

Quote
idealize the equipment so that you can reuse the same object multiple times.




Yes, but we can get to that later :)

Quote

...
UKHeliBob:

Oh man!  I was blind and now I see.  I tried out your elegant solution:
Quote

void flash(byte flashcount)
{
  for (byte x = 0; x <= flashcount; x++)
  {
      digitalWrite(LED, 1);
   delay (250);
      digitalWrite(LED, 0);
   delay (250);
  }
...
if (vodka >= 180) {
   
     flash(1);

   delay(warn);
    }



and it works like a champ!  Thanks so much!


Welcome to the world of functions!!
In your code you can take this one step further with good effect by adding this:

Code: [Select]

void flashdrink(int & amt, byte flashcount, int min=180, int max=190 )
{
  if ( amt > max) amt = 0;
  if ( amt >= min) {
    flash( flashcount);
    delay( warn);
  }
}



now your calling code can be simply like this:
Code: [Select]

  flashdrink(orange, 17);
  flashdrink(pine, 18);
  flashdrink(cola, 19);
  flashdrink(sprite, 20);
  ...
  flashdrink(water, 24, 220, 250);  // or if you find that it needs to be other than 180 and 190



Cheers,
John

PaulS

Quote
Do you know of a way to get it to do so, or were you just being mean?

The flash library and the F() macro have nothing to do with each other. Introducing another library into the mix really isn't the answer.

Not all classes support the F() macro (or the underlying architecture behind it), though they all could. It's simply a matter of adding a couple more overloads to the print() method.

Go Up