Can any one show me why this code does not work as I think it should (creating ellipses in a class)

dneilan

        `int LedColor[][] = { { #ffcccd, #ff1924 },            // Red off and on
                             { #bbffb5, #14ff00 },            //Green off and on
                             { #fce9a9, #ffc400 } };          // Yellow off and on
        int LabelVpos = 165;
        String LedName[] = { "Active", "Green", "Yellow" };
        int LedHpos[] = {60, 120, 180, 240, 320, 380};
        int LedVpos = 140;

        LEDs Led;

        void setup() {
          surface.setTitle("LED class test");
          size(476, 300);

          Led = new LEDs(0,"Red",LedColor[0],1,LedHpos[0],LedVpos,20,20); 
        }

        void draw() {
          delay(2000);
          Led.LEDon();
          delay(2000);
          Led.LEDoff();

        }


        class LEDs {

          String LEDname;
          int LEDid;
          int LEDcolor[]; // receiving an passed array
          int LEDstat;
          int Horiz;
          int Vert;
          int Wide;
          int Tall;

         LEDs(int A,String B,int C[],int D,int E,int F,int G,int H) {
            LEDid = A;
            LEDname = B;
            LEDcolor = C;   // LEDcolor[0]=off - LEDcolor[1]=on
            LEDstat = D;
            Horiz = E;
            Vert = F;
            Wide = G;
            Tall = H;
            if(LEDstat==1) fill(LEDcolor[1]); else fill(LEDcolor[0]);
            ellipse(Horiz,Vert,Wide,Tall);
          }

        void LEDswitch() {
          println("switch");
            if(LEDstat==1) { 
              LEDstat=0;
              fill(LEDcolor[0]);
              ellipse(Horiz,Vert,Wide,Tall);
              return;
            }
              LEDstat=1;
              fill(LEDcolor[1]);  
              ellipse(Horiz,Vert,Wide,Tall);
          }

         void LEDon() {                                     // Turn on LED
             LEDstat=1; 
             fill(LEDcolor[1]);
             ellipse(Horiz,Vert,Wide,Tall);
             println("on");
            }

         void LEDoff() {                                     // Turn off LED
             LEDstat=0;
              fill(LEDcolor[0]);
              ellipse(Horiz,Vert,Wide,Tall);
             println("off");
            }                           

         int LEDstatus() { return(LEDstat); }                   // Return LED status

        }`

dneilan

the Led.LEDswitch() method works as planned but the on and off methods don't :((

kfrajer

Please edit your post, select your code and hit ctrl+o in order to format your code.

Kf

koogs

No, edit the original post.

koogs

void draw() {
  delay(2000);
  Led.LEDon();
  delay(2000);
  Led.LEDoff();
}

delay is bad and really shouldn't be used. read the reference.

https://processing.org/reference/delay_.html

also, the screen is only updated at the end of draw so you won't see the LEDon because it's overwritten by LEDoff before you get to see it and the delays are just wasting your time.

https://forum.processing.org/two/discussion/8085/i-display-images-in-sequence-but-i-see-only-the-last-one-why

but this might help:

https://forum.processing.org/two/discussion/8084/how-do-i-display-a-message-for-a-few-seconds#latest

(now, go back and fix the formatting of your code)

dneilan

Thanks for taking a quick look... I only dropped the heinous delay() into the example code to show the issue. As per the site policy- I tried to give to most basic code that would replicate the issue. The LED methods all have an ellipse call in them and draw the LED as used. I have created other ways to call the methods in the class and they all work, but when I try from within the main draw() loop I get very inconsistent results. Example - I used keypressed call and called the methods directly.

Every time I look at the code I cut/pasted/edit I looks as it should. What formatting is needed ?

Lord_of_the_Galaxy

Select you code, control + o to indent, followed by leaving a linebreak before and after your code. That will do the job.

kfrajer

I used keypressed call and called the methods directly

There is no keyPressed function in the provided code. It will be great to show your updated code if you are still having problems.

Regarding your original code at line 50, aren't you missing and else in your if statement?

Going back to your first question:

final int timeFrame=2000; // 2 secs
int switchTime=0;
void draw() {
  if(millis()>switchTime){
     if(Led.LEDstatus())   
        Led.LEDon() 
     else  
        Led.LEDoff();  
     //or call Led.LEDswitch()
     switchTime=millis()+timeFrame;
  }
}

Kf

dneilan

    //int Red[] = { #ffcccd, #ff1924 }; 
    //int Green[] = { #bbffb5, #14ff00 };
    //int Yellow[] = { #fce9a9, #ffc400 };
    int LedColor[][] = { { #ffcccd, #ff1924 },            // Red off and on
                         { #bbffb5, #14ff00 },            //Green off and on
                         { #fce9a9, #ffc400 } };          // Yellow off and on
    int LabelVpos = 165;
    String LedName[] = { "Active", "Green", "Yellow" };
    //int RED = #ff1924; 
    //int GREEN = #14ff00;
    //int YELLOW = #ffc400;
    //int LedColor[] = { RED, GREEN, YELLOW };
    int LedHpos[] = {60, 120, 180, 240, 320, 380};
    int LedVpos = 140;

    LEDs[] LED = new LEDs[3];                               // Creat an array of LEDs


    void setup() {
      surface.setTitle("LED class test");
      size(476, 300);        //   Id - Name - Color[] - Status - HorizP - VertP - Wide - Tall

      for (int l=0; l<3; l++) {
        LED[l] = new LEDs(l,LedName[l],LedColor[l],1,LedHpos[l],LedVpos,20,20); // pass 2 dim array LedColor

      }
     }



    void draw() {
      /**
        doit(2000);
        LED[1].LEDswitch();               // reversed to on
        doit(2000);
        LED[1].LEDswitch();               // reversed to off
        doit(2000);
        LED[1].LEDon();               // switched  on
        doit(2000);
        LED[1].LEDoff();               // switched to off
        doit(2000);
        LED[1].LEDon();               // switched  on
        */
    }

    void doit(int time){
      int Start=millis();
        while(millis()< Start + time);
    }

dneilan

    class LEDs {

      String LEDname;
      int LEDid;
      int LEDcolor[]; // receiving an passed array
      int LEDstat;
      int Horiz;
      int Vert;
      int Wide;
      int Tall;

     LEDs(int A,String B,int C[],int D,int E,int F,int G,int H) {
        LEDid = A;
        LEDname = B;
        LEDcolor = C;   // LEDcolor[0]=off - LEDcolor[1]=on
        LEDstat = D;
        Horiz = E;
        Vert = F;
        Wide = G;
        Tall = H;
        if(LEDstat==1) fill(LEDcolor[1]); else fill(LEDcolor[1]);
        ellipse(Horiz,Vert,Wide,Tall);
      }

    void LEDswitch() {
      println("switch");
        if(LEDstat==1) { 
          LEDstat=0;
          fill(LEDcolor[0]);
          ellipse(Horiz,Vert,Wide,Tall);
          return;
        }
          LEDstat=1;
          fill(LEDcolor[1]);  
          ellipse(Horiz,Vert,Wide,Tall);
      }

     void LEDon() {                                     // Turn on LED
         LEDstat=1; 
         fill(LEDcolor[1]);
         ellipse(Horiz,Vert,Wide,Tall);
         println("on");
        }

     void LEDoff() {                                     // Turn off LED
         LEDstat=0;
          fill(LEDcolor[0]);
          ellipse(Horiz,Vert,Wide,Tall);
         println("off");
        }                           

     int LEDstatus() { return(LEDstat); }                   // Return LED status

    }



     //--------------------------------------------------------------------------------
     void keyPressed() {
         int w=key-48; 
         if(w==1) LED[0].LEDon();
         if(w==4) LED[0].LEDoff();
         if(w==7) LED[0].LEDswitch();
         if(w==2) LED[1].LEDon();
         if(w==5) LED[1].LEDoff();
         if(w==8) LED[1].LEDswitch();
         if(w==3) LED[2].LEDon();
         if(w==6) LED[2].LEDoff();
         if(w==9) LED[2].LEDswitch();

        }

dneilan

Tried testing without delay - using millis() Try using numeric keys - 123 turn on leds 456 turn them off and 789 swap off for on etc

koogs

but you have nothing in draw().

dneilan

Don't need anything in draw for this test. It was just to use the keypressed to check the class - method functionality

dneilan

As I previously stated, this is test code as per forum requirements.

kfrajer

If I understand correctly, if you call LEDon, it should keep the led ON for 2 secs. if you call the LEDoff...mmmm what should it do?

Now, if you want to time your led independently of each other, you need to create a member variable resetTime in your LEDs class. I show the concept next.

class LEDs {

  String LEDname;
  int LEDid;
  int LEDcolor[]; // receiving an passed array
  int LEDstat;
  int Horiz;
  int Vert;
  int Wide;
  int Tall;
  int resetTime;

  (...)
  (...)   OTHER FUCNTIONS HERE. Not working code... this is just a demo
  (...)

  void setResetTime(int newResetTime){resetTime=newResetTime;}

  int getResetTime(){return (resetTime);}

}//End of LED class

Now you need to set the proper reset time in draw and check for the time in order to do an action. Example:

   ******In setup():
   for (int j=0; j<3; j++) {
    LED[j].setResetTime(millis()+2000);

.

  ******In draw():
  for (int j=0; j<3; j++) {
    if(LED[j].getResetTime()>millis()){
      //
      //  TWO seconds have passed... trigger a change.. turn on, of, change color, toggle, etc.
      //
    }

This is the idea. I hope it helps,

Kf

dneilan

Hi kfrajer Thanks for information, but that's not the issue. I have a lot more code that was calling the methods, actually it was coming in from several Arduinos over ethernet. This was a test code to emulate various on-board LED's on the Arduinos. My point was the methods all work perfectly when called from keypressed but when logic was placed in the draw() loop, with or without delay() and the methods were then called, they didn't work.

I have these Arduinos running various heating systems, recently I wanted to improve my coding and started to use classes, wrote my own timers and started to use threading etc, but this class with the LED methods keeps mucking me around. I tried re-writing the classes in a new sketch, simplifying the calling code, just to test. Thus the delay - turn on - delay - turn off - delay turn on etc. boring, but testing. Even tried on a different PC, same results. Then I double checked my class and methods by calling from external functions.

koogs

you posted new code but you didn't say whether the code is working as expected or not working as expected or what the expected behaviour was. how are we meant to know?

again, the display is only updated at the end of draw(). if you do two or more things to the same LED in the same call to draw() you will only see the last one.

koogs

void draw() {
    fill(0); // black
    ellipse(100, 100, 10, 10);

    // do anything you want here 

    fill(255); // white
    ellipse(100, 100, 10, 10);
}

you will only ever see the white circle.

dneilan

Before rushing to tell me the same thing again, please try the code I posted. I realize what is good and what is bad code. I asked the forum because I have tried various different ways to test my class and methods and I can't fix the issue. Using the keypressed function, all the methods work and draw correctly. ONLY when I try testing calls from in the draw() loop do I get issues. The system runs the EXACT same methods as previously proven to work, but now don't.

Lord_of_the_Galaxy

You DO NOT use delay or any thing that does the same thing in draw. Use the answer provided by @kfrajer.