Notifications
Clear all

Help Needed - Some code in a Custom Function is messing up Code in Another

14 Posts
6 Users
4 Likes
726 Views
Sid
 Sid
(@sid)
Member
Joined: 3 years ago
Posts: 111
Topic starter  

So I have started coding my NeoPixel LED Cube (4x4x4). I have also a keypad attached. A few of the keys have been coded and all was working almost fine. "Almost" - because the KeyPad gives me issues at times and at the moment I am not much bothered on fixing that part.

All the functions that I did until today, were (and are) working as I expected them to.

Now, when I added a new Function and Called it from within the loop, the function seems to work okay, but it creates some unwanted mess when I call other function - specially the Raindrop one. No, all loop works good,with RainDrop - what goes wrong is that 3 Pixels generate RED color - while they should be all blue.

If I comment out the call to the chaserDisplay() - the raindrops works fine as intended.

So here is the code set -

void loop() 
{
  // put your main code here, to run repeatedly:
  static char switchKey = '\0';
  char customKey = customKeyPad.getKey();
  if(customKey != '\0')
  {
    switchKey = customKey;
    pixels.clear();
  }
   switch(switchKey)
    {
      case '1':{Serial.println("RAINDROPS"); rainDrops();break;}
      case '2':{Serial.println("Horizontal Planes");horizontalPlanes();break;}
      case '3':{Serial.println("Vertical Planes");verticalPlanes();break;}
      case '4':{Serial.println("Chaser Light");chaserDisplay(0,0,0);switchKey='z';break;}
      case '5':{Serial.println("Single Columns");randomLines(); break;}
      case '6':{Serial.println("Cube Borders");break;}
      case '7':{Serial.println("GLOW ALL");break;}
      case '8':{Serial.println("Fountain");displayFountain();break;}
      case '9':{Serial.println("Squares");break;}
      case '0':{Serial.println("RESET");break;}
      case 'A':{Serial.println("All Effects");break;}
      case 'B':{Serial.println("Default Color Settings");break;}
      case 'C':{Serial.println("PINK Color");break;}
      case 'D':{Serial.println("Blue Color");break;}
      case '*':{Serial.println("Increase Speed");break;}
      case '#':{Serial.println("Decrease Speed");break;}
    }
}
 

Here is the culprit function - chaserDisplay -

 
void chaserDisplay(int xpos, int ypos,int zpos)
{
  int arrChaser[4][4][4]
  {
      {
        {0,1,2,3},
        {28,29,30,31},
        {32,33,34,35},
        {60,61,62,63}
      },
      {
        {4,5,6,7},
        {24,25,26,27},
        {36,37,38,39},
        {56,57,58,59}
      },
      {
        {8,9,10,11},
        {20,21,22,23},
        {40,41,42,43},
        {52,53,54,55}
      },
      {
        {12,13,14,15},
        {16,17,18,19},
        {44,45,46,47},
        {48,49,50,51}
      }
   };
   
    
    while(customKeyPad.getKey()=='\0')
    {
       switch(random(0,6))
       {
        case 0:
          if(xpos > 0) {xpos--;break;}
        case 1:
          if(xpos < 3) {xpos++;break;}
          xpos--; break;
        case 2:
          if(ypos > 0) {ypos--;break;}
        case 3:
          if(ypos < 3) {ypos++;break;}
          ypos--; break;
        case 4:
          if(zpos > 0) {zpos--;break;}
        case 5:
          if(zpos < 3) {zpos++;break;}
          zpos--; break;
       }
    pixels.setPixelColor(arrChaser[xpos][ypos][zpos], pixels.Color(50,50,50));/*random(150),random(150), random(150)));-*/
    pixels.show();
    delay(SPEED);
    pixels.clear();
    delay(SPEED);
    }
}

And here is poor one that gets into problems - raindrops -

void rainDrops()
{
    int lineArray[][16]={
    {15,8,7,0},
    {14,9,6,1},
    {13,10,5,2},
    {12,11,4,3},
    {16,23,24,31},
    {17,22,25,30},
    {18,21,26,29},
    {19,20,27,28},
    {47,40,39,32},
    {46,41,38,33},
    {45,42,37,34},
    {44,43,36,35},
    {48,55,56,63},
    {49,54,57,62},
    {50,53,58,61},
    {51,52,59,60}
  };
  int tArr[]={0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15};
  
  int colNumber = random(16);
  for(int i=0; i< 4; i++)
  {
    pixels.setPixelColor(lineArray[colNumber][i], pixels.Color(0, 0, 150));
    pixels.show();
    delay(50);
  }
  delay(SPEED);
}

Life is exploring and learning


   
Sean451 reacted
Quote
SuperCharlie
(@supercharlie)
Member
Joined: 3 years ago
Posts: 81
 

Just a shot in the dark as I'm not really that good at C but in arrays I get tripped up a lot because in Python they count from zero so if you want 10 items it's zero through nine basically... Maybe somebody's falling off the end?

Edit

I also wouldn't use any reserved characters for control like the pound sign and the star just to be safe


   
ReplyQuote
byron
(@byron)
No Title
Joined: 5 years ago
Posts: 1122
 
Posted by: @supercharlie

in Python they count from zero

its the same in C, counting starts with a zero.


   
SuperCharlie reacted
ReplyQuote
(@yurkshirelad)
Member
Joined: 3 years ago
Posts: 493
 

I don't know what provides this:

pixels.Color(0, 0, 150)

But is it possible Color() expects something like BGR or GBR instead of RGB?


   
ReplyQuote
SuperCharlie
(@supercharlie)
Member
Joined: 3 years ago
Posts: 81
 

Is it possible for the positions to be zero like xpos? Or is it always a positive number above zero? Also is there any way to put breaks and tracers in here? Like pause execution and show you the variable values?


   
ReplyQuote
vieuxgeek
(@vieuxgeek)
Member
Joined: 3 years ago
Posts: 12
 

I don't have the hardware to test but I did mock up a test program.  There appears to be an odd behavior when chaserDisplay() is run. In chaserDisplay() the while loop is controlled by reading the keypad.  When you press any key the loop exits and the function returns  - note it appears to leave the LED cube in it's last state (it is not cleared). You then set switchKey to 'z'  and loop() continues to process.  In loop() getKey() returns '\0' and the condition of the if statement where switchKey is set and pixel.clear() is called is false - so it's as if no key has been pressed.  Since switchKey is 'z' there is in no match in the switch statement.  (kind of stops the chaserDisplay but does not do anything).  

I think this leaves the cube in the last configuration run in chaserDisplay().  Once you press a new key, say '1' then switchKey is set to '1', pixels.clear() runs and then rainDrops()  runs.

If you press '1' when chaserDisplay() is running and expect rainDrops() to run then you have an issue.

Also, you don't use tArr[] in rainDrops() - not an issue but if you don't need it then I would comment it out.

Hope this helps.

When you're retired, every day is Saturday.


   
ReplyQuote
frogandtoad
(@frogandtoad)
Member
Joined: 5 years ago
Posts: 1458
 

 @sid

void loop()
{
  static char switchKey = '\0';
  char customKey = customKeyPad.getKey();

  if (customKey != '\0')
  {
    switchKey = customKey;
    pixels.clear();
  }

  switch (switchKey)
  {

OK, firstly, this looks a bit convoluted to me.

I'm not sure where you got that code, but that character '\0' is known as a "nul terminator' - Normally used in character arrays to designate the end of an character array string, in this case, it represents the defined library constant: 'NO_KEY".  Now if we plug that in, we'll see why I said it looks a bit convoluted to me 🙂

void loop()
{
  static char switchKey = NO_KEY;
  char customKey = customKeyPad.getKey();

  if (customKey != NO_KEY)
  {
    switchKey = customKey;
    pixels.clear();
  }

  switch (switchKey)
  {

... so given the above, it's much better to just read as follows:

 char customKey = customKeypad.getKey();
  
  if (customKey) {
    // Work with the key
    Serial.println(customKey);
  }

I know this doesn't solve your problem directly, just something to consider when designing the logic/flow of your program.

I suggest doing some basic debugging (placing strategic print statement to confirm your values are as expected).  I would also remove the switch statement and hard code x, y and zpos to check.  You could also try to add a 'pixels.clear();' at the start of your 'rainDrops()' function.

See how you go...


   
ReplyQuote
Sid
 Sid
(@sid)
Member
Joined: 3 years ago
Posts: 111
Topic starter  

First, thank you all for the generous support of trying to help me out. I will try replying you all (to the best of info I can - just bear in mind, that I am no expert with all this, and this KeyPad is the first time I am using it anywhere).

Posted by: @supercharlie

I also wouldn't use any reserved characters for control like the pound sign and the star just to be safe

For the text above this quoted one on your response, well, in C/C++ too they (arrays) start off from the 0 as their first subscript - so tArr[0] is the first element in the array.

The reserved characters # and * are on the Keypad, and the tutorial says they can be coded - in my case, I am using these as simple characters in the switch case - where I will be comparing the keypressed on the keypad.

And, this far, they seem to print the text as intended. Unsure right now what effects would these key-presses have when I have their corresponding function(s) written. Keeping my fingers crossed, and all should be good :), if not, I will ask the fourm again, but only after I check that aspect.

Life is exploring and learning


   
ReplyQuote
Sid
 Sid
(@sid)
Member
Joined: 3 years ago
Posts: 111
Topic starter  
First, thank you all for the generous support of trying to help me out. I will try replying you all (to the best of info I can - just bear in mind, that I am no expert with all this, and this KeyPad is the first time I am using it anywhere).
 
Posted by: @yurkshirelad

I don't know what provides this:

 

pixels.Color(0, 0, 150)

 

But is it possible Color() expects something like BGR or GBR instead of RGB?

 

I get you. Let me confess, I have not tested specifically for this BGR or GBR or even the RGB. But the above code does give me a Blue at 50% or so. Hence I have assumed that these are the RGB ones. And, no, this assumption has been correct - because when I use this 150 for Blue with others set to Zero, I do get Blue on all - Raindrops without this function (chaserDisplay commented out - from the case in the loop) does work perfectly with everything in Blue.

Agreed, I have to add more drops falling at a later point.

 

Life is exploring and learning


   
ReplyQuote
Sid
 Sid
(@sid)
Member
Joined: 3 years ago
Posts: 111
Topic starter  
Posted by: @frogandtoad

 @sid

..Code and Original content truncated...

void
loop() { switch (switchKey)  {

... so given the above, it's much better to just read as follows:

 char customKey = customKeypad.getKey();
  
  if (customKey) {
    // Work with the key
    Serial.println(customKey);
  }

I know this doesn't solve your problem directly, just something to consider when designing the logic/flow of your program.

I suggest doing some basic debugging (placing strategic print statement to confirm your values are as expected).  I would also remove the switch statement and hard code x, y and zpos to check.  You could also try to add a 'pixels.clear();' at the start of your 'rainDrops()' function.

See how you go...

Thanks mate.

Initially, I had it exactly the same way as you mention - without the intermediatory "switchKey" variable. But then, it was not behaving right as I desired it. It kept returning back, I mean, my functions ran but only once and waited for me to press a key. I did not want that. I wanted them to keep running until I pressed a key. So I thought to have an intermediate variable and use it in the switch. That way, the values in the "switchKey" will not change until someone presses the key on the KeyPad.

To explain-

without the switchKey - if I pressed 1, it would run the RainDrops function but only once and all I would get to see is just one column appearing. It would halt then. And I have to press 1 again.But with the "switchKey" variable, it runs as long as I do not press another key.

Life is exploring and learning


   
ReplyQuote
Sid
 Sid
(@sid)
Member
Joined: 3 years ago
Posts: 111
Topic starter  
Posted by: @vieuxgeek

Also, you don't use tArr[] in rainDrops() - not an issue but if you don't need it then I would comment it out.

Thanks for pointing this one out. Yes, at the moment, I am not using that array. But have some codes for it. As I ran into issues with the chaserDisplay, for the moment, all other codes are yet to be tried/tested. (A very wrong approach, I know!)

For the rest of your reply, I needed to set that "switchKey" variable to something. I had to avoid using any of the values of the switch because if I do not, it gets back into that function. So for instance, if I set that switchKey to 1 (after the call to chaserDisplay), it gets into the Raindrops function - no matter I press a key or not (after exiting the chaserDisplay).

 

Life is exploring and learning


   
ReplyQuote
frogandtoad
(@frogandtoad)
Member
Joined: 5 years ago
Posts: 1458
 

@sid

Posted by: @sid

Initially, I had it exactly the same way as you mention - without the intermediatory "switchKey" variable. But then, it was not behaving right as I desired it. It kept returning back, I mean, my functions ran but only once and waited for me to press a key. I did not want that. I wanted them to keep running until I pressed a key. So I thought to have an intermediate variable and use it in the switch. That way, the values in the "switchKey" will not change until someone presses the key on the KeyPad.

Ah, I see what you're were after now... no worries.

I would personally think about changing it slightly, just to keep the main loop as clean as possible for program logic, for example, something like this:

char tmp(NO_KEY);
char customKey(NO_KEY);

void loop()
 { 
  if((tmp = customKeypad.getKey())) {
    customKey = tmp;
    pixels.clear();
   }
      
  switch(customKey) {

[snip]

...no need for local static variables, or creating new variables in the loop at every iteration, as global scope variables are already static by default.  Also, using the "NO_KEY" constant helps to clarify program intent.

Cheers.


   
Sid reacted
ReplyQuote
vieuxgeek
(@vieuxgeek)
Member
Joined: 3 years ago
Posts: 12
 

Hi, did you figure out your problem?   I have been playing around with your code using a simulated cube.  I found that animation was much easier to do when I created a cube array ( like you did in displayChaser() ) that maps cube coordinates to the actual LEDs.  This also will make the program a lot smaller, especially as you add more animations. 

Here is a cube ( I use a width, height, and depth so I can have asymmetric cubes) : 

constexpr int g_cubeWidth  = 4; // x axis
constexpr int g_cubeHeight = 4; // y axis
constexpr int g_cubeDepth = 4; // z axis

int g_neoCube[g_cubeWidth][g_cubeHeight][g_cubeDepth]
{
{ // layer 0
{ 0, 1, 2, 3}, // (0, 0, 0) (0, 0, 1) (0, 0, 2) (0, 0, 3)
{28,29,30,31}, // (0, 1, 0) (0, 1, 1) (0, 1, 2) (0, 1, 3)
{32,33,34,35}, // (0, 2, 0) (0, 2, 1) (0, 2, 2) (0, 2, 3)
{60,61,62,63} // (0, 3, 0) (0, 3, 1) (0, 3, 2) (0, 3, 3)
},
{ // layer 1
{ 4, 5, 6, 7}, // (1, 0, 1) (1, 0, 1) (1, 0, 2) (1, 0, 3)
{24,25,26,27}, // (1, 1, 0) (1, 1, 1) (1, 1, 2) (1, 1, 3)
{36,37,38,39}, // (1, 2, 0) (1, 2, 1) (1, 2, 2) (1, 2, 3)
{56,57,58,59} // (1, 3, 0) (1, 3, 1) (1, 3, 2) (1, 3, 3)
},
{ // layer 2
{ 8, 9,10,11}, // (2, 0, 1) (2, 0, 1) (2, 0, 2) (2, 0, 3)
{20,21,22,23}, // (2, 1, 0) (2, 1, 1) (2, 1, 2) (2, 1, 3)
{40,41,42,43}, // (2, 2, 0) (2, 2, 1) (2, 2, 2) (2, 2, 3)
{52,53,54,55} // (2, 3, 0) (2, 3, 1) (2, 3, 2) (2, 3, 3)
},
{ // layer 3
{12,13,14,15}, // (3, 0, 1) (3, 1, 1) (3, 0, 2) (3, 0, 3)
{16,17,18,19}, // (3, 1, 0) (3, 2, 1) (3, 1, 2) (3, 1, 3)
{44,45,46,47}, // (3, 2, 0) (3, 2, 1) (3, 2, 2) (3, 2, 3)
{48,49,50,51} // (3, 3, 0) (3, 3, 1) (3, 3, 2) (3, 3, 3)
}
};

Here is how rainDrops() looks using the cube.  I changed a few things - parameters for some variables and removed some of the hard coded values.

void rainDrops( int rainDelay, uint32_t color )
{
DEBUG_PRINT( "rainDrops() : Entered. rainDelay = " + String(rainDelay) + " color = " + String( color ) );
int xpos = random(g_cubeWidth);
int zpos = random(g_cubeDepth);
for( int ypos = g_cubeHeight - 1; ypos >= 0; ypos-- )
  {
pixels.setPixelColor(g_neoCube[xpos][ypos][zpos], color)
DEBUG_PRINT( "rainDrops() : xpos = " + String(xpos) + " ypos = " + String(ypos) + " zpos = " + String(zpos) );
DEBUG_PRINT( " g_neoCube[xpos][ypos][zpos] = " + String(g_neoCube[xpos][ypos][zpos]) );
pixels.show();
delay(rainDelay);
  }
delay(g_animationSpeed);
}

Here is an animation where a line flows through the cube.  

void flow( int flowDelay, uint32_t color )
{
DEBUG_PRINT( "flow() : Entered. flowDelay = " + String( flowDelay ) + " color = " + String( color ) );
for ( int ypos = 0; ypos < g_cubeHeight; ypos++ )
{
// show row - going left to right when ypos is even and right to left when ypos is odd
int xStart;
int xIncrement;
if ( ypos % 2 ) // is odd
{
xStart = g_cubeWidth - 1;
xIncrement = -1;
}
else // is even
{
xStart = 0;
xIncrement = 1;
}
for ( int xpos = xStart; ( xpos >= 0 ) && ( xpos < g_cubeWidth ); xpos += xIncrement )
{
for ( int zpos = 0; zpos < g_cubeDepth; zpos++ )
{
pixels.setPixelColor(g_neoCube[xpos][ypos][zpos], color);
DEBUG_PRINT( "flow() : xpos = " + String(xpos) + " ypos = " + String(ypos) + " zpos = " + String(zpos) );
DEBUG_PRINT( " g_neoCube[xpos][ypos][zpos] = " + String(g_neoCube[xpos][ypos][zpos]) );
}
pixels.show();
delay( flowDelay );
pixels.clear();
delay( flowDelay );
}
}
delay( g_animationSpeed );
}

When you're retired, every day is Saturday.


   
Sid reacted
ReplyQuote
Sid
 Sid
(@sid)
Member
Joined: 3 years ago
Posts: 111
Topic starter  
Posted by: @vieuxgeek

Hi, did you figure out your problem?

 

Thanks vieuxgeek for this beautiful set of codes and assitance here. Very much appreciated.

As for resolving the issue, I had a little occupied week. I am free today and am surely going to look into it, probably the things I have found out will help. If so, I will post the solution as a new post here and its link on this thread as well.

The codes, yes, I realized only when I started writing codes for the chaser, that I was mostly doing things the wrong way - the 3-D arrays and dynamic codes are surely going to make the codes more elegant and professional. So I will be using the time this weekend transferring codes into this.

Life is exploring and learning


   
ReplyQuote