## Solved: need a bit of math help with an array

32 posts / 0 new
Author
Message

I have this

for (int d = 0; d < 192; d++)
{
sendData( tmp = *(data + (d&0xfc) + (3-(d&0x03))) );//LCD Data
}

data is an array 192 in size.

I load my array with eeprom

for ( unsigned char i = 0; i < 192;i++)
{
}

I want to rid of the array and just read right from the eeprom.

!) do I need to add a delay to do this?

2) What does this crazy stuff mean (it was sampled)?  Looks like it takes a pointer to data at some specific addresses. I believe the data needs to be assembled 4,3,2,1,8,7,6,5..... How could I rewire this to use my eeprom instead of the array. I guess I could make an array and load it in 4 byte at a time but I'd like to ovoid using arrays all together. sendData is just a byte to wire transmission for a proprietary protocol.

Last Edited: Wed. May 16, 2018 - 07:37 PM

You have a choice - either shuffle the data before you put it into the eeprom or shuffle it when you read the data from the eeprom. As to what makes more sense, i can’t comment as you’ve not given enough context.

Well, 0xFC means your are keeping the top 6 bits of d (d varies 0-191) as-is and taking the lowest 2 bits (0-3) & "reversing"  them 3-0=3  (00-->11)   , 3-1=2 (01--10),  3-2=1 (10-->01), 3-3=0  (11-->00)

Consider the 6 bits as a "batch" count, 6 bits is enough for 64 batches  (if d went to 255), but with 191 max, that's 48 batches (including batch  zero).

So its grabbing a group of 4 & reversing their order, then doing the same with the next batch of 4 & the next batch of 4...for 48 batches (groups) of bytes...why??? don't know

When in the dark remember-the future looks brighter than ever.   I look forward to being able to predict the future!

You have a choice - either shuffle the data before you put it into the eeprom or shuffle it when you read the data from the eeprom. As to what makes more sense, i can’t comment as you’ve not given enough context.

Ok let me rephrase this.

My goal here is to save on code ram and space.

So I'm just guessing putting it in to an array is just wasted code (maybe the compiler is smarter then that?). The reason for the array was so that I could load the array in another cpp file, thus it was global. Now it's all down within one file so there is no need for the array (the global array is replace with eeprom). Yet again, if removing the array will not save me anything, then I don't need to worry.

So for example I could do this and be done with it.

for (int d = 0; d < 192; d++)
{

data[i]= eeprom_read_byte(160+ d);//data defined above for
sendData( tmp = *(data + (d&0xfc) + (3-(d&0x03))) );//LCD Data
}

I guess to me aesthetically, I see no need for data array, but again, I have no clue what all this bitmask addition is for.

Last Edited: Mon. May 14, 2018 - 11:11 PM

"why??? don't know" Ok that explanation helps. -- 48 by 32 is the resolution for the lcd and as I said I need to write the data to the device like  4,3,2,1,8,7,6,5.... I guess what confuses me is the data, I woudl have accepted data [d].

so simply doing

sendData(  eeprom_read_byte(160+ i)+ (d&0xfc) + (3-(d&0x03)) );//LCD Data

is not going to work.

Last Edited: Mon. May 14, 2018 - 11:17 PM

You’ve confused the indexing with the data. I’m on i tablet so i can’t correct the code easily, but change where the brackets are and i makes no sense as d is the variable of interest.

Oh I right I have an i in there sorry. I think I follow but maybe I need a bit of help.

sendData(  eeprom_read_byte( ((160+ d)&0xfc) + (3-((160+ d)&0x03)) ));//LCD Data

?

Last Edited: Mon. May 14, 2018 - 11:25 PM

Closer. I gather 160 is the base address of the array in eeprom. Add that last.

Right, the data starts at eeprom 160.

sendData(  eeprom_read_byte( (d&0xfc) + (3-(d&0x03))  + 160 ));//LCD Data

?

Last Edited: Mon. May 14, 2018 - 11:29 PM

Worked! Thank you Kartman.

avrcandies wrote:
So its grabbing a group of 4 & reversing their order, then doing the same with the next batch of 4 & the next batch of 4...for 48 batches (groups) of bytes...why??? don't know

Which looks suspiciously like a big/little endian conversion to me.

Instead of swapping all those bytes each time it seems much more logical to put them in eeprom in the same order as the LCD wants them.

Doing magic with a USD 7 Logic Analyser: https://www.avrfreaks.net/comment/2421756#comment-2421756

Bunch of old projects with AVR's: http://www.hoevendesign.com

Paulvdh wrote:

avrcandies wrote:
So its grabbing a group of 4 & reversing their order, then doing the same with the next batch of 4 & the next batch of 4...for 48 batches (groups) of bytes...why??? don't know

Which looks suspiciously like a big/little endian conversion to me.

Instead of swapping all those bytes each time it seems much more logical to put them in eeprom in the same order as the LCD wants them.

Currently I use the eeprom write and eeprom read. Data is sent in via usb. Won't the cost vs prepare and extract be the same?

Depends on the balance of prepares vs extracts. If you're going to be reading it a number of times, currently you pay the price of the shuffle each time. If you pre-shuffle when you store it, then you've only got that cost once and considering it is slow to write eeprom, then the extra cost should be minimal. Of course, this depends on whether the cost of the shuffle each time you read is a problem. If you're not pushed for time, then it is probably not an issue.

just because it looks nice in C don't mean it is efficient, (not that I think this is bad), but I would also try with two for loops one with +=4 and one with -=1 as step, and keep the variables 8 bit.

it's the same with your code d only need to be 8 bit.(so do the correction in 8 bit and then call read with 160+correction )

the low 2 bits:

00 -> 11

01 -> 10

10 -> 01

11 -> 00

That is the same that EOR with 11  if it helps (it would in ASM)
or just use the COM instruction.

other way

perhaps have union with 4 8bit and 1 32 then read a long and then used those 4 byte in reverse order

`        sendData( tmp = *(data + (d&0xfc) + (3-(d&0x03))) );//LCD Data`

Wonder who writes code like this? For one thing what is the purpose of "tmp" here? It is assigned and yet nothing in the loop uses it, then it is over-written on the next iteration. So that does not need to be there:

`        sendData( *(data + (d&0xfc) + (3-(d&0x03))) );//LCD Data`

then this next construct:

`*(data + offset)`

would usually be written by most sane humans as:

`data[offset]`

so surely it's:

`        sendData( data[ (d&0xfc) + (3-(d&0x03)) ] );//LCD Data`

then who writes C operators without space around them. So:

`        sendData( data[ (d & 0xfc) + (3 - (d & 0x03)) ] );//LCD Data`

As others have said the rest of this is just make a 6:2 split of the address (and wonder what prompted the author to use "d" for and address/index ?). The easiest way to see the pattern is simply to write:

```#include <stdio.h>

int main(void) {
int d;
for (d = 0; d < 192; d++) {
printf("%d, %d\n", d, (d & 0xfc) + (3 - (d & 0x03)));
}
return 0;
}```
```D:\c>cl sequence.c
Microsoft (R) 32-bit C/C++ Optimizing Compiler Version 16.00.40219.01 for 80x86

sequence.c
Microsoft (R) Incremental Linker Version 10.00.40219.01

/out:sequence.exe
sequence.obj

D:\c>sequence
0, 3
1, 2
2, 1
3, 0
4, 7
5, 6
6, 5
7, 4
8, 11
9, 10
10, 9
11, 8
etc.
```

So, that confirms what others have said - it's doing groups of 4: 3,2,1,0 then 7,6,5,4 etc.

But talk about obfuscation! Why not simply write what is actually required here:

```typedef union {
uint32_t word;
uint8_t bytes[4];
} combined_t;

combined_t output;

int i, j;

for (i = 0; i < 48; i++) {
// read a 4 byte group
// send the 4 bytes in reverse order
for (j = 3; j >= 0; j--) {
sendData(output.bytes[j]);
}
}```

Unlike the original code there is no obfuscation here. No clever counting sequence that has to be "worked out" by the maintainer. The maintainer can simply read this code and see exactly what is going on and what the author's original intention was.

I haven't built it but I'd even go so far as to say that I doubt it is actually going to be any more code than the original two for() loops and it drops the costly array.

The point of authoring code is that you should just write what you want to achieve. It makes the maintainer's job easier and most of the time "simple" code that simply expresses the designed algorithm is the "best" solution anyway. Sure, if it is built and does look "costly" then consider applying some optimisation to the process. But writing clean, simple code first is perhaps the most important goal - especially when you have to come back to this in 18 months to fix/improve it and you haven't a clue what all that 3 - (d & 0x03) stuff is on about. Worse still is when someone else has to come to your code in 18 months and add/fix it!!

If the code had been written as I just suggest I bet this thread would never have existed ;-)

To make it clear

(d&0xfc) + (3-(d&0x03))

Is the same as

d^3

Well, then I'm not going crazy. Thank for braking this all down. I can not speak for the author but the tmp was for parity. I just left that part out. The union is nice, I like that.

Kartman, yeah its one read per boot. So seldom does the chip need to read this eeprom.

Last Edited: Tue. May 15, 2018 - 12:08 PM

Note that some would say the union technique is "naughty" as it is implementation dependent. I am using "hidden" knowledge that an AVR happens to store a uint32_t in little endian order. The true way to access the 4 bytes in the 32 bit var is with a bunch of >>24, >>16, >>8 stuff and perhaps some & 0xFF's thrown in for good measure. That solution works regardless of endianism (and other potential "nasties" like packing etc). If one was lucky the compiler will see something like "(n32 >> 16) & 0xFF" and simply know it means "lift out one of the 4 bytes" and will simply generate the most efficient code anyway - chances are it has n32 in R25:R24:R23:R22:R21 anyway so it would just take R24. But my union just embodies the logic of all that and n32.bytes[2] should just lift out the chose 8 bits in much the same way.

Last Edited: Tue. May 15, 2018 - 12:03 PM

I like something like this with i as uint8_t

for (i = 0; i < 192; i++) {
}

That should generate a small code and low RAM use

or let i be a uint16_t and then let it run from 160 (add: would not work if 160 %4 != 0)

for speed perhaps the read of 32 bit at the time is faster!

Last Edited: Tue. May 15, 2018 - 12:40 PM

Space is preferred over speed.  I only read this once, its more of a customization splash screen on boot.

If you're not going to follow Cliff's advice then at least write some comments as to what this code is actually expected to do. You will probably have forgotten the fine details by tomorrow.

I just read the whole thead and am struggling to verify sparrow2 modification by merely reading the code. Actually I don't think it's right ... but then of course it may be. I just can't easily tell having already forgotten what is the expected behaviour.

BTW: What is the 160 all about ?

Edit: Why doesn't spell check work in the posting editor ?

Last Edited: Tue. May 15, 2018 - 12:38 PM

I do like Cliff advice, its very readable and efficient. I have yet to test any code and will not have a chance till later today.

BTW: What is the 160 all about ?

___________________________________________________________________

I gather 160 is the base address of the array in eeprom. Add that last.

Right, the data starts at eeprom 160.

All the code does is takes data out of eeprom and  write to an LCD screen device. The device is a game controller and it expects the certain endianess to be sent ( as explained above bytes in 4's reversed order)

4,3,2,1,8,7,6,5, etc... (48 x 32 )total.

Last Edited: Tue. May 15, 2018 - 12:46 PM

Look at #15 that is what you want.

But yes that will also need a comment.

This is a comment to #22

Last Edited: Tue. May 15, 2018 - 12:43 PM

N.Winterbottom wrote:
I just read the whole thead and am struggling to verify sparrow2 modification by merely reading the code.
That's exactly why I do things like:

```#include <stdio.h>

int main(void) {
int d;
for (d = 0; d < 192; d++) {
printf("%d, %d, %d\n", d, (d & 0xfc) + (3 - (d & 0x03)), d ^ 3);
}
return 0;
}```
```D:\c>sequence.exe
0, 3, 3
1, 2, 2
2, 1, 1
3, 0, 0
4, 7, 7
5, 6, 6
6, 5, 5
7, 4, 4
8, 11, 11
9, 10, 10
10, 9, 9
11, 8, 8
12, 15, 15
13, 14, 14
14, 13, 13
15, 12, 12
etc```

If one is ever in any doubt about such things (who would ever doubt sparrow? ;-) then a one liner like this proves things in about 30 seconds.

And my test was :

=BITXOR(A1;3)

on a row of numbers in excel :)

I guess I should add that there is nothing odd in my solution.

We know that one's complement reverse the order (ASM instruction COM), and that is the same a flip all bits.

EOR with 0xff will also flip all bits so that is the same.

The only bit out of the box part is that EOR with 0x03 will do a one's complement on only the last 2 bit, and do nothing to the rest.

For the lowest hit on memory and space the one's complement wins

As Borat is apt to say "Great Success!"

>> We know that one's complement reverse the order (ASM instruction COM)

Actually, that’s the part I missed. A bit embarrassing, since I noticed that inverters would make an up counter into a down counter in hardware, a long time ago!

I’m a little surprised that i havent seen it show up in networking code.

Last Edited: Wed. May 16, 2018 - 03:10 AM

I'm actually surprised that I haven't seen this solution before!

Not because of this problem, but that this is a cool way to flip between data stored in little and big endian.

no need of a  <if> just a variable that either have the value 0 or 3 for 32 bit and 0 or 1 for 16 bit

have in mind that data needs to be aligned for it to work.