Shifting array three bytes yet keeping all elements

Go To Last Post
20 posts / 0 new
Author
Message
#1
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Freaks,

 

I have been playing with some Neopixels for a small job and I am getting the hang of it up until now.  I have for the moment 5 leds in series with each other.  I created an array to store the color pattern and what I would like to do is the following:

 

1) load the array into the 5 NEOs and turn them on (1, 2, 3, 4, 5)

2) shift all the bytes in the array three places to the left(there are three bytes for each NEO) and take the 1st byte and move it to the end(2, 3, 4, 5, 1)

3) continuously repeat step 2 (the next sequence would be 3, 4, 5, 1, 2 and so on).

 

I have my colors(clrs) all set, and the array that stores individual pixel colors(pixelclrs)

__uint24 clrs[ ] = {BLACK, RED, GREEN, BLUE, WHITE, YELLOW, ORANGE};// BLACK is there as a fill - keep it there for now
__uint24 pixelclr[5]; //pixel color array

I have the routine that loads the 'pixelclr' array with the desired colors:

count = 0;
led = 1;
 for(count = 0; count < 5; count++)
  {
    pixelclr[count] = clrs[led]; //load array with color pattern
    led++;

  }

This works

 

Next I have the routine that displays the pattern I just loaded into the 'pixelclr' array:

count = 0;
     led = 0;

     for(count = 0; count < 5; count++)
     {
     leds.setPixelColor(count, pixelclr[led]); //load LEDs with color pattern from array
     led++;
     }
     leds.show();   //show the pattern

THis too works.

 

Where I am stuck is how to move the array elements to the left(or right) 24 places(there are three bytes per NEO).  I have been doing some searching and came across this:

// reverse array from start to end
void reverse(int a[], int start, int end)
{
  int i;
  int temp;
  while(start++ < end--)
  {
    temp = a[start];
    a[start] = a[end];
    a[end] = temp;
  }
}

Which looks like it could work, but it only shifts one place.  I need to shift three full bytes

 

Do I need to create an additional storage element to hold onto the three shifted bytes and then add them to the end of the array?

I get the feeling I am over thinking this....any suggestions?  This little 5 element experiment is for a larger array for teh final widget.

 

JIm

 

EDIT:

I just came up with an idea.....off to bed for now, will try tomorrow night and reply.

 

If you want a career with a known path - become an undertaker. Dead people don't sue! - Kartman

Please Read: Code-of-Conduct

Atmel Studio6.2/AS7, DipTrace, Quartus, MPLAB user

Last Edited: Sat. Jan 13, 2018 - 06:21 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Can't you just move the pointer to the array?

John Samperi

Ampertronics Pty. Ltd.

www.ampertronics.com.au

* Electronic Design * Custom Products * Contract Assembly

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Instead of manipulating the array, it may be easier to just introduce an offset when you read the array. I.e. initialize the led variable to something else than 0. Then make sure to do wrap-around when it reaches the end of the array.

/Jakob Selbing

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Both Royalty and jaksel offer the obvious method for using the hardware.    i.e. a circular pointer into a circular buffer.

 

But as a general problem,   you just draw a diagram on paper e.g. start array and result.   Then draw arrows for how individual elements move e.g.

uint8_t array[33] = "David Prentice is a jolly nice chap.";
uint8_t mvdwn[33] = "id Prentice is a jolly nice chap.Dav";
uint8_t movup[33] = "ap.David Prentice is a jolly nice ch";

You can see that the "move down" result requires you to iterate upwards e.g. array[i] = array[i + 3]; i++;

And "move up" result requires you to iterate from the end backwards e.g. array[i] = array[i - 3]; i--;

 

Then you look at the edge cases.  e.g. which array element gets overwritten.  how do you solve it?  

perhaps save in temporary variable at start.

likewise wrapping a circular buffer is simple.   if (i >= size) i -= size;   

 

There are C library functions like memmove() that will look after the move direction.

But they work on flat memory areas rather than circular buffers.

 

David.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Another vote for data fixed and indices/pointers that move.

 

As David says there is memmove() and memcpy() but you don't really want to be moving large chunks of data around.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Good morning!

After a bit of sleep, and the replied here I realized that my idea might not work as I thought, but as Mr. Prentice has said in the past use a paper PC to work out the problem.

 

js wrote:
Can't you just move the pointer to the array?

jaksel wrote:
Instead of manipulating the array, it may be easier to just introduce an offset when you read the array.

I thought of the offset, but I run into an issue in the next phase of the project.  I must know what color is where.  I will explain later.

 

david.prentice wrote:
/quote]

uint8_t array[33] = "David Prentice is a jolly nice chap.";
uint8_t mvdwn[33] = "id Prentice is a jolly nice chap.Dav";
uint8_t movup[33] = "ap.David Prentice is a jolly nice ch";

Yes, thats what I want to do.  And I agree David Prentice is a rather nice chap...who once in a while needs a cup of teawink

 

david.prentice wrote:
Then you look at the edge cases.  e.g. which array element gets overwritten.  how do you solve it?   perhaps save in temporary variable at start. likewise wrapping a circular buffer is simple.   if (i >= size) i -= size;   

THats what I am working out on the P.C. right now.  I can see  if (i >= size) i -= size; , but I don't quite understand it...lack of programming practice.

 

david.prentice wrote:
There are C library functions like memmove() that will look after the move direction. But they work on flat memory areas rather than circular buffers.

clawson wrote:
As David says there is memmove() and memcpy() but you don't really want to be moving large chunks of data around.

I did indeed look at these as options and with each buffer being only 15 bytes long(5 pixels, each controlled by three bytes) memmove() and memcpy() might be the way to go.

 

 

Let me see if I can explain what I need to do without leaving you all scratching your heads saying WTH.

 

I have three rows of five NEO pixels.  I need to be able to move the three rows left or right three spots individually

 

Example:

Row 3 = 1, 2, 3, 4, 5

Row 2 = 1, 2, 3, 4, 5

Row 1 = 1, 2, 3, 4, 5

 

I now shift row 1 three spots, leaving the other two alone:

Row 3 = 1, 2, 3, 4, 5

Row 2 = 1, 2, 3, 4, 5

Row 1 = 3, 4, 5, 1, 2

 

Each shift needs to be displayed as the shift occurs as well.  Meaning after each single shift I need to show this on the display so it looks fluid, not jumping around.

Like this:

Row 3 = 1, 2, 3, 4, 5

Row 2 = 1, 2, 3, 4, 5

Row 1 = 5, 1, 2, 3, 4

DISPLAY

Row 3 = 1, 2, 3, 4, 5

Row 2 = 1, 2, 3, 4, 5

Row 1 = 4, 5, 1, 2, 3

DISPLAY

Row 3 = 1, 2, 3, 4, 5

Row 2 = 1, 2, 3, 4, 5

Row 1 = 3, 4, 5, 1, 2

DISPLAY

 

This needs to occur for each row,

 

Why I need to manipulate the array as opposed to the pointers is because I also need to shift the colors UP as well.  Meaning led 5 in row 1 can shift UP to Row 2 and then to Row 3.  THAT is somewhat simple enough by just a couple of lines of code that say:

row3[5] = row2[5];
row2[5] = row1[5];
row1[5] = row1[1];

The last line is deliberate by getting element [1].  Its just an example.

 

So, hence why I need to rotate the array as opposed to offsetting the pointers.  I would think it simpler to let the hardware store whats what and simply shift things around.

 

In total there are 19 LED's.  9 on one face, 6, on another, and 4 on the third.  Picture half of a box's 6 sides covered in LEDs.  Albeit these are not even, but the unit is already built and its not possible to make it even out - Once I have it doing what is expected I'll post a video.

 

Thanks!

JIm

If you want a career with a known path - become an undertaker. Dead people don't sue! - Kartman

Please Read: Code-of-Conduct

Atmel Studio6.2/AS7, DipTrace, Quartus, MPLAB user

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

@Jim,
.
If you only have a 15 byte array, I would just work from a copy array. You can afford an extra 15 bytes of SRAM with any AVR.
If you had monster patterns, you would need to be careful with SRAM.
.
Just copy from your Master array to the Work array with whatever circular offsets that take your fancy.
.
Yes, you can manipulate your Master array but it gets quite fiddly i.e. directions, overwritten locations to save, ...
Let's face it. Your Master array could even live in Flash. Your Work array in SRAM.
.
Will we see some new graphics in Times Square?
.
David.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

jgmdesign wrote:
I can see  if (i >= size) i -= size; , but I don't quite understand it

Yes, that can be confusing at first sight.

 

The straight-forward way would be to just

if (i >= size) i = 0;

making the index wrap. In this case it might well suffice.

 

But in general, when you want a counter, index or similar wrap around there might be situations where the counter might be increased twice and in that situation you might want to "stay in phase" and actually reflect that two increments have been seen. So instead of setting to zero you subtract the size of the array from the index. Note that in cases where no such double-increments occur it will be equivalent to setting the index to zero.

 

As an example, consider if you're counting seconds. Somewhere a variable for that is being incremented, maybe in an ISR. Then, potentially somewhere else, you want to check if a full minute has passed and if so increment a minute variable. But to safeguard against the case of the seconds variable has been incremented twice since the last check you don't just want to set the seconds variable back to zero. You want to subtract exactly the number of seconds that you are carrying over to the minutes variable. I.e.:

 

if (seconds >= 60) {
    minutes++;
    seconds -= 60;
}

 

As of January 15, 2018, Site fix-up work has begun! Now do your part and report any bugs or deficiencies here

No guarantees, but if we don't report problems they won't get much of  a chance to be fixed! Details/discussions at link given just above.

 

"Some questions have no answers."[C Baird] "There comes a point where the spoon-feeding has to stop and the independent thinking has to start." [C Lawson] "There are always ways to disagree, without being disagreeable."[E Weddington] "Words represent concepts. Use the wrong words, communicate the wrong concept." [J Morin] "Persistence only goes so far if you set yourself up for failure." [Kartman]

Last Edited: Sat. Jan 13, 2018 - 08:33 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

david.prentice wrote:
Yes, you can manipulate your Master array but it gets quite fiddly i.e. directions, overwritten locations to save, ... Let's face it. Your Master array could even live in Flash. Your Work array in SRAM.

 

After looking at things in reality I have three arrays, that after I have done all of my shifts and such can simply be sent out to the NEO's and then updated.  Since each array is so small it is simpler to just manipulate the array(memcpy() or the like).

 

I will have to play around to see how this is all going to work out.

 

david.prentice wrote:
Will we see some new graphics in Times Square?

LOL, hardly.  If I was ever to do something like that I would probably hire The King as he is working with NEOpixels on a much bigger scale than I am for sure.

 

@johan,

Thanks for the explanation..

 

Jim

If you want a career with a known path - become an undertaker. Dead people don't sue! - Kartman

Please Read: Code-of-Conduct

Atmel Studio6.2/AS7, DipTrace, Quartus, MPLAB user

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Success!  With an "I DON'T GET IT"...

 

This is what I came up with:

 while(1)
 {
    //shift the colors
    int j = 3;  //simple counter start value

        __uint24 temp = pixelclr[4];  //save last element in array to temporary location

            for(int i = 4; i >=0; i--)
            {
              pixelclr[i] = pixelclr[j];  //copy one pixelclr location to another
              j--;    //decrement counter
            }

            pixelclr[0] = temp;   //put last element in array in first

      //display results
     count = 0;
     ledclr = 0;

     for(count = 0; count < 5; count++)
     {
     leds.setPixelColor(count, pixelclr[ledclr]); //load LEDs with color pattern from array
     ledclr++;
     }
     leds.show();   //show the pattern
     delay(2000);   //wait 2 seconds

 }

But the compiler spit this back:

X:\SERVER_Data_Drive\Datasheets\Displays\51f1806cce395fcd20000004\Arduino\WS2812_Breakout_Example\WS2812_Breakout_Example.ino: In function 'loop':

X:\SERVER_Data_Drive\Datasheets\Displays\51f1806cce395fcd20000004\Arduino\WS2812_Breakout_Example\WS2812_Breakout_Example.ino:81:39: warning: iteration 4 invokes undefined behavior [-Waggressive-loop-optimizations]

               pixelclr[i] = pixelclr[j];  //copy one pixelclr location to another

                                       ^

X:\SERVER_Data_Drive\Datasheets\Displays\51f1806cce395fcd20000004\Arduino\WS2812_Breakout_Example\WS2812_Breakout_Example.ino:79:13: note: containing loop

             for(int i = 4; i >=0; i--)

             ^

C:\Program Files (x86)\Arduino\hardware\arduino\avr\cores\arduino\main.cpp: In function 'main':

X:\SERVER_Data_Drive\Datasheets\Displays\51f1806cce395fcd20000004\Arduino\WS2812_Breakout_Example\WS2812_Breakout_Example.ino:81:39: warning: iteration 4 invokes undefined behavior [-Waggressive-loop-optimizations]

               pixelclr[i] = pixelclr[j];  //copy one pixelclr location to another

                                       ^

X:\SERVER_Data_Drive\Datasheets\Displays\51f1806cce395fcd20000004\Arduino\WS2812_Breakout_Example\WS2812_Breakout_Example.ino:79:13: note: containing loop

             for(int i = 4; i >=0; i--)

             ^

 

I don't get what the use of '4' would invoke undefined behavior.

 

Jim 

 

EDIT:

Even though I am getting the warnings, I came to the point I am at of a working model by employing David Prentices all to often advice of using a P.C. - Paper Computer

 

Sometimes it becomes clearer when you WRITE it, rather than TYPE it.  Gotta LOVE erasers too.

If you want a career with a known path - become an undertaker. Dead people don't sue! - Kartman

Please Read: Code-of-Conduct

Atmel Studio6.2/AS7, DipTrace, Quartus, MPLAB user

Last Edited: Sun. Jan 14, 2018 - 05:39 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

This is the code for your pencil work!

for your example max = 15

rotate_right_rgb(int a[], int max)
{

int i;
int r, g, b;

r = a[max - 3];
g = a[max - 2];
b = a[max - 1];

for (i = max - 1; i > 2; i--)
{
a[i] = a[i - 3];
}

a[0] = r;
a[1] = g;
a[2] = b;

}

Majid

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Success!

I doubt that, Jim. [EDIT: A build with warnings is not a clean build]

With an "I DON'T GET IT"..

[..]

warning: iteration 4 invokes undefined behavior [-Waggressive-loop-optimizations]

 

On the last iteration of the loop i will be 0 and j will be -1!

 

Simple test/proof run on a PC:

#include <stdio.h>
#include <stdlib.h>

int main(int argc, char** argv) {

    int j = 3;

    for(int i = 4; i >=0; i--) {
        printf("%i  %i\n", i, j);
        j--;
    }

    return 0;
}

Output:

4  3
3  2
2  1
1  0
0  -1

 

EDIT/ADDENDUM: You're simply looping one step too far. The last move will be of "the element -1 to element 0". This is benign in this case since right after the loop the correct (previous last) element is copied into element 0. 

 

So, change loop to

for(int i = 4; i >0; i--) {

notice > rather than >=.

 

 

As of January 15, 2018, Site fix-up work has begun! Now do your part and report any bugs or deficiencies here

No guarantees, but if we don't report problems they won't get much of  a chance to be fixed! Details/discussions at link given just above.

 

"Some questions have no answers."[C Baird] "There comes a point where the spoon-feeding has to stop and the independent thinking has to start." [C Lawson] "There are always ways to disagree, without being disagreeable."[E Weddington] "Words represent concepts. Use the wrong words, communicate the wrong concept." [J Morin] "Persistence only goes so far if you set yourself up for failure." [Kartman]

Last Edited: Sun. Jan 14, 2018 - 09:37 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

@Johan,
Success to me is getting a better result than what I started with....I started with nothing working, I now have something that is doing what I wanted(with a WTH, but that's ok). It's better than what I had, so I would call this a success of a sorts YOMD.

But I thank you for pointing out what is causing the warning, and I agree as I have with you in other threads that warnings are not clean builds....errors waiting to happen. I will implement the fix and test a little later.

@m.majid
The whole purpose of that picture(there is an error or two in it but that's ok) was to demonstrate a common piece of advice given by a respected community member, that is to work out the problem on paper rather than stare at the screen, typing aimlessly in the hope of it coming together. We get countless threads where the OP posts snippets after snippets begging explanations on why, or how this does or does not do something when taking a few minutes...or a few hours working it out the old fashioned way saves a lot of headaches.

Thanks for posting your dissection as well, although it probably is flawed if you followed exactly what was in my picture

Jim

If you want a career with a known path - become an undertaker. Dead people don't sue! - Kartman

Please Read: Code-of-Conduct

Atmel Studio6.2/AS7, DipTrace, Quartus, MPLAB user

Last Edited: Sun. Jan 14, 2018 - 02:01 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

m.majid has shown you what to do.   Note that you have to save 8-bit r, g, b.

 

If you work in uint24_t arrays,  you would only save a single 24-bit rgb.

 

Rotating one position left or right is solved by m.majid.

 

I suspect that you will want to do several transformations on your LED patterns.    Handling a pointer to a circular buffer is very easy.   You can move any number of positions without fear.

 

David.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

jgmdesign wrote:
The whole purpose of that picture(there is an error or two in it but that's ok) was to demonstrate a common piece of advice given by a respected community member, that is to work out the problem on paper rather than stare at the screen,

Ofcourse, me too use drawing when working on algorithms, very useful to understand and describe

Regards

Majid

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

David
I only want to save a single 24bit RGB in the temp location as I move the remaining components over one place then place the temp back.

As far as pointers go yes I am thinking of using them but for this very simple application it's not a necessity at the moment. Just having it working is enough

Jim

If you want a career with a known path - become an undertaker. Dead people don't sue! - Kartman

Please Read: Code-of-Conduct

Atmel Studio6.2/AS7, DipTrace, Quartus, MPLAB user

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

m.majid wrote:
This is the code for your pencil work!

or your example max = 15

rotate_right_rgb(int a[], int max)
{

int i;
int r, g, b;

r = a[max - 3];
g = a[max - 2];
b = a[max - 1];

for (i = max - 1; i > 2; i--)
{
a[i] = a[i - 3];
}

a[0] = r;
a[1] = g;
a[2] = b;

}

I have to admit that I do not understand this very well.. I saw the:

int a[]

in my searches, but I am not too sure what that means exactly...I am more a hardware person, and as well known here, my C is good enough to get me by most of the time.  But I like to learn!

 

Will have to create a new project to explore this.

 

@David:

david.prentice wrote:
I suspect that you will want to do several transformations on your LED patterns.    Handling a pointer to a circular buffer is very easy.   You can move any number of positions without fear.

I will be exploring this as well...It too came up in the searches before I posted here.  

 

Thanks all,

Jim

If you want a career with a known path - become an undertaker. Dead people don't sue! - Kartman

Please Read: Code-of-Conduct

Atmel Studio6.2/AS7, DipTrace, Quartus, MPLAB user

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

jgmdesign wrote:
I have to admit that I do not understand this very well.. I saw the:

int a[]

in my searches, but I am not too sure what that means exactly...

 

In that context, essentially the same as

int * a

 

Since passing an array as a parameter to a function is actually passing the address of the first element (and the pointer then can be used as an array in the function) the latter notation is used by some.

 

The first notation makes it clearer that an array is expected, rather than just any pointer to int. So, one coding habit might be:

 

  • Pass a pointer to a single something as func(something * p)
  • Pass an array of somethings as func(something p[])

 

 


 

Having the array "fixed" and traversing it with an index or a pointer will be more efficient - more so the bigger the array gets.

As of January 15, 2018, Site fix-up work has begun! Now do your part and report any bugs or deficiencies here

No guarantees, but if we don't report problems they won't get much of  a chance to be fixed! Details/discussions at link given just above.

 

"Some questions have no answers."[C Baird] "There comes a point where the spoon-feeding has to stop and the independent thinking has to start." [C Lawson] "There are always ways to disagree, without being disagreeable."[E Weddington] "Words represent concepts. Use the wrong words, communicate the wrong concept." [J Morin] "Persistence only goes so far if you set yourself up for failure." [Kartman]

Last Edited: Sun. Jan 14, 2018 - 10:19 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Sorry I mixed c# code!
İn c# its not allowed to define pointers using *

İ m not sure here is true, i meant int * here

Majid

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

JohanEkdahl wrote:
EDIT/ADDENDUM: You're simply looping one step too far. The last move will be of "the element -1 to element 0". This is benign in this case since right after the loop the correct (previous last) element is copied into element 0.    So, change loop to for(int i = 4; i >0; i--) { notice > rather than >=.

 

Removing the '=' removed the warning and all is well in the world.  Now on with vertical movements...

 

Will keep thread updatred

 

Thanks everyone

 

Jim

If you want a career with a known path - become an undertaker. Dead people don't sue! - Kartman

Please Read: Code-of-Conduct

Atmel Studio6.2/AS7, DipTrace, Quartus, MPLAB user