Programming issue

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

  Hello everyone

 

  I don't know what topic name should I give it. I am working on a small project in AS7, Mega328P, in fact an Arduino Uno board. I got stuck using arrays and ran out of ideas. Here is a simplified part of my code to show the problem.

/*
 * test1.c
 *
 * Created: 2019-10-25 13:11:41
 * Author : User
 */

#include <avr/io.h>
#include <string.h>

uint32_t ui32MyVar;
uint8_t ui8ArRxTx[600];
uint8_t ui8ArGraph[1089];

int main(void)
{
    /* Replace with your application code */
    DDRD = 2; // set Tx pin as output
    UCSR0A = (1 << U2X0);
    UCSR0B = (1 << RXCIE0 | 1 << RXEN0 | 1 << TXEN0);
    UCSR0C = (1 << UCSZ01 | 1 << UCSZ00);
    UBRR0 = 207; // 9600 kbps
    
    ui8ArRxTx[42] = 0xE1;
    ui8ArRxTx[43] = 0x5D;
    ui8ArRxTx[44] = 0xA7;
    ui8ArRxTx[45] = 0xC0;
    
    ui32MyVar = ui8ArRxTx[42] * 16777216 + ui8ArRxTx[43] * 65536 + ui8ArRxTx[44] * 256 + ui8ArRxTx[45];    
//    ui32MyVar = ui8ArRxTx[42] * 16777216UL + ui8ArRxTx[43] * 65536UL + ui8ArRxTx[44] * 256 + ui8ArRxTx[45]; // second try
//    ui32MyVar = (ui8ArRxTx[42] << 24) + (ui8ArRxTx[43] << 16) + (ui8ArRxTx[44] << 8) + ui8ArRxTx[45]; // third try
    
    memcpy(ui8ArGraph + 1082, &ui32MyVar, 4);
    
    UDR0 = ui8ArGraph[1082];
    while ((UCSR0A & (1 << UDRE0)) == 0) {}
    UDR0 = ui8ArGraph[1083];
    while ((UCSR0A & (1 << UDRE0)) == 0) {}
    UDR0 = ui8ArGraph[1084];
    while ((UCSR0A & (1 << UDRE0)) == 0) {}
    UDR0 = ui8ArGraph[1085];
    while ((UCSR0A & (1 << UDRE0)) == 0) {}
    
    while (1)
    {
    }
}

 

  I get in a serial terminal:  C0 A7 5C E1.

 

  I do have three question:

1. The obvious one, why 5D changes to 5C ? This is the problem in my real project.

 

2. If I use the second line (commented out), nothing changes, so the question is do I need to add "UL" in there or not ?

 

3. At first, I used shifts, not multiplications. The compiler complains that the resulting shifted value is larger than the recipients size. Why ? I want to shift an 8bit value 24 positions to the left and place it in a 32 bit variable. Anyway, the result is that bytes 2 and 3 come out as FF.

 

Thank you.

 

Edited: typo

This topic has a solution.
Last Edited: Fri. Oct 25, 2019 - 07:51 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

This won't answer your question, but why do you need to combine the bytes in the first place (lots of overhead).  If you have the bytes, just move them to where you want them, or just send them out.

Or were you just moving/sending them just to see that they were combining (calculating) correctly?

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

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

  The .lss file shows:

ui8ArRxTx[42] = 0xE1;
  b0:	e0 e0       	ldi	r30, 0x00	; 0
  b2:	f1 e0       	ldi	r31, 0x01	; 1
  b4:	81 ee       	ldi	r24, 0xE1	; 225
  b6:	82 a7       	std	Z+42, r24	; 0x2a
	ui8ArRxTx[43] = 0x5D;
  b8:	8d e5       	ldi	r24, 0x5D	; 93
  ba:	83 a7       	std	Z+43, r24	; 0x2b
	ui8ArRxTx[44] = 0xA7;
  bc:	87 ea       	ldi	r24, 0xA7	; 167
  be:	84 a7       	std	Z+44, r24	; 0x2c
	ui8ArRxTx[45] = 0xC0;
  c0:	80 ec       	ldi	r24, 0xC0	; 192
  c2:	85 a7       	std	Z+45, r24	; 0x2d
	
	ui32MyVar = ui8ArRxTx[42] * 16777216 + ui8ArRxTx[43] * 65536 + ui8ArRxTx[44] * 256 + ui8ArRxTx[45];	
  c4:	40 ec       	ldi	r20, 0xC0	; 192
  c6:	57 ea       	ldi	r21, 0xA7	; 167
  c8:	6c e5       	ldi	r22, 0x5C	; 92
  ca:	71 ee       	ldi	r23, 0xE1	; 225
  cc:	40 93 58 03 	sts	0x0358, r20
  d0:	50 93 59 03 	sts	0x0359, r21
  d4:	60 93 5a 03 	sts	0x035A, r22
  d8:	70 93 5b 03 	sts	0x035B, r23

  At PC = b8 it correctly initializes the first array. However, at PC = c8, it loads the wrong value 0x5C instead 0x5D. I use optimization -O1. It correctly recognizes the magic number for the multiplication so the multiplication does not takes place as expected.

 

  This is a UnixTime value. I get it from a time server and I want to increment it every second to have it ready when a client asks for it. It took me half day to narrow it down to this. If I copy it directly it works, but then I can't increment it. Without optimization the result is still wrong, so I may be wrong instead.

 

 

This reply has been marked as the solution. 
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 2

What you are seeing demonstartes some of the more subtle points of C, amongst other things it involves

what is the type of a decimal constant,

integer promotions and usual arithmetic conversions,

signed integer overflow.

 

The 5D changing to 5C ultimately comes from signed integer overflow here

angelu wrote:

ui8ArRxTx[44] * 256 

the type of the decimal contant 256 is int, that's equivqluent to int16_t on this platform.

So you have 0xa7 * 256 = 0xa700 = 42752

but because this particular multiply is done as int16_t, it has overflowed signed 16 bit range and so becomes -22784.

 

The multiply by 12777216 is done as type long (int32_t) because that's the type of the decimal constant 12777216.

Likewise the multiply by 65536.

 

Then you do the adds, starting from left you have

int32_t + int32_t + int16_t

where the int16_t is the 0xa700 value (-22784)

So this int16_t gets converted to int32_t for the add, so it becomes 0xffffa700.

 

If in your 2nd version you add U (or UL)  onto 256

ui8ArRxTx[44] * 256U

then I expect you get the right answer.

 

Personally I would do the shifts, but then you need some casts.

If you do this

ui8ArRxTx[42] << 24

the ui8 is promoted to type int for the shift, and int is only 16 bits, and then you shift by 24 which is out of range.

So you need some casts

    ui32MyVar = ((uint32_t)ui8ArRxTx[42] << 24) + ((uint32_t)ui8ArRxTx[43] << 16) + ((uint32_t)ui8ArRxTx[44] << 8) + (uint32_t)ui8ArRxTx[45]; // third try
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 1

@ MrKendo Thank you for your detailed reply.

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

angelu wrote:
2. If I use the second line (commented out), nothing changes, so the question is do I need to add "UL" in there or not ?
A linter is producing 'loss of sign' for each of the three tries.

 

P.S.

memcpy - A linter outputs a type mismatch for argument one (uint8_t vs uint32_t)

 


PC-lint Plus Online Demo - Gimpel Software - The Leader in Static Analysis for C and C++ with PC-lint Plus

 

"Dare to be naïve." - Buckminster Fuller

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

This is also one of those cases where if you ran the same  code on a platform with 32 bit ints you would get the right answer

(looking at it again, you also have signed overflow in the 1st term if written as ui8ArRxTx[42] * 16777216, which is not a good thing, but that wasn't causing the 5d to 5c problem).

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

gchapman wrote:

P.S.

memcpy - A linter outputs a type mismatch for argument one (uint8_t vs uint32_t)

What I understand is the memcpy is blindly copying  a number of bytes from one address to another no matter what is there, which I kinda like it. Or is there a better way to put/extract an unsigned 32 into an array to a particular location ?

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

Resolved that linter's informational message by replacing '+ 1082' with an index.

 

"Dare to be naïve." - Buckminster Fuller

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

  So you have uint8_t x int16_t and the compiler performs this as 16 bit multiplication ? It has every chance to overflow. It needs 24 bits. This is awful.

 

 Seeing 256 as signed int when multiplied by an uint8_t is awful too.

 256 * uint8_t needs 24 bits. Does it fit in an uint32_t ? Yes. Next please.

 

  Promoting an uint8_t to an int16 when shifted is awful as well. It looks archaic. Any (uint8_t << up to 8) should fit in an uint16_t let alone uint32_t.

 

  I don't have these issues in Pascal. The only thing I need to worry in Pacal in these cases is if the end result fits in the destination.

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

gchapman wrote:
Resolved that linter's informational message by replacing '+ 1082' with an index.

 

If I try with:

memcpy(ui8ArGraph[1082], &ui32MyVar, 4);

I get a warning: passing argument 1 of 'memcpy' makes pointer from integer without a cast

and I don't know what it means.

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

The linter's lack of warning on that may be because its front end is from Clang.

 

"Dare to be naïve." - Buckminster Fuller

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

angelu wrote:
So you have uint8_t x int16_t and the compiler performs this as 16 bit multiplication ?

If you have 16 bit int  (as will be the case for AVR micros) then yes.

If you have 32 bit int (like on a PC or 'bigger' microcontrollers) then no, the arguments are promoted to int and then the multiplication is done as int, which in this case would be 32 bits.

 

angelu wrote:
Promoting an uint8_t to an int16 when shifted is awful as well.
 

Same here. It's promoted to int for the operation So on AVR with 16 bit int it's 16 bits, if you have 32 bit int it's 32 bit.

These are the rules of C, whether you like it or not!

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

angelu wrote:

 

If I try with:

memcpy(ui8ArGraph[1082], &ui32MyVar, 4);

I get a warning: passing argument 1 of 'memcpy' makes pointer from integer without a cast

and I don't know what it means.

 

Memcpy wants two pointers to memory and a length in bytes. You're giving it a value - whatever is in ui8ArGraph[1082] - and then a pointer. If that value is four bytes long, then you probably just want to prefix it with '&' to return the *address* of that data.

 

Neil