Help to optimize my sketch



  • Hi,

    I've wrote some bad code and I'm sure that someone here can help me to optimize it. I'm still begginer.

    Goal:

    • Serial gateway for Domoticz system without any wireless nodes
    • running on Arduino Mega 2560
    • 8 relays with digital input buttons
    • 8 digital input for door contact (currently 3 implemented)
    • temperature sensord on 1 wire in future
    
    
    // Enable debug prints to serial monitor
    #define MY_DEBUG 
    
    
    // Enable and select radio type attached
    //#define MY_RADIO_NRF24
    //#define MY_RADIO_RFM69
    
    // Set LOW transmit power level as default, if you have an amplified NRF-module and
    // power your radio separately with a good regulator you can turn up PA level. 
    //#define MY_RF24_PA_LEVEL RF24_PA_LOW
    
    // Enable serial gateway
    #define MY_GATEWAY_SERIAL
    
    // Define a lower baud rate for Arduino's running on 8 MHz (Arduino Pro Mini 3.3V & SenseBender)
    #if F_CPU == 8000000L
    #define MY_BAUD_RATE 38400
    #endif
    
    // Flash leds on rx/tx/err
    //#define MY_LEDS_BLINKING_FEATURE
    // Set blinking period
    #define MY_DEFAULT_LED_BLINK_PERIOD 300
    
    // Inverses the behavior of leds
    // #define MY_WITH_LEDS_BLINKING_INVERSE
    
    // Enable inclusion mode
    #define MY_INCLUSION_MODE_FEATURE
    // Enable Inclusion mode button on gateway
    #define MY_INCLUSION_BUTTON_FEATURE
    
    // Inverses behavior of inclusion button (if using external pullup)
    //#define MY_INCLUSION_BUTTON_EXTERNAL_PULLUP
    
    // Set inclusion mode duration (in seconds)
    #define MY_INCLUSION_MODE_DURATION 120 
    // Digital pin used for inclusion mode button
    #define MY_INCLUSION_MODE_BUTTON_PIN  3 
    
    // Flash leds on rx/tx/err
    #define MY_DEFAULT_ERR_LED_PIN 22  // Error led pin
    #define MY_DEFAULT_RX_LED_PIN  24  // Receive led pin
    #define MY_DEFAULT_TX_LED_PIN  26  // the PCB, on board LED
    
    #include <SPI.h>
    #include <MySensors.h>  
    #include <Bounce2.h>
    
    // Enable repeater functionality for this node
    #define MY_REPEATER_FEATURE
    
    //=RELAY=
    #define RELAY_0  4  // Arduino Digital I/O pin number for first relay (second on pin+1 etc)
    #define RELAY_1  5
    #define RELAY_2  6
    #define RELAY_3  7
    #define RELAY_4  8
    #define RELAY_5  9
    #define RELAY_6  10
    #define RELAY_7  11
    #define NUMBER_OF_RELAYS 8 // Total number of attached relays
    #define RELAY_ON 0  // GPIO value to write to turn on attached relay
    #define RELAY_OFF 1 // GPIO value to write to turn off attached relay
    
    #define BUTTON0_PIN A0
    #define BUTTON1_PIN A1
    #define BUTTON2_PIN A2
    #define BUTTON3_PIN A3
    #define BUTTON4_PIN A4
    #define BUTTON5_PIN A5
    #define BUTTON6_PIN A6
    #define BUTTON7_PIN A7
    //=RELAY=
    
    //=DIGITAL_INPUT=
    #define CHILD_ID_DIGITAL8 10
    #define CHILD_ID_DIGITAL9 11
    #define CHILD_ID_DIGITAL10 12
    #define BUTTON8_PIN  A8  // Arduino Digital I/O pin for button/reed switch
    #define BUTTON9_PIN  A9  // Arduino Digital I/O pin for button/reed switch
    #define BUTTON10_PIN A10  // Arduino Digital I/O pin for button/reed switch
    
    int oldValue8=-1;
    int oldValue9=-1;
    int oldValue10=-1;
    
    // Initialize message
    MyMessage msg8(CHILD_ID_DIGITAL8, V_TRIPPED);
    MyMessage msg9(CHILD_ID_DIGITAL9, V_TRIPPED);
    MyMessage msg10(CHILD_ID_DIGITAL10, V_TRIPPED);
    //=DIGITAL_INPUT=
    
    
    void before() { 
      for (int sensor=1, pin=RELAY_0; sensor<=NUMBER_OF_RELAYS;sensor++, pin++) {
        // Then set relay pins in output mode
        pinMode(pin, OUTPUT);   
        // Set relay to last known state (using eeprom storage) 
        digitalWrite(pin, loadState(sensor)?RELAY_ON:RELAY_OFF);
      }
    }
    Bounce debouncer0 = Bounce();
    Bounce debouncer1 = Bounce();
    Bounce debouncer2 = Bounce();
    Bounce debouncer3 = Bounce();
    Bounce debouncer4 = Bounce();
    Bounce debouncer5 = Bounce();
    Bounce debouncer6 = Bounce();
    Bounce debouncer7 = Bounce();
    Bounce debouncer8 = Bounce();
    Bounce debouncer9 = Bounce();
    Bounce debouncer10 = Bounce();
    void setup() { 
      // Setup locally attached sensors
      delay(5000);
       // Setup the button.
      pinMode(BUTTON0_PIN, INPUT_PULLUP);
      pinMode(BUTTON1_PIN, INPUT_PULLUP);
      pinMode(BUTTON2_PIN, INPUT_PULLUP);
      pinMode(BUTTON3_PIN, INPUT_PULLUP);
      pinMode(BUTTON4_PIN, INPUT_PULLUP);
      pinMode(BUTTON5_PIN, INPUT_PULLUP);
      pinMode(BUTTON6_PIN, INPUT_PULLUP);
      pinMode(BUTTON7_PIN, INPUT_PULLUP);
      pinMode(BUTTON8_PIN, INPUT_PULLUP);
      pinMode(BUTTON9_PIN, INPUT_PULLUP);
      pinMode(BUTTON10_PIN, INPUT_PULLUP);
      
      // After setting up the button, setup debouncer.
      debouncer0.attach(BUTTON0_PIN);
      debouncer0.interval(5);
      debouncer1.attach(BUTTON1_PIN);
      debouncer1.interval(5);
      debouncer2.attach(BUTTON2_PIN);
      debouncer2.interval(5);
      debouncer3.attach(BUTTON3_PIN);
      debouncer3.interval(5);
      debouncer4.attach(BUTTON4_PIN);
      debouncer4.interval(5);
      debouncer5.attach(BUTTON5_PIN);
      debouncer5.interval(5);
      debouncer6.attach(BUTTON6_PIN);
      debouncer6.interval(5);
      debouncer7.attach(BUTTON7_PIN);
      debouncer7.interval(5);
      debouncer8.attach(BUTTON8_PIN);
      debouncer8.interval(5);
      debouncer9.attach(BUTTON9_PIN);
      debouncer9.interval(5);
      debouncer10.attach(BUTTON10_PIN);
      debouncer10.interval(5);
    
      //presentation();
    }
    void presentation()  
    {   
      // Send the sketch version information to the gateway and Controller
      sendSketchInfo("Arduino SmartHub 1", "1.0");
      present(CHILD_ID_DIGITAL8, S_DOOR);
      present(CHILD_ID_DIGITAL9, S_DOOR);
      present(CHILD_ID_DIGITAL10, S_DOOR);
      
      for (int sensor=1, pin=RELAY_0; sensor<=NUMBER_OF_RELAYS;sensor++, pin++) {
        // Register all sensors to gw (they will be created as child devices)
        present(sensor, S_LIGHT);
      }
    }
    // Initialize Relay message
    MyMessage msg0(0, V_LIGHT);
    MyMessage msg1(1, V_LIGHT);
    MyMessage msg2(2, V_LIGHT);
    MyMessage msg3(3, V_LIGHT);
    MyMessage msg4(4, V_LIGHT);
    MyMessage msg5(5, V_LIGHT);
    MyMessage msg6(6, V_LIGHT);
    MyMessage msg7(7, V_LIGHT);
    
    void loop() { 
      // Send locally attached sensor data here 
      if (debouncer0.update()) {
        // Get the update value.
        int value0 = debouncer0.read();
        // Send in the new value.
        if(value0 == LOW){
             saveState(1, !loadState(1));
             digitalWrite(RELAY_0, loadState(1)?RELAY_ON:RELAY_OFF);
             send(msg0.set(loadState(1)));
             }
      }
      if (debouncer1.update()) {
          int value1 = debouncer1.read();
        if(value1 == LOW){
             saveState(2, !loadState(2));
             digitalWrite(RELAY_1, loadState(2)?RELAY_ON:RELAY_OFF);
             send(msg1.set(loadState(2)));
             }
      }
      if (debouncer2.update()) {
          int value2 = debouncer2.read();
        if(value2 == LOW){
             saveState(3, !loadState(3));
             digitalWrite(RELAY_2, loadState(3)?RELAY_ON:RELAY_OFF);
             send(msg2.set(loadState(3)));
             }
      }
      if (debouncer3.update()) {
          int value3 = debouncer3.read();
        if(value3 == LOW){
             saveState(4, !loadState(4));
             digitalWrite(RELAY_3, loadState(4)?RELAY_ON:RELAY_OFF);
             send(msg3.set(loadState(4)));
             }
      }
      if (debouncer4.update()) {
          int value4 = debouncer4.read();
        if(value4 == LOW){
             saveState(5, !loadState(5));
             digitalWrite(RELAY_4, loadState(5)?RELAY_ON:RELAY_OFF);
             send(msg4.set(loadState(5)));
             }
      }
      if (debouncer5.update()) {
          int value5 = debouncer5.read();
        if(value5 == LOW){
             saveState(6, !loadState(6));
             digitalWrite(RELAY_5, loadState(6)?RELAY_ON:RELAY_OFF);
             send(msg5.set(loadState(6)));
             }
      }
      if (debouncer6.update()) {
          int value6 = debouncer6.read();
        if(value6 == LOW){
             saveState(7, !loadState(7));
             digitalWrite(RELAY_6, loadState(7)?RELAY_ON:RELAY_OFF);
             send(msg6.set(loadState(7)));
             }
      }
      if (debouncer7.update()) {
          int value7 = debouncer7.read();
        if(value7 == LOW){
             saveState(8, !loadState(8));
             digitalWrite(RELAY_7, loadState(8)?RELAY_ON:RELAY_OFF);
             send(msg7.set(loadState(8)));
             }
      }
       if (debouncer8.update()) {
        // Get the update value.
        int value8 = debouncer8.read();
        // Send in the new value.
        if (value8 != oldValue8) {
        // Send in the new value
             send(msg8.set(value8==HIGH ? 1 : 0));
             oldValue8 = value8;
             }
      }
      if (debouncer9.update()) {
        // Get the update value.
        int value9 = debouncer9.read();
        // Send in the new value.
        if (value9 != oldValue9) {
        // Send in the new value
             send(msg9.set(value9==HIGH ? 1 : 0));
             oldValue9 = value9;
             }
      }
      if (debouncer10.update()) {
        // Get the update value.
        int value10 = debouncer10.read();
        // Send in the new value.
        if (value10 != oldValue10) {
        // Send in the new value
             send(msg10.set(value10==HIGH ? 1 : 0));
             oldValue10 = value10;
             }
      }
    }
    
    
    void receive(const MyMessage &message) {
      // We only expect one type of message from controller. But we better check anyway.
      if (message.type==V_LIGHT) {
         // Change relay state
         digitalWrite(message.sensor-1+RELAY_1, message.getBool()?RELAY_ON:RELAY_OFF);
         // Store state in eeprom
         saveState(message.sensor, message.getBool());
         // Write some debug info
         Serial.print("Incoming change for sensor:");
         Serial.print(message.sensor);
         Serial.print(", New status: ");
         Serial.println(message.getBool());
       } 
    }
    
    
    
    
    

  • Mod

    @1kohm are you looking to optimize the sketch for readability/maintainability, ram size, flash size, power consumption or something else?


  • Plugin Developer

    Looking at the code, I suspect it could be smart to create some for-loops.

    Look at the code in the before() part, and try to do that for other parts too?

    for (int sensor=1, pin=RELAY_0; sensor<=NUMBER_OF_RELAYS;sensor++, pin++) {
    


  • @mfalkvidd said in Help to optimize my sketch:

    maintainability

    Hi,
    yes, I'd like to optimize readability/maintainability of that code. RAM or flash size does not mater currently.
    I want to learn more how to use loops and #define parameters for more transparency.



  • @alowhum
    yes, tried to use loops but no luck so far.

    I have one loop with viarable

    #define NUMBER_OF_RELAYS 8 // Total number of attached relays
    

    which then is used

    void before() { 
      for (int sensor=1, pin=RELAY_0; sensor<=NUMBER_OF_RELAYS;sensor++, pin++) {
        // Then set relay pins in output mode
        pinMode(pin, OUTPUT);   
        // Set relay to last known state (using eeprom storage) 
        digitalWrite(pin, loadState(sensor)?RELAY_ON:RELAY_OFF);
      }
    }
    

    but how to use it i.e. for

    Bounce debouncer
    

    Maybe you can explain.
    Thanks


  • Plugin Developer

    Have a look at this. I left a lot of things out. The core is to use setSensor to select a device.

    // TOP OF SKETCH
    
    #define RELAY1_CHILD_ID 5                           // Set switch ID number on this node.
    #define RELAY2_CHILD_ID 6                           // Set switch ID number on this node.
    
    MyMessage relaymsg(RELAY1_CHILD_ID, V_LOCK_STATUS);
    
    
    
    boolean desiredDoorStates[DOOR_COUNT];              // An array that holds the desired door states. These are turned into:
    boolean actualDoorStates[DOOR_COUNT];               // An array that holds the actual door states.
    // These are arrays that will be looped over.
    
    
    // FROM SETUP
    
      memset(desiredDoorStates,0,sizeof(desiredDoorStates));
      memset(actualDoorStates,0,sizeof(actualDoorStates));
      // This fills the array with 0's.
    
    FROM LOOP
    
      for( byte checkingDoor = 0; checkingDoor < DOOR_COUNT; checkingDoor++ ){ // We loop over all the doorlocks in the system, and check if they are in the right position.
        if( desiredDoorStates[checkingDoor] != actualDoorStates[checkingDoor] ){
          if( desiredDoorStates[checkingDoor] == LOCKED ){
            digitalWrite(RELAY1_PIN + checkingDoor, RELAY_ON);
          }
          else if( desiredDoorStates[checkingDoor] == UNLOCKED ){
            digitalWrite(RELAY1_PIN + checkingDoor, RELAY_OFF);
          }
          send(relaymsg.setSensor(RELAY1_CHILD_ID + checkingDoor).set( desiredDoorStates[checkingDoor] )); wait(RF_DELAY); // Tell the controller in what state the lock is.
          actualDoorStates[checkingDoor] = desiredDoorStates[checkingDoor];
        }
      }```


  • @1kohm Perhaps you have a look at this sketch to get a good example on how to "loop" alsmost eveything:
    https://forum.mysensors.org/topic/4847/multi-button-relay-sketch/33



  • @alowhum
    Thanks for the input and example.
    I'll test your code now.



  • @rejoe2
    Hi,
    Thanks for feedback, I'll try code from alowhum first.


 

194
Online

8.5k
Users

9.3k
Topics

98.7k
Posts