Announcement

Collapse
No announcement yet.

Roast my Code Arduino 4D Display Project

Collapse
X
 
  • Filter
  • Time
  • Show
Clear All
new posts

  • Roast my Code Arduino 4D Display Project

    Afternoon, it's been a long time since I programmed anything and took me a lot of ****ing around to get a working setup but now I have got everything working as intended. I work on a power plant and took on a project to create a test unit for a crane spreader (the part that picks up containers) which includes a touchscreen. Long story short it lets you toggle relays and displays feedback in the form of LEDs. Wanted to see if anyone could give me criticism for my code to make it function better or for it to perform better.

    Code:
    /*
              Control Sketch 2
                This sketch is the first draft of the complete control and feedback
                When connected together this should allow full control of the relays and display of feedback
    */
    #include <genieArduino.h>
    Genie genie;
    #define RESETLINE 4
    // Declare all initial relay states
    bool relayStates[] = {0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0};
    // Set pins
    int relayPins[] = {22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32, 33, 34, 35, 36, 37};
    int feedbackPins[] = {40, 41, 42, 43, 44, 45, 46, 47};
    // Variables for timing
    unsigned int buttonTimes[] = {0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0};
    int index;
    void setup() {
      // put your setup code here, to run once:
      checkRelayBools(); // running this first so that the outputs are set to HIGH immediately to avoid sending any signals to spreader accidently
                          // this is because when the outputs are set to output they will be LOW as default which would close all the relays
      for (int i = 0; i < 16; i++)
      {
        pinMode(relayPins[i], OUTPUT);  // set all relay pins as OUTPUT
        //Serial.print(relayPins[i]); // use for debugging to see if for loop is correct size for array
      }
      for (int i = 0; i < 8; i++)
      {
        pinMode(feedbackPins[i], INPUT);  // set all feedback pins as INPUT
      }
      
      Serial.begin(9600); // start communication @115200 baud rate  on Serial0 (9600 BAUD WHILE CANNOT EDIT 4D PROJECT)
      genie.Begin(Serial); // tell genie instance to communicate on Serial0 (RX0 and TX0)
      genie.AttachEventHandler(genieEventHandler); // attach event handler
      pinMode(RESETLINE, OUTPUT); // Set D4 on Arduino to Output (4D Arduino Adaptor V2 - Display Reset)
      digitalWrite(RESETLINE, 1); // Reset the Display via D4
      delay(100);
      digitalWrite(RESETLINE, 0); // unReset the Display via D4
      // Increase to 4500 or 5000 if you have sync problems as your project gets larger. Can depent on microSD init speed.
      delay (3500); //let the display start up after the reset (This is important)
    }
    void loop() {
      // put your main code here, to run repeatedly:
      static unsigned int waitPeriod = millis();
      genie.DoEvents(); // This calls the library each loop to process the queued responses from the display
      
      if (millis() >= waitPeriod)
      {
        //Serial.println("=== PERFORMING CHECKS ===");
        checkRelayBools(); // check to see if relays need closing as a result of inputs
        // marked this out as it doesn't seem neccessary to call this every frame rather it seems better to call it when relay states are changed
        // put back in until a better solution to timing is found as otherwise relays will not self toggle
      
        checkFeedbackInputs();
        waitPeriod = millis() + 200; // don't rerun code for 200ms just to save processing power
      }
    }
    void genieEventHandler(void)
    {
      genieFrame Event;
      genie.DequeueEvent(&Event); // Remove the next queued event from the buffer, and process it below
      if (Event.reportObject.cmd == GENIE_REPORT_EVENT)
      {
        //Serial.println("EVENT REPORTED");    
        if (Event.reportObject.object == GENIE_OBJ_4DBUTTON)
        {
          if (Event.reportObject.index < 14 && Event.reportObject.index >= 0) // FOR PRESS BUTTONS INDEXES 0-13
          {        
            index = Event.reportObject.index; // store received index locally
            //Serial.print("BUTTON INDEX ");
            //Serial.println(index);
            relayStates[index] = 1; // toggle boolean (toggle relay)
            buttonTimes[index] = millis();
          }
          if (Event.reportObject.index == 14) // HOLD BUTTON
          {
            relayStates[14] = !relayStates[14];
          }
          if (Event.reportObject.index == 15) // HOLD BUTTON
          {
            relayStates[15] = !relayStates[15];
          }
        }
      }
    }
    void checkRelayBools()
    {
      for (int i = 0; i < 16; i++)
      {
          if (relayStates[i] == 1)
          {
            digitalWrite(relayPins[i], 0);
            
            if (millis() > buttonTimes[i] + 1000) // wait for a second then toggle the relay again
            {
               relayStates[i] = 0;
            }        
            /*Serial.print("RELAY ");
            Serial.print(relayPins[i]); // DEBUGGING :: USES ALL BANDWITDH AND PREVENTS PROPER USE OF PROGRAM AT 9600 BAUD RATE
            Serial.println(" CLOSED");*/
          } else
          {
            digitalWrite(relayPins[i], 1);  
            /*Serial.print("RELAY ");
            Serial.print(relayPins[i]); // DEBUGGING :: USES ALL BANDWITDH AND PREVENTS PROPER USE OF PROGRAM AT 9600 BAUD RATE :/
            Serial.println(" OPENED");*/
          }
      }
    }
    void checkFeedbackInputs()
    {
      for (int i = 0; i < 8; i++)
      {
          if (digitalRead(feedbackPins[i]) == 1)
          {
            genie.WriteObject(GENIE_OBJ_LED, i, 1);
            //Serial.print("INPUT ON PIN ");
            //Serial.println(feedbackPins[i]);
          } else
          {
            genie.WriteObject(GENIE_OBJ_LED, i, 0);
          }
      }
    }
    All that being said I have learnt a lot about the components involved and specifically the genieArduino library so if anyone is trying to get anything similar working or struggling to get something like communication between display and Arduino I'd be happy to try and help.

  • #2
    Hello

    I havent looked in too much detail yet, but right off the bat I can see you are doing Serial.print statements, as well as using Serial to talk to the display.
    This will never work, as the display will be receiving those serial print statements too and have no idea what to do with them, as they are not genie protocol

    I think you should have a read of this sticky post:
    https://forum.4dsystems.com.au/node/40596
    James

    Comment


    • #3
      Hi James yes I noticed this caused issues. I also found that post yesterday after spending the whole previous day solving issues myself which I could've found the solution to immediately from that post. I have now changed the serial statements to serial1. I am encountering a problem actually which I am currently looking into where the program still thinks it is getting input sometimes despite the input no longer being live. This is causing issues with my feedback as the LEDs on the display will stay green when they should be red. And they seem to randomly change back with no reason.

      For reference this is what my display looks like
      Click image for larger version

Name:	image.png
Views:	12
Size:	311.2 KB
ID:	78563

      Comment


      • #4
        Hi again

        I can sort of understand what you are doing, but not sure your code is doing quite what you want it to..

        Code:
        relayStates[index] = 1; // toggle boolean (toggle relay)
        Comment states you are toggling the boolean, however all you are doing is setting that index of the array to 1, I don't see you ever changing it back to 0 again. It doesn't look like you are doing relayStates[index] again in the code you provided.

        Maybe this is contributing to what you are seeing...
        James

        Comment


        • #5
          My comments might be a bit outdated what the was before was:
          Code:
          relayStates[index] = !relayStates[index]
          It was literally toggling it but now I make sure it sets to true and then later in code it sets to false.

          Code:
          void checkRelayBools()
          {
            for (int i = 0; i < 16; i++)
            {
                if (relayStates[i] == 1)
                {
                  digitalWrite(relayPins[i], 0);
                  
                  if (millis() > buttonTimes[i] + 1000 && i < 14) // wait for a second then toggle the relay again
                  {
                     relayStates[i] = 0;
                  }
          In the checkRelayBools() function it waits a second then turns the relay off.

          I have troubleshooted a bit more now and realised that when the digital input pins are not connected to anything it causes issues. Once connected they start to behave as expected but only when plugged into USB power. I'm not sure how much you know about Arduinos and if this is the kind of issue you can diagnose but it functions as expected off of USB power but when I power the Arduino off a 5v DC power supply through the Vin and GND pins then the LEDs constantly change state. As if the digital inputs are constantly toggling between reading HIGH and reading LOW. I hope I have explained that well enough? I am using an Arduino Mega 2560 rev3 fyi.

          TLDR it isn't a programming issue it appears to be some kind of power issue. I cannot use USB power as it needs to be installed into a box with electronics and a 110v supply is what will be powering the 5v PSU. Any ideas would be greatly appreciated. Thank you.

          Comment


          • #6
            I cant see how USB vs 5V power would cause any difference, however with Arduino and may other microcontrollers, you need the pins pulled high or low, and then when the thing that is toggling the input actuates, it pulls it the opposite direction. That way nothing is ever floating - which can be the killer for many things like this.

            So ideally you would have a pull up resistor on each of the inputs, say 10K to 5V, and then your inputs would all GND the input when they actuate (or the opposite, pull down to GND with 10K and the input raises it to 5V). It depends how you wire things up. If you just have the input going to your external thing and its not connected to anything normally, but then set high or low when the input is active, then it will be floating all the other time which can lead to these 'unknown' states.

            You can use Internal Pullups on the Arduino, if you dont want to be using external components.
            Code:
            pinMode(2, INPUT_PULLUP);
            for example.
            The pinMode is by default set to INPUT for all GPIO, but if you want to enable the internal pull up then add the above to whatever arduino GPIO you are using, and you might have a more favourable experience.
            James

            Comment


            • #7
              Another thing you could try is debouncing the input with code, there are a number of Arduino demos on this.

              https://www.arduino.cc/en/Tutorial/B...mples/Debounce

              or this library for example
              https://github.com/Mokolea/InputDebounce

              Have a look into that, see if that might help you.
              James

              Comment


              • #8
                Cheers James using the pullup resistors in the inputs solved the issue and now works completely as expected. Heres my final code if you get the time to glance over would accept any criticisms however it is functioning completely as intended and I believe it is quite efficient as well.
                Code:
                /*
                           ==============================================
                          
                                        SPREADER CONTROL
                                Fully functioning control and feedback
                                    for spreader            
                                    Last updated
                                                03.08.22                      
                
                
                           ==============================================
                            
                                                    
                */
                #include <genieArduino.h>
                Genie genie;
                #define RESETLINE 4
                // Declare all initial relay states
                bool relayStates[] = {0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0};
                // Set pins
                int relayPins[] = {22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32, 33, 34, 35, 36, 37};
                /*  COMMAND PINS
                22 F1 DOWN
                23 F2 DOWN
                24 F3 DOWN
                25 F4 DOWN
                26 F1 UP
                27 F2 UP
                28 F3 UP
                29 F4 UP
                30 -90 ROT
                31 0 ROT
                32 +90 ROT
                33 180 ROT
                34 L TWL
                35 UL TWL
                36 R ROT
                37 L ROT
                */
                int feedbackPins[] = {40, 41, 42, 43, 44, 45, 46, 47};
                /*  FEEDBACK PINS
                40 SEATED
                41 FAULT
                42 TWL L
                43 TWL UL
                44 +90 ROT
                45 0 ROT
                46 -90 ROT
                47 180 ROT
                */
                // Variables for timing
                unsigned int buttonTimes[] = {0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0};
                int index;
                void setup() {
                  // put your setup code here, to run once:
                  checkRelayBools(); // running this first so that the outputs are set to HIGH immediately to avoid sending any signals to spreader accidently
                                      // this is because when the outputs are set to output they will be LOW as default which would close all the relays
                  for (int i = 0; i < 16; i++)
                  {
                    pinMode(relayPins[i], OUTPUT);  // set all relay pins as OUTPUT
                  }
                  for (int i = 0; i < 8; i++)
                  {
                    pinMode(feedbackPins[i], INPUT_PULLUP);  // set all feedback pins as INPUT_PULLUP
                  }
                  
                  Serial.begin(200000); // start communication @115200 baud rate on Serial0 // trying higher rate
                  Serial1.begin(9600); // for debugging comments
                  genie.Begin(Serial); // tell genie instance to communicate on Serial0 (RX0 and TX0)
                  genie.AttachEventHandler(genieEventHandler); // attach event handler
                  pinMode(RESETLINE, OUTPUT); // Set D4 on Arduino to Output (4D Arduino Adaptor V2 - Display Reset)
                  digitalWrite(RESETLINE, 1); // Reset the Display via D4
                  delay(100);
                  digitalWrite(RESETLINE, 0); // unReset the Display via D4
                  // Increase to 4500 or 5000 if you have sync problems as your project gets larger. Can depent on microSD init speed.
                  delay (5000); //let the display start up after the reset (This is important)
                }
                void loop() {
                  // put your main code here, to run repeatedly:
                  static unsigned int waitPeriod = millis();
                  static unsigned int fbWaitPeriod = millis();
                  genie.DoEvents(); // This calls the library each loop to process the queued responses from the display
                  
                  if (millis() >= waitPeriod)
                  {
                    //Serial1.println("=== PERFORMING CHECKS ===");
                    checkRelayBools(); // check to see if relays need closing as a result of inputs
                    // marked this out as it doesn't seem neccessary to call this every frame rather it seems better to call it when relay states are changed
                    // put back in until a better solution to timing is found as otherwise relays will not self toggle
                  
                    
                    waitPeriod = millis() + 200; // don't rerun code for 200ms just to save processing power
                  }
                  if (millis() >= fbWaitPeriod)
                  {
                    checkFeedbackInputs();
                    fbWaitPeriod = millis() + 500; // dont rerun code for 500ms
                  }
                }
                void genieEventHandler(void)
                {
                  genieFrame Event;
                  genie.DequeueEvent(&Event); // Remove the next queued event from the buffer, and process it below
                  if (Event.reportObject.cmd == GENIE_REPORT_EVENT)
                  {
                    //Serial1.println("EVENT REPORTED");    
                    if (Event.reportObject.object == GENIE_OBJ_4DBUTTON)
                    {
                      if (Event.reportObject.index < 12 && Event.reportObject.index >= 0) // FOR PRESS BUTTONS INDEXES 0-11
                      {        
                        index = Event.reportObject.index; // store received index locally
                        //Serial1.print("BUTTON INDEX ");
                        //Serial1.println(index);
                        relayStates[index] = 1; // close the relay
                        buttonTimes[index] = millis();
                      }
                      if (Event.reportObject.index == 12) // LOCK TWL
                      {
                        if (digitalRead(feedbackPins[0]) == 0 && digitalRead(feedbackPins[3]) == 0) // checker spreader is seated and that twistlocks are unlocked
                        {
                          relayStates[12] = 1;
                        }
                      }
                        
                      if (Event.reportObject.index == 13) // UNLOCK TWL
                      {
                        if (digitalRead(feedbackPins[0]) == 0 && digitalRead(feedbackPins[2]) == 0) // checker spreader is seated and that twistlocks are locked
                        {
                          relayStates[13] = 1;
                        }
                      }
                      
                      if (Event.reportObject.index == 14) // TOGGLE LEFT ROT
                      {
                        if (relayStates[15] == 1) // check I am not turning right already
                        {
                          // do not allow to turn left
                        } else
                        {  
                          relayStates[14] = !relayStates[14];
                        }
                      }
                      if (Event.reportObject.index == 15) // TOGGLE RIGHT ROT
                      {
                        if (relayStates[14] == 1) // check I am not turning left already
                        {
                          // do not allow to turn right
                        } else
                        {  
                          relayStates[15] = !relayStates[15];
                        }
                      }
                    }
                  }
                }
                void checkRelayBools()
                {
                  for (int i = 0; i < 16; i++)
                  {
                      if (relayStates[i] == 1)
                      {
                        digitalWrite(relayPins[i], 0);
                        
                        if (millis() > buttonTimes[i] + 300 && i < 14) // wait for 300ms then open the relay
                        {
                           relayStates[i] = 0;
                        }        
                        /*Serial1.print("RELAY ");
                        Serial1.print(relayPins[i]); // DEBUGGING :: USES ALL BANDWITDH AND PREVENTS PROPER USE OF PROGRAM AT 9600 BAUD RATE
                        Serial1.println(" CLOSED");*/
                      } else
                      {
                        digitalWrite(relayPins[i], 1);  
                        /*Serial1.print("RELAY ");
                        Serial1.print(relayPins[i]); // DEBUGGING :: USES ALL BANDWITDH AND PREVENTS PROPER USE OF PROGRAM AT 9600 BAUD RATE :/
                        Serial1.println(" OPENED");*/
                      }
                  }
                }
                void checkFeedbackInputs()
                {
                  for (int i = 0; i < 8; i++)
                  {
                      if (digitalRead(feedbackPins[i]) == 0)
                      {
                        genie.WriteObject(GENIE_OBJ_LED, i, 1);
                        //Serial1.print("INPUT ON PIN ");
                        //Serial1.println(feedbackPins[i]);
                      } else
                      {
                        genie.WriteObject(GENIE_OBJ_LED, i, 0);
                      }
                  }
                }

                Comment


                • #9
                  Glad to hear that worked and its all running nicely for you now
                  James

                  Comment

                  Working...
                  X