More troubles.. Why didn't it go in to feeding mode?

[lnevo]

No feeding mode this morning.

Mode... Not feeder.

But even if its an autofeeder. It usually is a hack to close the relay. It usually will only trigger one feeding for a button press so no need to have a timer. If it holds the button for one second or one minute it should be fine. At least with an eheim.

[Smotz]

thanks all. I will remove the seconds from the feed mode and see if that helps. Yes the other part is the autofeeder and that doesn't seem to be a problem.

[re76]

Give this a shot, but keep in mind that if it was in fact the problem, you might now have the same exact problem with the feeder too since you are also using seconds in those lines! This is the point I was trying to make. It would be really bad if your feeder turned on but you happened to miss the specific second where you are trying to turn it off, and then it never turns off!

Make sure you keep an eye on this! At a minimum I would change the line where you turn the feeder off to be a greater than or equal check on seconds, that way you know you will be guaranteed to turn the feeder off. I would rather miss turning the feeder on than miss turning it off and pouring a ton of food into the tank.

[lnevo]

Look at the function I have for my Swabbie and autofeeder. It follows what re76's example is doing too

[Smotz]

Give this a shot, but keep in mind that if it was in fact the problem, you might now have the same exact problem with the feeder too since you are also using seconds in those lines! This is the point I was trying to make. It would be really bad if your feeder turned on but you happened to miss the specific second where you are trying to turn it off, and then it never turns off!

Make sure you keep an eye on this! At a minimum I would change the line where you turn the feeder off to be a greater than or equal check on seconds, that way you know you will be guaranteed to turn the feeder off. I would rather miss turning the feeder on than miss turning it off and pouring a ton of food into the tank.

Odd because it worked for years on my other (PWM) RA. As a matter of fact, it still does. I think this new RA may be 'sluggish' - is that even possible ?

[Smotz]

Look at the function I have for my Swabbie and autofeeder. It follows what re76's example is doing too

Dude, your code is too advanced. I could barely read it.

[re76]

Give this a shot, but keep in mind that if it was in fact the problem, you might now have the same exact problem with the feeder too since you are also using seconds in those lines! This is the point I was trying to make. It would be really bad if your feeder turned on but you happened to miss the specific second where you are trying to turn it off, and then it never turns off!

Make sure you keep an eye on this! At a minimum I would change the line where you turn the feeder off to be a greater than or equal check on seconds, that way you know you will be guaranteed to turn the feeder off. I would rather miss turning the feeder on than miss turning it off and pouring a ton of food into the tank.

Odd because it worked for years on my other (PWM) RA. As a matter of fact, it still does. I think this new RA may be 'sluggish' - is that even possible ?

Have you added more extensions and functionality since then?

[Smotz]

Give this a shot, but keep in mind that if it was in fact the problem, you might now have the same exact problem with the feeder too since you are also using seconds in those lines! This is the point I was trying to make. It would be really bad if your feeder turned on but you happened to miss the specific second where you are trying to turn it off, and then it never turns off!

Make sure you keep an eye on this! At a minimum I would change the line where you turn the feeder off to be a greater than or equal check on seconds, that way you know you will be guaranteed to turn the feeder off. I would rather miss turning the feeder on than miss turning it off and pouring a ton of food into the tank.

It 'should' never dump the feeder continually. The relay is a "button push" - the feeder rotates once upon pressing it.

[Smotz]

Give this a shot, but keep in mind that if it was in fact the problem, you might now have the same exact problem with the feeder too since you are also using seconds in those lines! This is the point I was trying to make. It would be really bad if your feeder turned on but you happened to miss the specific second where you are trying to turn it off, and then it never turns off!

Make sure you keep an eye on this! At a minimum I would change the line where you turn the feeder off to be a greater than or equal check on seconds, that way you know you will be guaranteed to turn the feeder off. I would rather miss turning the feeder on than miss turning it off and pouring a ton of food into the tank.

Odd because it worked for years on my other (PWM) RA. As a matter of fact, it still does. I think this new RA may be 'sluggish' - is that even possible ?

Have you added more extensions and functionality since then?

Not sure I understand your question? I have been slowly moving the modules from my PWM RA to the Analog RA.

Standard stuff - 2 relay boxes, multi water level, expansion hub, wifi, bluetooth

[re76]

Give this a shot, but keep in mind that if it was in fact the problem, you might now have the same exact problem with the feeder too since you are also using seconds in those lines! This is the point I was trying to make. It would be really bad if your feeder turned on but you happened to miss the specific second where you are trying to turn it off, and then it never turns off!

Make sure you keep an eye on this! At a minimum I would change the line where you turn the feeder off to be a greater than or equal check on seconds, that way you know you will be guaranteed to turn the feeder off. I would rather miss turning the feeder on than miss turning it off and pouring a ton of food into the tank.

It 'should' never dump the feeder continually. The relay is a "button push" - the feeder rotates once upon pressing it.

That is good to know. I am not a Reef Angel expert by any means, and in most non-reef angel cases I've encountered setting an output high would leave it on until you set it low again. My background is in embedded software for medical devices though where everything is super strict so I have a tendency to be extra careful...

[Smotz]

Give this a shot, but keep in mind that if it was in fact the problem, you might now have the same exact problem with the feeder too since you are also using seconds in those lines! This is the point I was trying to make. It would be really bad if your feeder turned on but you happened to miss the specific second where you are trying to turn it off, and then it never turns off!

Make sure you keep an eye on this! At a minimum I would change the line where you turn the feeder off to be a greater than or equal check on seconds, that way you know you will be guaranteed to turn the feeder off. I would rather miss turning the feeder on than miss turning it off and pouring a ton of food into the tank.

It 'should' never dump the feeder continually. The relay is a "button push" - the feeder rotates once upon pressing it.

That is good to know. I am not a Reef Angel expert by any means, and in most non-reef angel cases I've encountered setting an output high would leave it on until you set it low again. My background is in embedded software for medical devices though where everything is super strict so I have a tendency to be extra careful...

Well, your help is very much appreciated. I think you nailed it when you said it is 'missing the second'. I suspect this RA may be 'sluggish' or something.

[re76]

To clarify a bit. The Reef Angel is based off of the Arduino platform. The Arduino tries to makes things simple for the end user by having only two important functions that are needed: a setup() function and loop() function.

The loop function gets called by the arduino as much as possible. If you have a really basic loop function then it might be able to call it 1000 times a second. It can't call it again though until the loop function is finished from the last time it was called. So as your loop() gets more and more code in it it will be getting called less times every second.

Not to mention for every new extension you add to the RA there is more library code to support it. As an end user you don't see this a lot unless you go looking for it. This is why I asked if you had added more extensions or features. As you add complexity to your code it is going to run a bit slower. This isn't necessarily a bad thing, it just is. It is a standard understood trade off for all embedded systems whether they are using Arduino or not.

There is a concept in software development called defensive programming. It is the idea that you make your code robust enough to handle unforeseen conditions. This is why I am a big advocate of using >= or <= instead of == wherever possible. You sometimes have to work your logic a bit differently but it usually isn't a lot more work, and it ensures that if for some crazy reason you don't catch something on the first loop through you will get it on the second time around, and will never just miss it totally like you can with ==.

Hope this makes sense and isn't just causing more confusion!

[lnevo]

Your code was good just sometimes we foresake complexity for ease of troubleshooting and readability welcome to the club

[Smotz]

To clarify a bit. The Reef Angel is based off of the Arduino platform. The Arduino tries to makes things simple for the end user by having only two important functions that are needed: a setup() function and loop() function.

The loop function gets called by the arduino as much as possible. If you have a really basic loop function then it might be able to call it 1000 times a second. It can't call it again though until the loop function is finished from the last time it was called. So as your loop() gets more and more code in it it will be getting called less times every second.

Not to mention for every new extension you add to the RA there is more library code to support it. As an end user you don't see this a lot unless you go looking for it. This is why I asked if you had added more extensions or features. As you add complexity to your code it is going to run a bit slower. This isn't necessarily a bad thing, it just is. It is a standard understood trade off for all embedded systems whether they are using Arduino or not.

There is a concept in software development called defensive programming. It is the idea that you make your code robust enough to handle unforeseen conditions. This is why I am a big advocate of using >= or <= instead of == wherever possible. You sometimes have to work your logic a bit differently but it usually isn't a lot more work, and it ensures that if for some crazy reason you don't catch something on the first loop through you will get it on the second time around, and will never just miss it totally like you can with ==.

Hope this makes sense and isn't just causing more confusion!

Hi!

Working off your example and a good amount of reading I put the below code together in an effort to be more efficient. I am trying to set the feeding mode twice a day and bounce the feeding relay for 10 seconds.

I set "static time_t NoteTime=millis();" as a global variable so I can call it in other parts of the code.

Code: Select all

[code]//FEEDING MODE
// Begin Feeding Mode
    if ( ((hour() == 7)) && (minute() == 30) || ((hour() == 18)) && (minute() == 00)) {
      ReefAngel.FeedingModeStart(); // FEEDING MODE @ 7:30 AM or 6:00 PM
      
      NoteTime = millis(); 
      unsigned long DeltaTS = millis() - NoteTime;
      
      if ( DeltaTS >= 60000 ) ReefAngel.Relay.On(Feeder); // turn on after 1 minute
      if ( DeltaTS >= 70000 ) ReefAngel.Relay.On(Feeder); // turn off shortly after 
      if ( DeltaTS >= 120000 ) ReefAngel.Relay.On(Feeder); // turn on after 2 minutes
      if ( DeltaTS >= 130000 ) ReefAngel.Relay.On(Feeder); // turn off shortly after
    }

[/code]

[re76]

This is a really good start a few things though. I wouldn't use time_t, if you are intending it to store the output of a call to millis() you might want to change it to static unsigned long, which is what millis() returns. As far as I know time_t only has second accuracy and not millisecond accuracy so I'm not sure what your deltaTS will have in it after the subtraction since you are subtracting two different types.

Next up is your logic for the timing of the feeding relay. You should be using if-else-if statements. You only want one thing to happen every time, not multiple. As you have written above, if your DeltaTS is bigger than 130000 then all of those cases would trigger, which I don't think is what you want.

Take a look at the code I posted previously again and notice how I used several if-else-if statements. Also notice how I have the case with the biggest deltaTS first. That is on purpose so that it doesn't do multiple cases on each pass through he code.

Next, somewhere in this code you need to add a check to make sure you don't ALWAYS run this code. There are several ways to do this. You could use a Boolean variable that you set to true in your "begin feeding mode" code and then set to false in your last step where you turn your feeder off. Then wrap all of the feede logic in a big if statement that checks that variable.

Finally, and this is me being nitpicky, you probably don't need to be using a static variable here. All of the user code is in the arduino loop() function, you will probably never use NoteTime out of loop() so it would probably be fine to not be static. It is good practice to avoid static variables where possible.

Also You are always turning the feeder on. Some of those should be an off call.

Sorry I can't write any code examples for you, I am writing this on my phone. Let me know if I need to elaborate more about anything!


Sent from my iPhone using Tapatalk

[Smotz]

This is great! Learning..
You're right - I am always turning it on - that was a CTRL-C error

After reading what you wrote 3 times I now understand that all of my cases will trigger @ 130000. I will work on the else statements.

Regarding the use of the static global variable - I am actually going to use it a few times in the code - like for my ATO - I want to ensure it waits several seconds after feeding and waterchange mode before it checks for the ATO timeout. For instance, I have this code also:

Code: Select all

if (ReefAngel.WaterLevel.GetLevel(1) <= 25 && ReefAngel.DisplayedMenu!=WATERCHANGE_MODE)
   {
     NoteTime = millis(); 
     unsigned long DeltaTS = millis() - NoteTime;
     if ( DeltaTS >= 15000 )
   {    
       ReefAngel.Relay.Override(Return,0);
       ReefAngel.Relay.Override(Skimmer,0);
       ReefAngel.Relay.Override(UVlight,0);
       ReefAngel.Relay.Override(Feeder,0);
       ReefAngel.Relay.Override(CalcDose,0);
       ReefAngel.Relay.Override(AlkDose,0);
       ReefAngel.Relay.Override(Fugelight,1);
         pinMode(lowATOPin,OUTPUT);  //Buzzer
         digitalWrite(lowATOPin,HIGH); //Buzzer
    }}
// END EMERGENCY WATER LEVEL ROUTINE

[lnevo]

It doesn't need to be static if you set it every time to millis() but it may need to be global only if you are using it in other functions outside of loop. If you are setting it once and want it retained like a flag that you want to set if something happens then you'll want to use static.

Also time_t is an unsigned long so no difference here.

[re76]

It doesn't need to be static if you set it every time to millis() but it may need to be global only if you are using it in other functions outside of loop. If you are setting it once and want it retained like a flag that you want to set if something happens then you'll want to use static.

Also time_t is an unsigned long so no difference here.

You are correct that time_t is an unsigned long, and it will work in this scenario. It is intended however to work with the time.h library which stores the seconds since epoch (seconds since 1970), which is a lot of seconds. Hence why it is an unsigned long. The call to millis() returns the millisecond counter (also very big).

By making a new type just for the time library, it helps keep track of what the variable is intended to be used for. When I see someone using time_t to store the millisecond counter value, it makes me worried they will try to use it with the time library somewhere down the line and not understand why it isn't working.

It is fine to keep using it as long as you don't try to use it with any of the functions in time.h, but for readability and code clarity I still think unsigned long is the better choice.

[lnevo]

Fair point.

[Smotz]

here is my updated code with the else's incorporated

Code: Select all

//FEEDING MODE
// Begin Feeding Mode
    if (((hour() == 7)) && (minute() == 30) || (hour() == 18) && (minute() == 00)) {
      ReefAngel.FeedingModeStart(); // FEEDING MODE @ 7:30 AM or 6:00 PM
      NoteTime = millis(); 
      unsigned long DeltaTS = millis() - NoteTime;
      if ( DeltaTS >= 130000 ) ReefAngel.Relay.Off(Feeder); // turn off 2 minutes 10 secs 
      else
      if ( DeltaTS >= 120000 ) ReefAngel.Relay.On(Feeder); // turn on after 2 minutes
      else
      if ( DeltaTS >= 70000 ) ReefAngel.Relay.Off(Feeder); // turn off 1 min 10 secs 
      else
      if ( DeltaTS >= 60000 ) ReefAngel.Relay.On(Feeder); // turn on after 1 minute
      }

[re76]

here is my updated code with the else's incorporated

Code: Select all

//FEEDING MODE
// Begin Feeding Mode
    if (((hour() == 7)) && (minute() == 30) || (hour() == 18) && (minute() == 00)) {
      ReefAngel.FeedingModeStart(); // FEEDING MODE @ 7:30 AM or 6:00 PM
      NoteTime = millis(); 
      unsigned long DeltaTS = millis() - NoteTime;
      if ( DeltaTS >= 130000 ) ReefAngel.Relay.Off(Feeder); // turn off 2 minutes 10 secs 
      else
      if ( DeltaTS >= 120000 ) ReefAngel.Relay.On(Feeder); // turn on after 2 minutes
      else
      if ( DeltaTS >= 70000 ) ReefAngel.Relay.Off(Feeder); // turn off 1 min 10 secs 
      else
      if ( DeltaTS >= 60000 ) ReefAngel.Relay.On(Feeder); // turn on after 1 minute
      }

You are definitely on the right track. This will almost work, but you are only giving everything 1 minute to complete, even though you have 2 minute and 10 seconds worth of events programmed. Here is how I would change it:

Code: Select all

// Static bool to track if we are currently feeding or not
static boolean feedingActive = false;

//FEEDING MODE
// Begin Feeding Mode
      if (((hour() == 7)) && (minute() == 30) || (hour() == 18) && (minute() == 00)) {
          ReefAngel.FeedingModeStart(); // FEEDING MODE @ 7:30 AM or 6:00 PM
          NoteTime = millis(); // Grab millis value of when feeding mode started
          feedingActive = true; // We are entering feeding mode, so set to true
      }

      if (feedingActive) {
          unsigned long DeltaTS = millis() - NoteTime;
          if ( DeltaTS >= 130000 ) {
              ReefAngel.Relay.Off(Feeder); // turn off 2 minutes 10 secs
              feedingActive = false; // Feeding mode is now over, set to false
          } else
          if ( DeltaTS >= 120000 ) ReefAngel.Relay.On(Feeder); // turn on after 2 minutes
          else
          if ( DeltaTS >= 70000 ) ReefAngel.Relay.Off(Feeder); // turn off 1 min 10 secs 
          else
          if ( DeltaTS >= 60000 ) ReefAngel.Relay.On(Feeder); // turn on after 1 minute
      }

The code above would even allow you to use feedingActive elsewhere in your code, if you wanted to do things based on whether or not you are in feeding mode.

[lnevo]

For the record we don't need sub second tracking here. You could assign NoteTime=now() and DeltaTS being now()-NoteTime.

Then your comparisons would be clearer in seconds versus milli.

As I said earlier examples are in my code already to do this.

[Smotz]

here is my updated code with the else's incorporated

Code: Select all

//FEEDING MODE
// Begin Feeding Mode
    if (((hour() == 7)) && (minute() == 30) || (hour() == 18) && (minute() == 00)) {
      ReefAngel.FeedingModeStart(); // FEEDING MODE @ 7:30 AM or 6:00 PM
      NoteTime = millis(); 
      unsigned long DeltaTS = millis() - NoteTime;
      if ( DeltaTS >= 130000 ) ReefAngel.Relay.Off(Feeder); // turn off 2 minutes 10 secs 
      else
      if ( DeltaTS >= 120000 ) ReefAngel.Relay.On(Feeder); // turn on after 2 minutes
      else
      if ( DeltaTS >= 70000 ) ReefAngel.Relay.Off(Feeder); // turn off 1 min 10 secs 
      else
      if ( DeltaTS >= 60000 ) ReefAngel.Relay.On(Feeder); // turn on after 1 minute
      }

You are definitely on the right track. This will almost work, but you are only giving everything 1 minute to complete, even though you have 2 minute and 10 seconds worth of events programmed. Here is how I would change it:

Code: Select all

// Static bool to track if we are currently feeding or not
static boolean feedingActive = false;

//FEEDING MODE
// Begin Feeding Mode
      if (((hour() == 7)) && (minute() == 30) || (hour() == 18) && (minute() == 00)) {
          ReefAngel.FeedingModeStart(); // FEEDING MODE @ 7:30 AM or 6:00 PM
          NoteTime = millis(); // Grab millis value of when feeding mode started
          feedingActive = true; // We are entering feeding mode, so set to true
      }

      if (feedingActive) {
          unsigned long DeltaTS = millis() - NoteTime;
          if ( DeltaTS >= 130000 ) {
              ReefAngel.Relay.Off(Feeder); // turn off 2 minutes 10 secs
              feedingActive = false; // Feeding mode is now over, set to false
          } else
          if ( DeltaTS >= 120000 ) ReefAngel.Relay.On(Feeder); // turn on after 2 minutes
          else
          if ( DeltaTS >= 70000 ) ReefAngel.Relay.Off(Feeder); // turn off 1 min 10 secs 
          else
          if ( DeltaTS >= 60000 ) ReefAngel.Relay.On(Feeder); // turn on after 1 minute
      }

The code above would even allow you to use feedingActive elsewhere in your code, if you wanted to do things based on whether or not you are in feeding mode.

So I was looking at this code and doing some testing and learned that it doesn't compile because the second part declares "unsigned long DeltaTS = millis() - NoteTime;" based on the if/then condition above it. It says that NoteTime was not declared.

[lnevo]

You said you had NoteTime declared globally earlier. If not, just put unsigned long NoteTime either in the top of the function or in a global declaration.

unsigned long NoteTime;

[Smotz]

Yes, thanks. I took it out for some reason. I put it on top of the function.