First code

Whandall: If you know what the issue is, why ask us?

Common ground (or not) is not the issue. I've tried both setups with no visible change. The bug happens no matter the relays and arduino share a power supply or have different ones with or without common ground. I have no idea what's going on but at least that is not the issue. Don't take me wrong, I didn't mean to sound agressive or disrespectful. If that's the case, please forgive me.

UKHeliBob Im not sure about that, the 4 relay module is this one:

I suspect the issue is within the code but that's only a hunch.....

Morke: Common ground (or not) is not the issue. I've tried both setups with no visible change. The bug happens no matter the relays and arduino share a power supply or have different ones with or without common ground. I have no idea what's going on but at least that is not the issue. Don't take me wrong, I didn't mean to sound agressive or disrespectful. If that's the case, please forgive me.

UKHeliBob Im not sure about that, the 4 relay module is this one:

I suspect the issue is within the code but that's only a hunch.....

Do you hear clicks from it when engaging and disengaging, indicating the relays are mechanical?

I see the main block of pins containing GND, in1-4 and VCC. But there are three more pins, located by themselves. I See GND and VCC, but VCC is connected to a third pin, via a yellow jumper. What's written next to the third pin?

May it possibly be extra pins for supplying extra current, for driving the relays? My theory is still that your Arduino struggles with delivering the amount of current required to operate these. The jumper I suspect is used to select VCC from control-inputs row, for devices supplying enough current to drive the relays.

Rupert909: Do you hear clicks from it when engaging and disengaging, indicating the relays are mechanical?

I see the main block of pins containing GND, in1-4 and VCC. But there are three more pins, located by themselves. I See GND and VCC, but VCC is connected to a third pin, via a yellow jumper. What's written next to the third pin?

May it possibly be extra pins for supplying extra current, for driving the relays? My theory is still that your Arduino struggles with delivering the amount of current required to operate these. The jumper I suspect is used to select VCC from control-inputs row, for devices supplying enough current to drive the relays.

Yes, I can hear the clicks.

The three more pins are for the external power supply

here's how the relays are powered:

|500x164

Morke: Yes, I can hear the clicks.

The three more pins are for the external power supply

here's how the relays are powered:

|500x164

No GND connection from Arduino?? Why not?

Rupert909: No GND connection from Arduino?? Why not?

To use opto-isolation. The control lines provide path to GND when switched low.

The following is a connection diagram and some test code. The operational and load test will sequentially turn on all relays then sequentially turn off all relays. Print monitor will display what part of the loop is running.

If you’re still having USB problems, could try a different USB port and new or different USB cable.

Note that some USB ports might be current limited 100mA, 250mA, 500mA or other. The only way to guarantee 500mA is to use a USB hub with power adapter connected.

Test code:

/* 4 Relay Module configured for Opto-isolation
   Arduino pins 3 to 6 connected to IN1 to IN4
   Connections: http://i.imgur.com/6m95zlV.png */

void setup() {
  Serial.begin(115200);
  Serial.println("In sequence:");

  for (int i = 3; i <= 6; i++) {
    pinMode(i, INPUT_PULLUP);
    pinMode(i, OUTPUT); // defaults HIGH (relays off)
  }
}

void loop() {
  Serial.println("Relays turning ON ...");
  for (int i = 3; i <= 6; i++) {
    digitalWrite(i, LOW); // energize relays until all on
    delay(1000);
  }
  Serial.println("Relays turning OFF ...");
  for (int i = 3; i <= 6; i++) {
    digitalWrite(i, HIGH); // de-energize relays until all off
    delay(1000);
  }
}

I give up. :astonished:

Re-worked your code using arrays, faster printing (115200), only print once per second, added conditional temp limits for relay control and averaged readings (untested):

void loop()
{
  // readings and control
  for (int i = 0; i < 4; i++) {
    sum[i] = analogRead(tempPin[i]);  // invalid
    sum[i] = 0;
    sum[i] += analogRead(tempPin[i]); // reading 1
    sum[i] += analogRead(tempPin[i]); // reading 2
    sum[i] += analogRead(tempPin[i]); // reading 3
    sum[i] += analogRead(tempPin[i]); // reading 4
    reading[i] = sum[i] / 4;          // averaged reading
    tempC[i] = reading[i] / 9.31;
    if (tempC[i] < lowerLimit) {
      digitalWrite(relayPin[i], LOW);   //relay OFF
    }
    else if (tempC[i] > upperLimit) {
      digitalWrite(relayPin[i], HIGH);  // relay ON
    }
  }
  printCheck = millis() - lastPrintTime;
  if (printCheck >= printInterval) {
    for (int i = 0; i < 4; i++) {
      Serial.print("tempC");
      Serial.print(i + 1);
      Serial.print(" ");
      Serial.println(tempC[i]);
    }
    Serial.println();
    lastPrintTime = millis(); // reset print timer
  }
}

dlloyd:
Re-worked your code using arrays, faster printing (115200), only print once per second, added conditional temp limits for relay control (untested):

const byte tempPin[] = {A1, A2, A3, A4};

const byte relayPin = {6, 7, 8, 9};

// hysteresis = upperLimit - lowerLimit
const byte lowerLimit = 24;
const byte upperLimit = 26;

float tempC[4];
word reading[4];

word printInterval = 1000; // 1 second
unsigned long printCheck = 0, lastPrintTime = 0;

void setup()
{
  analogReference(INTERNAL);
  Serial.begin(115200);
  for (int i = 0; i < 4; i++) {
    pinMode(relayPin[i], INPUT_PULLUP);
    pinMode(relayPin[i], OUTPUT); // defaults HIGH, relay OFF
  }
}

void loop()
{
  // readings and control
  for (int i = 0; i < 4; i++) {
    reading[i] = analogRead(tempPin[i]);
    tempC[i] = reading[i] / 9.31;
    if (tempC[i] < lowerLimit) {
      digitalWrite(relayPin[i], LOW);  //relay OFF
    }
    else if (tempC[i] > upperLimit) {
      digitalWrite(relayPin[i], HIGH);  // relay ON
    }
  }
  printCheck = millis() - lastPrintTime;
  if (printCheck >= printInterval) {
    for (int i = 0; i < 4; i++) {
      Serial.print(“tempC”);
      Serial.print(i + 1);
      Serial.print(" ");
      Serial.println(tempC[i]);
    }
    Serial.println();
    lastPrintTime = millis(); // reset print timer
  }
}



Now I give up ;)

Woah, that’s a lot of work. First of all thank you. I’ll test it. I’m quite a newbie in programming so i’m struggling to understand all of the code.

But what I don’t get is this part:

    pinMode(relayPin[i], INPUT_PULLUP);
    pinMode(relayPin[i], OUTPUT); // defaults HIGH, relay OFF

Why do you set the relay pin first to INPUT and right after that as OUTPUT?

The INPUT_PULLUP functionality is activated by writing a HIGH to the output register. So the Arduino output will start with a HIGH level, which is the relay off state.

Whandall: The INPUT_PULLUP functionality is activated by writing a HIGH to the output register. So the Arduino output will start with a HIGH level, which is the relay off state.

Oh, ok, so, the code uses INPUT_PULLUP to turn off all the relays the first time it runs, and then sets them as OUTPUT It's like using digitalwrite(relay, HIGH) ?

It writes to the port like digitalWrite() does.

void pinMode(uint8_t pin, uint8_t mode)
{
    uint8_t bit = digitalPinToBitMask(pin);
    uint8_t port = digitalPinToPort(pin);
    volatile uint8_t *reg, *out;

    if (port == NOT_A_PIN) return;

    // JWS: can I let the optimizer do this?
    reg = portModeRegister(port);
    out = portOutputRegister(port);

    if (mode == INPUT) { 
        uint8_t oldSREG = SREG;
                cli();
        *reg &= ~bit;
        *out &= ~bit;
        SREG = oldSREG;
    } else if (mode == INPUT_PULLUP) {
        uint8_t oldSREG = SREG;
                cli();
        *reg &= ~bit;
        *out |= bit;
        SREG = oldSREG;
    } else {
        uint8_t oldSREG = SREG;
                cli();
        *reg |= bit;
        SREG = oldSREG;
    }
}
void digitalWrite(uint8_t pin, uint8_t val)
{
    uint8_t timer = digitalPinToTimer(pin);
    uint8_t bit = digitalPinToBitMask(pin);
    uint8_t port = digitalPinToPort(pin);
    volatile uint8_t *out;

    if (port == NOT_A_PIN) return;

    // If the pin that support PWM output, we need to turn it off
    // before doing a digital write.
    if (timer != NOT_ON_TIMER) turnOffPWM(timer);

    out = portOutputRegister(port);

    uint8_t oldSREG = SREG;
    cli();

    if (val == LOW) {
        *out &= ~bit;
    } else {
        *out |= bit;
    }

    SREG = oldSREG;
}

digitalwrite(relay, HIGH); Seems a bit more intuitive.

.

LarryD: digitalwrite(relay, HIGH); Seems a bit more intuitive.

Absolutely.

Morke: Oh, ok, so, the code uses INPUT_PULLUP to turn off all the relays the first time it runs, and then sets them as OUTPUT It's like using digitalwrite(relay, HIGH) ?

Simply, the two configuration methods described prevent the relay pins from temporarily going LOW prior to entering the main loop. A relay pin temporarily going LOW could cause the relay to temporarily energize during setup.

Using one of the recommended setup methods, the pin does this:

  1. Most pins are initially configured as INPUT (high impedance, floating). Relay remains OFF
  2. Then the internal pullup gets enabled. The pin is weakly pulled to 5V. Relay remains OFF
  3. Then the pin gets configured as OUTPUT. The pin is driven to 5V (the new default). Relay remains OFF

Now all relays are under your control in the main loop.

Ok so verifying the code this error pops up

Arduino: 1.6.8 (Windows 7), Board: "Arduino/Genuino Uno"

 In function 'void setup()':

temprelayfinal2:42: error: a function-definition is not allowed here before '{' token

 {

 ^

temprelayfinal2:75: error: expected '}' at end of input

 }

 ^

exit status 1
a function-definition is not allowed here before '{' token

This report would have more information with
"Show verbose output during compilation"
option enabled in File -> Preferences.

And it shows a red line here:

void setup()
[color=red]{[/color]
  analogReference(INTERNAL);
  Serial.begin(115200);
  for (int i = 0; i < 4; i++) {
    pinMode(relayPin[i], INPUT_PULLUP);
    pinMode(relayPin[i], OUTPUT); // defaults HIGH, relay OFF
  }
}

And the code above the failing definition of setup() has the error.

nvm i found the error

Would it be possible to add an array to make the relays work on Avg temperatures to smooth the readings?

Averaging might not be needed. Try adding an extra reading so there's 2 readings in a row. The first will be overwritten. The second will be more accurate because there's been a bit more time for the ADC value to settle.

reading[i] = analogRead(tempPin[i]);
reading[i] = analogRead(tempPin[i]);

EDIT: added above and some simple averaging for readings in code in reply#27.