Multiple functions into one

Projects General Guidance
1

Hi there. I have 16 relays and for each relay I had the same function

void r1change() {
  if (r1State == 0){
    Serial.println("R1 Before: ");
    Serial.println(r1State);
    digitalWrite(r1, r1State);
    r1State = !r1State;
    Serial.println("R1 After: ");
    Serial.println(r1State);
    relayPub(r1State, l1, "01");
  }
  else if (r1State == 1) {
    Serial.println("R1 Before: ");
    Serial.println(r1State);
    digitalWrite(r1, r1State);
    r1State = !r1State;
    Serial.println("R1 After: ");
    Serial.println(r1State);
    relayPub(r1State, l1, "01");
  } 
}

I am trying to convert into one

void relayChange(bool relayState, const int relay, String relayName) {
  if (relayState == 0){
    Serial.println(relayName+" Before: ");
    Serial.println(relayState);
    digitalWrite(relay, relayState);
    relayState = !relayState;
    Serial.println(relayName+" After: ");
    Serial.println(relayState);
    relayPub(relayState, l1, "01");
  }
  else if (relayState == 1) {
    Serial.println(relayName+" Before: ");
    Serial.println(relayState);
    digitalWrite(relay, relayState);
    relayState = !relayState;
    Serial.println(relayName+" After: ");
    Serial.println(relayState);
    relayPub(relayState, l1, "01");
  } 
}

and when passing variables

bool r1State = 1; //relay state
const int r1 =  42; //relay pin
const char* l1 = "light/light01/status";

I cannot change relayState when using function

relayChange(r1State, r1, "R1");

What do I have to change?

2

Hello
Yes you have.
Drop all information related to the relays into an object using the struct instruction and design a method or service to handle these data.
Have a nice day and enjoy coding in C++.

1 Like
3

in C and C++ function parameters are passed by value - any changes made to parameters inside the function (such as relayState) are not passed back to calling program
you should use pass by reference which enables changes made to parameter values within the function to be passed back to the calling program
e.g. try passing relayState by reference

void relayChange(bool &relayState, const int relay, String relayName)
1 Like
4

Seems to be an application to use one or more arrays.

1 Like
5

Change the function as @horace details, and no changes to how it is called.

The other advice is for next steps, put one feather in you cap for doing this solid improvement, recognizing the issue and getting a solution.

a7

1 Like
6

I suspect that you need another parameter for l1 or the call to relayPub will be wrong for the other relays.

7

I notice that the code executed for "if (relayState == 0) {" is identical to the code executed for "if (relayState == 1) {" so you can get rid of the 'if' statements:

Note: Shouldn't you change the state BEFORE you write the new state to the pin?

    relayState = !relayState;
    digitalWrite(relay, relayState);
1 Like
8

That works and was marked as a solution. Thank you

9

@horace solution worked but I started to watch tutorials on arrays as well.
How would you approach that using arrays?

10

I changed it and it worked.
As to putting one feather in my cap I will do that but I was kinda forced into rewriting my code as the memory in my Arduino Mega was over 91% full and I had warning about issues with memory :slight_smile:

11

It worked :slight_smile:

12

I can only mark one post as a solution and I have to give credits to @horace as he was first and his idea worked.

Thank you for rewriting the code.

I think I was changing the state AFTER as relays are High when the PIN is LOW and other way around.

I could use !rState in digitalWrite(r16, !r16State) instead and I might do that in the future

13

This topic was automatically closed 180 days after the last reply. New replies are no longer allowed.