Page 1 of 1

Dosing Pump conflicts with Wavemaker

Posted: Thu Jan 05, 2012 7:39 am
by dedvalson
Hi,

My system includes the dual dosing pump on 2 relays plus 2 pumps running as wavemakers. My sketch includes the following:

Code: Select all

    // run wavemakers during the day
    if (hour() > 10 && hour() < 23)
    {
      ReefAngel.Wavemaker1(WAVEMAKER1);
      ReefAngel.Wavemaker2(WAVEMAKER2);
    }
    else
    {
      ReefAngel.Relay.Off(WAVEMAKER1);
      ReefAngel.Relay.Off(WAVEMAKER2);
    }
    // run dosing pumps at night
    if (hour() < 10)
    { 
      ReefAngel.DosingPumpRepeat1(ALK_DOSER);
      ReefAngel.DosingPumpRepeat2(CALC_DOSER);
    }
    else
    {
      ReefAngel.Relay.Off(ALK_DOSER);
      ReefAngel.Relay.Off(CALC_DOSER);
    }
This causes the wave makers to run from 10 AM to 11 PM and the dosing pumps to run from Midnight to 10 AM. The problem is that the wavemakers don't come back on the second day (or ever again till the Reef Angel is reset)

I looked at the source code for the Dosing Pumps and the Wavemakers and found the following:

1. Dosing Pump 1 uses the same timer as Wavemaker 1.
2. The Dosing pump code leaves the timer untriggered when it is done running.
3. The Wavemaker code leaves the timer triggered when it is done running.
4. The wavemaker code uses a static variable to control it's initialization, so it only ever initializes the timer once.

I am really glad that I didn't try to have the wavemakers and the dosing pumps run at the same time, the interference could have caused a severe overdose of my tank. As it is, I am just losing the use of the wavemakers.

It seems to me like the wavemaker function should be rewritten to not use timers. I don't see any reason for them to. Couldn't the wavemaker function be rewritten as follows:

Code: Select all

void ReefAngelClass::Wavemaker1(byte WMRelay)
{
     if ((now() / InternalMemory.WM1Timer_read()) & 0x01);
          Relay.On (WMRelay);
     else
          Relay.Off (WMRelay);
}
This would free up Timers 1 and 2 for the Dosing pumps and avoid the current dangerous conflict between the two functions. It also allows the user to run as many wavemakers as they want.

Re: Dosing Pump conflicts with Wavemaker

Posted: Thu Jan 05, 2012 9:08 am
by rimai
I like the idea of not using timers too :)
I think even the dosing pumps don't need timer if we used some funtion similar to what you just posted.
Something like

Code: Select all

now()%Interval<Duration
I think this would work for both WM and dosers.
What do you think?

Re: Dosing Pump conflicts with Wavemaker

Posted: Thu Jan 05, 2012 9:28 am
by dedvalson
rimai wrote:I like the idea of not using timers too :)
I think even the dosing pumps don't need timer if we used some funtion similar to what you just posted.
Something like

Code: Select all

now()%Interval<Duration
I think this would work for both WM and dosers.
What do you think?
That looks like it would work and is elegantly simple.

If we are going to change the dosing pumps I would like to add something else as well. The current code for repeating dosing pumps ignores the Start Time value in memory. I think we should use the start time even in the repeating dosing pump algorithim in order to offset the two pumps. Currently I run my pumps for 5 seconds every hour. I would like to make the pumps not run at the same time, but I can't do that with the current code. So instead of basing the time from midnight like it is now, we could base it on the start time in memory.

So perhaps we would have something like:

Code: Select all

(now()+(InternalMemory.DP1OnMinute_read * 60))%Interval<Duration
Or (even better) it would be awesome if there were an on time and an off time in memory, but that would be a big change that would involve the client and everything.

Re: Dosing Pump conflicts with Wavemaker

Posted: Thu Jan 05, 2012 9:55 am
by rimai
Yeah. Agreed.
That was the same reason I didn't use the function and went on using my own.

Code: Select all

  if (ReefAngel.DisplayedMenu==255 && minute()==0 && second()<InternalMemory.DP1Timer_read())  //Alk Doser - Only works if Main screen is showing
    ReefAngel.Relay.On(AlkDoser);  //Turn Alk Doser on
  else
    ReefAngel.Relay.Off(AlkDoser);  //Turn Alk Doser off

Re: Dosing Pump conflicts with Wavemaker

Posted: Thu Jan 05, 2012 10:47 am
by dedvalson
Is the test for DisplayedMenu actually needed? Wouldn't waterchange mode or feeding mode override whatever is done here anyway?

Don

Re: Dosing Pump conflicts with Wavemaker

Posted: Thu Jan 05, 2012 10:52 am
by dedvalson
This is kind of an aside, but I noticed something else here. I keep having to do this:

Code: Select all

if (whatever)
     ReefAngel.Relay.On (SomeRelay)
else
     ReefAngel.Relay.Off (SomeRelay)
We need to add a function to the Relay class called Set so you could do:

Code: Select all

ReefAngel.Relay.Set (byte Relay, boolean on)
Then we could do

Code: Select all

ReefAngel.Relay.Set (SomeRelay, Whatever)
Don

Re: Dosing Pump conflicts with Wavemaker

Posted: Thu Jan 05, 2012 2:12 pm
by rimai
Good suggestion

Re: Dosing Pump conflicts with Wavemaker

Posted: Thu Jan 05, 2012 3:06 pm
by dedvalson
rimai wrote:I like the idea of not using timers too :)
I think even the dosing pumps don't need timer if we used some funtion similar to what you just posted.
Something like

Code: Select all

now()%Interval<Duration
I think this would work for both WM and dosers.
What do you think?
Just realized a minor problem here, Interval is in minutes, duration is in seconds. So you would need

Code: Select all

now()%(Interval * 60)<Duration

Re: Dosing Pump conflicts with Wavemaker

Posted: Fri Jan 06, 2012 8:27 pm
by binder
I'm all open for suggestions. I had issues when creating the repeatdosinginterval stuff. I didn't know how much it was being used either. I'm going to look at the code suggestions you gave and try to follow them more. I've only glanced at them briefly and my brain isn't fully functioning and grasping it yet. But that should easily be remedied.


And as for the Set function...making sure i follow properly, that is supposed to take the place of the "if else" clause, right?

Re: Dosing Pump conflicts with Wavemaker

Posted: Fri Jan 06, 2012 8:55 pm
by rimai
Hey Don,

I have to admit that your contribuitions are always awesome!!!
Look how simple the function became:

Code: Select all

void ReefAngelClass::DosingPumpRepeat(byte DPRelay, byte OffsetMinute, int RepeatMinute, byte RunTime)
{
  Relay.Set(DPRelay,((now()-(OffsetMinute*60))%(RepeatMinute*60))<RunTime);
}

Re: Dosing Pump conflicts with Wavemaker

Posted: Fri Jan 06, 2012 8:58 pm
by binder
Looks like I've got some additions to make for my .20 release. :)

Re: Dosing Pump conflicts with Wavemaker

Posted: Fri Jan 06, 2012 8:59 pm
by rimai
I'm going to have these on the dev branch for you :)

Re: Dosing Pump conflicts with Wavemaker

Posted: Fri Jan 06, 2012 9:01 pm
by binder
Oh, and that should also eliminate the need for some of those 'timer' variables which would therefore cut down on the code size. PLUS it will make adding on additional wavemakers and dosing pumps easier (like Don already said).

cool....i'll check it out and merge it on in to my master branch. :)

Re: Dosing Pump conflicts with Wavemaker

Posted: Fri Jan 06, 2012 9:13 pm
by rimai
And this is the new Wavemaker():

Code: Select all

void ReefAngelClass::Wavemaker(byte WMRelay, byte WMTimer)
{
  Relay.Set(WMRelay,(now()%(WMTimer*2))<WMTimer);
}

Re: Dosing Pump conflicts with Wavemaker

Posted: Wed Jan 11, 2012 4:57 pm
by binder
Here's what I've got for the new DosingPump function (non repeat function):

Code: Select all

void ReefAngelClass::DosingPump(byte DPRelay, byte OnHour, byte OnMinute, byte RunTime)
{
    Relay.Set(DPRelay, (now()%((OnHour*3600)+(OnMinute*60)))<RunTime);
}
Doing it this way will completely eliminate the need for the TIMER function. That will also simplify the code, however the "long" form code signatures will change thus breaking some PDE/INO files. Oh well, it happens and is an easy fix.

Re: Dosing Pump conflicts with Wavemaker

Posted: Wed Jan 18, 2012 12:48 pm
by dedvalson
binder wrote:I'm all open for suggestions. I had issues when creating the repeatdosinginterval stuff. I didn't know how much it was being used either. I'm going to look at the code suggestions you gave and try to follow them more. I've only glanced at them briefly and my brain isn't fully functioning and grasping it yet. But that should easily be remedied.


And as for the Set function...making sure i follow properly, that is supposed to take the place of the "if else" clause, right?
Right. The Set function would supplement the On and Off functions for when you have a boolean indicating that you want On or Off.