arduino uno aquarium water changer

Using Arduino Project Guidance
#1

hello everyone,

I am in the process of building a water changer for my 400l reef aquarium. I am after a little guidance for the help with coding the arduino as I have attempted to code it modifying some code i found online running some pumps. it has not worked as I would like, I have added two extra portions to the code along the lines of an auto top-up to put in what has evaporated and a pump to mix the salt water in a remote container in my garden.

the main part of the code is to dose the tank every hour to have the tank running on a constant water change until the container in the garden is empty which a illuminated switch inside will flash to show it is empty. there are three switches on the tank, one to start the mixing at which it will illuminate while it is mixing. the other two switches are to me used to prime the dosing pumps used to top up the tank and change the water. these are also illuminated so they will will give feedback as to what is going on(when topping up it will illuminate the top up switch whilst the pump to top the tank up is running. when the water changer pumps are running the switch to prime the water changer pumps will illuminate.)
I have attempted to code it so when the container in the garden is empty the switch for the water changer and the top up will flash is series.

I felt I should explain what my aim of what I have coded should do as to someone with experience might look at it and see it makes no sense, I haven’t come on here to outline my project and just ask for the code to be written for me. I know for someone with little experience this may be a little bit of a big project but I would like help in outlining my mistakes in a view to learn and learn to code properly and maybe with a little guidance I will be able to have my project running properly.

I have attached my code for you to look at as I do not know if there would be specific parts that need looking at.

thanks in advance for your help.

Aquarium_water_changer_v1_1edit1.ino (8.28 KB)

#2

If you want advice on fixing problems with your code, you will need to explain what the problem is.

Some comments from a brief skim through:

Comments about the use of parameters to functions such as setDateDs1307 much appreciated, well done. However, the comment for the hour parameter looks wrong - they would be 0 - 23, surely? I suggest you make sure you're absolutely clear how you're using these numbers, document it just as you have, and then review your code to make sure that is how they are actually used everywhere.

By the time your code gets to the point where you're asking for help or feedback, you should IMO have eliminated redundant code such as the commented-out sections. Every bit of unused code (commented out or not) makes it that bit harder to see what the rest of the code does. One common reason for wanting to keep legacy code is that you don't want to lose it. That would only be a concern if you don't have a change control system in place. For a sketch containing this much work I strongly recommend that you should have a change control system in place. There are plenty of good free ones around, and you can even kludge your own just by coping your source files somewhere else from time to time, but I find File Hamster is very convenient when I can't be bothered to use a more formal code versioning system.

Anywhere in your code where you have two blank lines together, get rid of one of them. If you have a blank line immediate after an opening brace { or before a closing brace }, get rid of it.

I don't know what the huge if statement on lines 148 - 171 is intended to achieve, but what it actually does is certainly not what you will be expecting.

Indent your code. The Arduino Tools / Autoformat command will do this for you.

There is a spurious semicolon on lines 216, 235, 253 and 267 which completely change the logic. Get used to putting every { and } on separate lines with matching pairs indented by the same amount - it will help you spot typos like this.

The way you're using delay() to control the timing means that your sketch will not respond to user actions or sensor input changes during the delay. Is that what you want?

#3

Thanks for the reply.

I have reviewed my code as to the comments you have suggested.
It has been reduced to the main code with added comments explaining specific functions within the code as my annotations in the code became a little lacking to say the least.

As you have said with the delay, I don’t want the system to be unresponsive which they are at the moment but I am unsure how to make the system respond quickly to user inputs as rigging a push button to test inputs there is a lot of lag between pushing the button and the function associated actually happening. I don’t mind thought that when the mixing pump is running that the other buttons do not respond and to prime water that is not fully mixed into the pipe lines.

With the code better arranged the lag has reduced to a matter of milliseconds which I can live with.

the but I am having trouble with now is having the water changer pumps run at a specific time as you said in my original code there was a lot of lines that would not perform as I would like, do you have any advice as to how I can have it run every hour as long as the other parameters in the line are true.

(hour == 1)&&(minute == 00)&&(second==00)&&(floatState == LOW)&&(motorPin4 == 0),

to again reduce it surely as mins and secs are not used this would be better

(hour == 1)&&(floatState == LOW)&&(motorPin4 == 0),

but to have the pumps run every hour in what way should I write it, as the 24 lines that are pretty much the same apart from the 1 hour increment?

again I have attached code, if you could take a skim again to see if you are happier with its layout now, i have also removed the 24 lines of timing code to just one line to make things easier to review as it was a eye sore.

Aquarium_water_changer_v1_1edit1.ino (6.45 KB)

#4

Names like buttonState2 aren’t helpful. Often having a number of similar variables with numeric suffixes suggest that an array would be useful. Not so in this case - you just need names that show their function. As you went half way with the pin name, it looks like buttonState2 should really be ChangeButtonState. Clearly there are many others that have the same issue.

This appears a number of times in the code:

    getDateDs1307(&second, &minute, &hour, &dayOfWeek, &dayOfMonth, &month, &year);
    Serial.print(hour, DEC);
    Serial.print(":");
    Serial.print(minute, DEC);
    Serial.print(":");
    Serial.print(second, DEC);
    Serial.println(" ");

It looks like a good candidate for being put in its own function.

This:

  if((hour == 0)&&(floatState == LOW)&&(motorPin4 == 0))

Will fail as motorPin4 is a pin number, 6 in this case, so it will never be zero. You need to use digitalread (and perhaps a nicely named state variable) as you have elsewhere.

To run the pumps hourly, just check that minute is zero and second is in some range; given your current delay of 38000 you could check that sec is less than 38. It would likely work just as well < 10, but you’ll need to change it when you get rid of the delay later.

#5

If you want something to happen every hour on the hour then read the hour and compare the value to what it was last time you read it - if the two differ, you've just rolled over an hour so start whatever it is you want to happen. This is just like edge detection on a switch input, and the StateChangeDetection example sketch demonstrates how to do that.

#6

the buttons are not used as a change of state detection the CHANGEBUT and TOPBUT are only to run the motors when the switch is HIGH having the name CHANGEBUT is prob just a little confusing, I have attempted to model the code based on the button sketch available to learn arduino with.

In terms of the motorPin4 issue I must admit that has been an oversight as yes without telling the arduino that it has to read that pins output it would never have worked I have changed the code to this:

void loop() // run over and over again
{
motorState = analogRead(motor4);
floatState = digitalRead(float1);
floatState2 = digitalRead(float2);
buttonState = digitalRead(MIXBUT);
buttonState2 = digitalRead(CHANGEBUT);
buttonState3 = digitalRead(TOPBUT);

byte second, minute, hour, dayOfWeek, dayOfMonth, month, year;

// Set the time you want the motors to kick in
if((hour == 0)&&(floatState == LOW)&&(motorState == 0))

I trust this would remedy the issue with the motor by declaring it has to check its state.

With the serial printing of the time in the code you say bill that it would be sensible to put it as a function, I am having trouble working out how to do this, would I be right in saying that I should put it in its own void to have it just once written and telling it be to executed where required within the code writing the void as:

void time ()
{
byte second, minute, hour, dayOfWeek, dayOfMonth, month, year;
getDateDs1307(&second, &minute, &hour, &dayOfWeek, &dayOfMonth, &month, &year);
Serial.print(hour, DEC);
Serial.print(":");
Serial.print(minute, DEC);
Serial.print(":");
Serial.print(second, DEC);
}

if this is right I have tried getting it to run further down the code without any real success as if I use it on the mixer section of code it doesent serial print the time and instead just prints "mixing" without the timestamp.

if(buttonState == HIGH)
{ time
Serial.print(" MIXING");
analogWrite(motorPin4, 255);
digitalWrite(REDLED, HIGH);
delay(1000);
Serial.print(" MIXING DONE");
Serial.println(" ");
analogWrite(motorPin4, 0);
digitalWrite(REDLED, LOW);
}

I trust with that I am just being abit idiotic in how I am trying to get it to run the void time before printing "MIXING".

I am looking to have the pumps run for 38 seconds every hour so I will have a look at the StateChangeDetection sketch and have a go at coding it into my project.

I appreciate the help you are giving me for this hopefully as time goes on and I gain more experience I could return the favor to others also.

#7 
  if(buttonState == HIGH)
  { time

This doesn’t even compile, does it? Perhaps you’ve noticed that digitalRead() and Serial.print() and all other functions are CALLED using parentheses to indicate that you are calling a function.

time;

does not all a function. It is equivalent to

42;

syntactically valid, but useless.

time();

on the other hand, calls the function.

#8

Thanks Paul for pointing that out, I thought it was just something simple that I was failing to see and it was just because I wasn't pointing out to the arduino which piece of code to run.
With all this help my code is looking more sensible as the original code I uploaded was horrendous compared to what I have now.

In terms of the hourly run, I haven't yet looked into the StateChangeDetection code but I will look into it tomorrow with the view to implement it and it should function better than what I had first written.

#9

I have had a look into using some code based on the StateChangeDetection example and am not having much success with coding it to check if the hour has changed to run the pumps hourly. I have written the code like this (which to you experienced folk must be a laugh as it probably completely wrong):

void loop() // run over and over again
{
motorState = analogRead(motor4);
floatState = digitalRead(float1);
floatState2 = digitalRead(float2);
buttonState = digitalRead(MIXBUT);
buttonState2 = digitalRead(CHANGEBUT);
buttonState3 = digitalRead(TOPBUT);

byte second, minute, hour, dayOfWeek, dayOfMonth, month, year;

// Set the time you want the motors to kick in
if(hour != lastHour){

if((hour == lastHour++)&&(floatState == LOW)&&(motorState == 0))
// this prints the output to the serial window (tools > serial monitor in arduino) and is great for testing
{ time();
Serial.print(" TRUE");
Serial.println(" “);
Serial.println(” WATER CHANGE");
digitalWrite(BLUELED, HIGH);
analogWrite(motor1, 255);
analogWrite(motor2, 255);
delay(38000); // set how long you want the motor to run… 1000 = aprox 1ml

analogWrite(motor1, 0);
analogWrite(motor2, 0);
digitalWrite(BLUELED, LOW);
}

I set the

int lastHour = 0;

to just start the code with a default value.

I assume this is completely wrong, I thought I would have a go before coming back on here to see if I could actually code it properly but I cannot figure out how to use the StateChangeDetection code as a guide for what I need.
I know it needs to recognise the current time (hour in particular) and compare it when it cycles through the code each time untill the hour changes by +1 to then implement the output I want, I need to include the motorState and the floatState so this part of the code cannot run if these values aren’t true to prevent my tank from being drained (at 400l I know it will take a while when i’m only moving 38ml every hour).

Any pointers? as I would like to try and figure it out rather than just straight forward being given the correct piece of code.

#10

Is anyone able to help out? I have tried coding it a few different ways and all don't function I have coded it using "byte hour" and tried using "byte &hour" to which some don't compile and others do but don't function and I haven't got the foggiest of how to overcome it. A couple pointers to get me in the right direction as my projects code is nearly complete. When it is I can look about getting the pumps and connecting it to the circuitry and testing the system out.

#11

Your time variables need to be global. I assume that they are if your code compiles, but the redeclaration in loop is causing a problem. You can safely get rid of it I think - need to see all your code to be sure though. When you post it, use code tags (# button), not quote.

#12

I cannot put my code here as it is too long I have put the part of how I currently have the time change detection piece thats wrong and i have attached my full code incase there are params from the code needed to help.

void loop() // run over and over again
{ 
  motorState = analogRead(motor4);
  floatState = digitalRead(float1);
  floatState2 = digitalRead(float2);
  buttonState = digitalRead(MIXBUT);
  buttonState2 = digitalRead(CHANGEBUT);
  buttonState3 = digitalRead(TOPBUT);

 byte second, minute, hour, dayOfWeek, dayOfMonth, month, year;
    getDateDs1307(&second, &minute, &hour, &dayOfWeek, &dayOfMonth, &month, &year);
    
  lastHour = hour;
  // Set the time you want the motors to kick in
  if(hour != lastHour){
 
    if((hour == lastHour++)&&(floatState == LOW)&&(motorState == 0))
    // this prints the output to the serial window (tools > serial monitor in arduino) and is great for testing
  { time();
    Serial.print(" TRUE");
    Serial.println(" ");
    Serial.println(" WATER CHANGE");
    digitalWrite(BLUELED, HIGH);
    analogWrite(motor1, 255);
    analogWrite(motor2, 255);
    delay(38000); // set how long you want the motor to run... 1000 = aprox 1ml

    analogWrite(motor1, 0);
    analogWrite(motor2, 0);
    digitalWrite(BLUELED, LOW);
  }

what redeclaration do you mean?

Aquarium_water_changer_v1_1edit1.ino (5.6 KB)

#13

This section is strange:

  getDateDs1307(&second, &minute, &hour, &dayOfWeek, &dayOfMonth, &month, &year);

  lastHour = hour;
  // Set the time you want the motors to kick in
  if(hour != lastHour)
  {
    if((hour == lastHour++)&&(floatState == LOW)&&(motorState == 0))
      // this prints the output to the serial window (tools > serial monitor in arduino) and is great for testing
    { 
      time();

I take back what I said about globals - the declarations you have make sense now I see the call to getDateDs1307.
That lastHour=hour needs to move to the end of loop. As it is, it's defeating the first if - it'll never be true.
I don't understand what motorstate is doing and I see no reason to test hour against lasthour again. I'd write the second if as:

    if(floatState == LOW)
#14

So moving the lastHour = hour to the end of the loop should enable the time controlled side to my project work. The motorstate is to stop the water change side of the code running if the mixing pump is running as it's programmed to go to 255 and I will add a delay of an hour maybe two depending on testing so that the system locks out for that time making it impossible to dose the tank with unsalted water.

Thanks for your help bill I will edit my code and upload it to arduino and leave it running for an hour and see if the serial gets a print out.

#15

For initial testing purposes, I'd be inclined to make the time based functions run based on a minute changing rather than an hour. Maybe reduce the 38 second delay to 5 or so too. Otherwise, every code change will incur a huge wait for the top of the hour and you'll get bored before you get it working.

#16

Very true I'll adjust all hour timing to minute and reduce delay too

#17

So far with having a play all seems good. I'll continue to have a fiddle and look about finishing its construction and do a follow up post when I have tested the hardware but the pumps come from china so might be a few weeks before I get to play properly.

thanks again to everyone who helped me with my project I'll be adding you to the comment introduction at the top of the code :slight_smile:

#18