Seeking better testing language

define# TURN_ON 0
define TURN_OFF 1

Spot the difference

Neither of which is correct, of course.

  if (now.hour() == 9 && now.minute() >= 0 && now.minute() < 10) // FeedPump1, ON 9am
  {  
...
  }
  else // Turn off time for FeedPump1, OFF 9:10am
  {  
    digitalWrite(Relay_B, TURN_OFF);
  }
  //******************* 2nd FEED TIME - RELAY B *********************************************//
  if (now.hour() == 18 && now.minute() >= 0 && now.minute() < 10) // FeedPump, ON 6PM
  {  
...
  }
 else  
  {
  digitalWrite(Relay_B, TURN_OFF);
  }

The net effect, surely, is that Relay_B will always be turned off. The hour can't be be both 9 and 18.

Let me put it like this … if I said:

If it is not 9 o’clock, slap yourself.
If it is not 6 pm, slap yourself.

Are you going to get a slap? If you aren’t sure, try it out. You might even get two slaps. :wink:

UKHeliBob: Spot the difference

Aren't they BOTH wrong?

Shouldn't it be #define ?

You might even get two slaps

or just continuous slaps ?

If it was my project I would test for the HOUR and then in a subsidiary test check the MINUTES just so I could understand what I was doing Something like

if (HOUR == 9) {
  if (MINUTE >0 && MINUTE < 10) {
    // do whatever
  }
}

...R

They way it is written, yes.

Aren't they BOTH wrong?

Indeed.

myggle:
However, when I get to my second alarm, I fear my knowledge of the code is failing me and keeping me from asking the right question(s) of the RTC.

Perhaps use the IPO principle and do a logical seperation of:

  • Input
  • Processing
  • Output

Do not try to mix input with just a small part of the processing logic and then output directly before all processing is done.

For your relay switching I’d suggest a programming logic like that looping around in the loop() function (untested code):

  // Input ==> read the current time from RTC

  // Processing
  boolean relayBstate=false; // initial guess is "relay B is OFF"
  if (now.hour() == 9 && now.minute() >= 00 && now.minute() < 10) relayBstate=true; // 1st 'ON' time
  if (now.hour() == 12 && now.minute() >= 00 && now.minute() < 10) relayBstate=true; // 2nd 'ON' time
  if (now.hour() == 15 && now.minute() >= 00 && now.minute() < 10) relayBstate=true; // 3nd 'ON' time

  // Output
  if (relayBstate==true)
  {
    if (digitalRead(Relay_B)!=TURN_ON) Serial.println("Turn ON relay B");
    digitalWrite(Relay_B, TURN_ON);
  }
  else
  {
     if (digitalRead(Relay_B)!=TURN_OFF) Serial.println("Turn OFF relay B");
     digitalWrite(Relay_B, TURN_OFF);
  }

Thanks for the feedback. I pasted the snippet about the test and mistyped the includes by mistake with hopes I would not have to explain why they were defined in the first place. Yet here I am again explaining them for other reasons. I was hoping someone would point out the flaws in my test(s) so I can learn from my mistake(s), but I guess not.

Thank you jurs for at least giving me a different way of doing things, I will start testing immediately.

Edit - @jurs, can you explain the purpose of using the NOT operator? I see that it is used in many places throughout the example sketches I've been through and never understood why it is important to ask in code if something is NOT, when the code never told it to be? Or why is the code testing to see if the light was turned on before the mention of the command to turn it on?

TYIA

myggle: Thanks for the feedback. I pasted the snippet about the test and mistyped the includes by mistake with hopes I would not have to explain why they were defined in the first place. Yet here I am again explaining them for other reasons. I was hoping someone would point out the flaws in my test(s) so I can learn from my mistake(s), but I guess not.

Thank you jurs for at least giving me a different way of doing things, I will start testing immediately.

Well, that can happen when you ignore the very first response.

myggle: Edit - @jurs, can you explain the purpose of using the NOT operator?

The NOT operator will do a little "state change detection" before the OUTPUT is set to its value.

As I have seen in your source code, you want to have a small message on Serial each time the state changes.

So with this code:

  if (digitalRead(Relay_B)!=TURN_ON) Serial.println("Turn ON relay B");
  digitalWrite(Relay_B, TURN_ON);

I first check if the relay is already in the ON state, and only if it is NOT in ON state, I show the message. Then it is set to ON state.

Same check happens when turning to OFF state.

I just tried a few combinations with, and without the not operator statement, and the result was the same. I even relocated the whole block to occur before the Relay_A test, and the resulted in Relay_B not even pulsing high momentarily. So I’m at a loss as to why this is the result.

The funny thing is that this was happening when I was using the Mega with an Ethernet shield on it, but now I am using an EtherMega which rules out hardware issues. To the best of your knowledge, can you spot why Relay_B is only pulsing HIGH and not steadily HIGH? Some weeks ago I tested to see if the relay channel was faulty by running the blink sketch for pins 30-37, and all pins went to HIGH for the duration I specified, so I know it has to be something in my code.

@aarg, I did not ignore the first response, I attached my sketch as an attachment because it is too many characters to fit into code brackets.

I will attach an updated version of my sketch as I added the boolean tests that jurs suggested, and modified the println language to be generic to the channel and not the event. I also cut out the second block for Relay_B as the boolean test can incorporate other tests.

Phase_1.ino (8.78 KB)

  if (now.hour() >= 6 && now.minute() >= 0) {  // Turn on time for Veg Lights, ON at 6am
    digitalWrite(Relay_A, TURN_ON);

    Serial.print("\t");
    Serial.println(F("Vegetative Lights On"));  //  Text printed to serial monitor
    Serial.print("\t");
  }
  else if (now.hour() >= 0 && now.minute() >= 0) // "else if" was needed to create opposition to lights on
  {  // Turn off time for Veg Lights, OFF at 12am
    digitalWrite(Relay_A, TURN_OFF);
    Serial.print("\t");
    Serial.println(F("Vegitative Lights OFf"));  //  Text printed to serial monitor
    Serial.print("\t");
  }

Can you envision a scenario where the hour value is 6 or more, and the minute value is not greater than or equal to 0?

Can you envision a scenario where the else if test would not evaluate to true?

PaulS: ```   if (now.hour() >= 6 && now.minute() >= 0) {  // Turn on time for Veg Lights, ON at 6am     digitalWrite(Relay_A, TURN_ON);

    Serial.print("\t");     Serial.println(F("Vegetative Lights On"));  //  Text printed to serial monitor     Serial.print("\t");   }   else if (now.hour() >= 0 && now.minute() >= 0) // "else if" was needed to create opposition to lights on   {  // Turn off time for Veg Lights, OFF at 12am     digitalWrite(Relay_A, TURN_OFF);     Serial.print("\t");     Serial.println(F("Vegitative Lights OFf"));  //  Text printed to serial monitor     Serial.print("\t");   }



Can you envision a scenario where the hour value is 6 or more, and the minute value is not greater than or equal to 0?

*Hours count from 0-23, minutes 0-59. So if hours are 6-23 and minutes are 0-59, the tests becomes true and Relay_A is HIGH*

Can you envision a scenario where the else if test would not evaluate to true?

*If for example the now.hour is 3, this else test is false as the now.hour is outside of the true state as depicted above.*

The test for Relay_A does work flawlessly, but it will only be tested true once every 24 hours and for a duration of 18 hours. Relay_B will test true no less than twice daily, for the 10 minute period I desire. Initially I wanted for individualized text outputs on the serial monitor which then justified the need to break up the 24 hour time span for Relay_B into multiple tests that needed to be true in themselve only and false during other tests which further made for more complicated code I'm sure.

Now with using jurs scheme of using the boolean tests and my decision to change the output language to a one phrase fits all, the code no longer needs to reflect individualized blocks for individualized events. This all of course is predicated that I can find why Relay_B is only pulsing when it should be constant HIGH like that of Relay_A.

This all of course is predicated that I can find why Relay_B is only pulsing when it should be constant HIGH like that of Relay_A.

Do the pulses match the serial output? If not, you have a hardware problem that no amount of software tweaking is going to fix.

PaulS: Do the pulses match the serial output? If not, you have a hardware problem that no amount of software tweaking is going to fix.

I don't know what serial output is but the serial monitor shows the text with every iteration of the loop, the same as it does for the Relay_A condition which is true 75% of the time. I just tested the hardware with this sketch;

void setup() {
  // initialize digital pin 13 as an output.
  pinMode(30, OUTPUT);
  pinMode(31, OUTPUT);
}

// the loop function runs over and over again forever
void loop() {
  digitalWrite(30, HIGH);   // turn the LED on (HIGH is the voltage level)
  digitalWrite(31, HIGH);
  delay(1000);              // wait for a second
  digitalWrite(30, LOW);    // turn the LED off by making the voltage LOW
  digitalWrite(31, LOW);
  delay(3000);              // wait for a second
}

... and it resulted in the relays turning on the test lights for 3 seconds and off for 1 second as expected, so it has to be code?

I don't know what serial output is but the serial monitor shows

The Serial Monitor is an application that runs on the PC. It displays serial data that the Arduino output.

Since the simple sketch works, add to it.

Put all the code for printing the time to the serial port in a function. Call that function from loop(). Do NOT clutter up loop with a bazillion lines of code to print the time.

Get rid of the useless curly braces and the useless comments. Everybody knows that delay() causes a delay. It really isn't necessary to add a comment that says that delay() delays.

myggle: This all of course is predicated that I can find why Relay_B is only pulsing when it should be constant HIGH like that of Relay_A.

You don't try to drive a relay directly from the 20mA that an Arduino output pin can deliver, don't you?

When you write 'relay' you mean something like 'relay driving transistor' or 'circuit on a relay board' or something like that?

Except of very small "reed relays" with a "extra sensitive rating" you will need much more current than 20mA to switch a relay.

I have a Sainsmart 8 channel Relay Module that is rated to be driven from Arduino's source capabilities. I do intend to ultimately put it on external power supply, but b/c of a shortcoming with EtherMega, I need to keep it with USB power for the time being. I do wish to point out that I am able to set all 8 channels to HIGH, as in the blink sketch, and all 8 relays change states as desired, but for some reason, channel 2 only clicks to produce the pulse when used in the Phase_1 sketch. Here is a pic of my AC wiring to help convey what I built thus far. |500x375 After this pic was taken, I put the connecting wires from N.O. to each receptacle. Everything is 14 AWG.

PaulS: The Serial Monitor is an application that runs on the PC. It displays serial data that the Arduino output.

Since the simple sketch works, add to it.

Put all the code for printing the time to the serial port in a function. Call that function from loop(). Do NOT clutter up loop with a bazillion lines of code to print the time.

Get rid of the useless curly braces and the useless comments. Everybody knows that delay() causes a delay. It really isn't necessary to add a comment that says that delay() delays.

Sorry, I'm still too new to code and languages, I don't yet know what constitutes as a useless curly brace. Useless comments I will nix. Will putting all of time blocks into a function make the sketch smaller or run smoother?

Will putting all of time blocks into a function make the sketch smaller or run smoother?

No. But, if you say that printing the time works fine, then we don't need to look, at the code to see what the "not working fine" code is doing.

As for the "useless curly braces" comment, sorry about that. It was code in another thread that had them.

myggle: for some reason, channel 2 only clicks to produce the pulse when used in the Phase_1 sketch.

Did you already post the full code of your "Phase_1 sketch"? Which is the sketch that causes the faulty relay switching?

I pasted the snippet about the test and mistyped the includes by mistake with hopes I would not have to explain why they were defined in the first place. Yet here I am again explaining them for other reasons.

The point is, that when you post code that clearly doesn’t compile, we wonder whether you retyped the whole thing. In other words, are we looking at the code you actually tested? Maybe you reversed a > and a < somewhere? I don’t want to seem pedantic, but copying and pasting the whole sketch is the way to go.

http://snippets-r-us.com/


You seem to have ignored my comments about why it was doing what it was, but just to check I’ve looked at your newer code. Look at this:

 if (relayBstate == true)
  { 
    digitalWrite(Relay_B, TURN_ON);
    Serial.print("\t");
    Serial.println(F("Feeding the plants"));  //  Text printed to serial monitor
    Serial.print("\t");
  }
  else relayBstate = false;  // Turn off time for FeedPump1, OFF 9:10am
  {  
    digitalWrite(Relay_B, TURN_OFF);
  }

First, you normally don’t test a boolean for “== true” because this looks cleaner:

  if (relayBstate)

Now, you think “what does ‘true’ or ‘false’ mean for a relay”? It is like saying “is my door true or false?”.

Surely a better test would be:

 if (relayBClosed) ...

Now we know that “true” means “closed” and “false” means “not closed”.


Moving on …

 if (relayBstate == true)
  { 
  ...
  }
  else relayBstate = false;  // Turn off time for FeedPump1, OFF 9:10am

If the relayBstate is false, make it false. How does this do anything useful?


Next:

   if (relayBstate == true)
    { 
    ...
    }
  else 
     relayBstate = false;  // Turn off time for FeedPump1, OFF 9:10am


// this is always done ! ---------------------------
    {  
      digitalWrite(Relay_B, TURN_OFF);
    }

If you indented your code with the auto-indent tool you might have spotted this. Regardless of relayBstate you always turn the relay off. Presumably not what you intend. Those lines do not fall under the scope of the else.