|
|
View previous topic :: View next topic |
Author |
Message |
hayee
Joined: 05 Sep 2007 Posts: 252
|
union is not working |
Posted: Thu May 12, 2016 5:54 am |
|
|
Hi
I am using CCS compiler version 4.124 and Pic Controller PIC24HJ128GP306.
I am using union function. Made a simple program for testing but working not as it has to be.
Code: |
#include <24HJ128GP306.h>
#fuses HS,NOWDT,PR
#use delay(crystal=20M)
//!#use rs232(baud=9600, xmit=PIN_F3,rcv=PIN_F2)
#use rs232(baud=9600, xmit=PIN_F5,rcv=PIN_F4)
union input
{
int8 bits_8[4];
int16 bits_16[2];
int32 bits_32;
}value;
void main()
{
//! value.bits_32=0;
//! value.bits_16[0]=0;
//! value.bits_16[1]=0;
//! value.bits_8[0]=0;
//! value.bits_8[1]=0;
//! value.bits_8[2]=0;
//! value.bits_8[3]=0;
value.bits_16[0]=232;
value.bits_16[1]=11;
printf("H=%lu ",value.bits_32);
printf("H=%lu ",value.bits_16[1]);
printf("L=%lu ",value.bits_16[0]);
printf("0=%u ",value.bits_8[0]);
printf("1=%u ",value.bits_8[1]);
printf("2=%u ",value.bits_8[2]);
printf("3=%u ",value.bits_8[3]);
printf("\r\n");
while(TRUE)
{
}
}
|
and checked the output on Hyper Terminal as follows
Quote: |
H=721128 H=11 L=232 0=65512 1=0 2=11 3=0
|
value.bits_8[0] having a value of 65512. How?
What is wrong in my code?
Kindly tell me |
|
|
Ttelmah
Joined: 11 Mar 2010 Posts: 19510
|
|
Posted: Thu May 12, 2016 8:07 am |
|
|
You need to give the union the packed attribute.
The sizes are not what you think. In PCD, if you declare an array of int8 values, by default, each will actually be stored as an int16, so they can be 'word' aligned. Remember an 'int' in PCD, is actually an int16. Accessing bytes on odd boundaries is much harder than on the older chips, especially using an array index.
%u will default to being able to print an int16 as well.
So your int8 array, is actually using 8 bytes, and this is then the size of the union.....
Code: |
union __attribute__((packed)) input
{
int8 bits_8[4];
int16 bits_16[2];
int32 bits_32;
}value;
|
This is one of the 'learning experiences' of dealing with byte data on these larger chips.
Be very careful as well. If you use a int8 *, and then try to access an int16 value using this, and use an odd value, this will cause an address error trap, since accessing an int16 value will always use a word access, and this cannot be done on an odd byte address. |
|
|
jeremiah
Joined: 20 Jul 2010 Posts: 1348
|
|
Posted: Thu May 12, 2016 11:01 am |
|
|
PCD shouldn't have any trouble with the 8bit array. It only buffers 8bit values when they are not word aligned. But an even sized array of 8bits is word aligned so it should be fine. It looks more like a display bug. 232 is 0xE8 in hex. 65512 is 0xFFE8 in hex. Looks like it is displaying a 16bit value, but the actual stored value is 8bits.
Looking at the sym file confirms this:
Code: |
W0 @SCRATCH
W0L _RETURN_
W0 @delay_us1.P1
W0 -W1 @READ_ROM_MEMORY.P2
W0 -W1 @PRINTFU32_9600_5893_5892.P3
W0 -W1 @DIV3232B.P4
W0 @PRINTFU16_9600_5893_5892.P5
W0 @delay_ms1.P6
W1 @SCRATCH
W1 @WRITE_PACKED_MEMORY.P1
W1 @READ_PACKED_MEMORY.P2
W2 @READ_PACKED_MEMORY.P1
W2 @WRITE_PACKED_MEMORY.P2
W2 @READ_ROM_MEMORY.P3
W2 -W3 @DIV3232B.P4
W3 @READ_ROM_MEMORY.P1
W3 @WRITE_PACKED_MEMORY.P2
W3 @READ_PACKED_MEMORY.P3
W4 @PRINTFU32_9600_5893_5892.P1
W4 @PRINTFU16_9600_5893_5892.P2
800-803 value
4780-47FF Stack
|
Only 4 bytes allocated for value. The LST file also shows a MOV.B used for the index in question, so I would wager a printf sign extension bug. |
|
|
Ttelmah
Joined: 11 Mar 2010 Posts: 19510
|
|
Posted: Thu May 12, 2016 2:22 pm |
|
|
%u prints an unsigned int16 by default, not an int8 (on PCD).
int8 on PCD, is by default _signed_.
So it is converting a signed value to it's 16bit equivalent, and then printing this as unsigned.....
The int8 declaration needs to be changed to unsigned int8.
So not a bug, but a type conversion result. |
|
|
Ttelmah
Joined: 11 Mar 2010 Posts: 19510
|
|
Posted: Fri May 13, 2016 1:07 am |
|
|
Just to expand on this. 0xE8, as a signed int8, is -18. -18 (signed int8) converted to a signed int16, is stored as 0xFFE8. This is implicitly done by passing an int8 to the in16 print. 0xFFE8, printed as _unsigned_, is 65512. Exactly what is being seen......
There are two important things here.
First I would always use the packed attribute for an operation like this. In this case it doesn't matter, but if at any point in the union, there are 'non array' bytes, data will not be aligned as wanted. This is vital when setting up things like structures into a union. If (for instance), you declared a structure of two 8 bit integers, for the LSB/MSB, these would not work, unless packed. So for anything where stuff is being declared to align data for something else, it is worth defaulting to packed.
Then remember that in PCD, the default integers are int16, and signed.
At one point I actually tried using "#typdef unsigned" to avoid this, but this causes problems with some of the CCS code, so doesn't solve the problem.
Instead load the 'stdint.h' file, and the code then becomes:
Code: |
#include <24HJ128GP306.h>
#fuses HS,NOWDT,PR
#use delay(crystal=20M)
#use rs232(baud=9600, xmit=PIN_F5,rcv=PIN_F4)
#include <stdint.h>
union __attribute__((packed)) input
{
uint8_t bits_8[4];
uint16_t bits_16[2];
uint32_t bits_32;
}value;
void main()
{
//! value.bits_32=0;
//! value.bits_16[0]=0;
//! value.bits_16[1]=0;
//! value.bits_8[0]=0;
//! value.bits_8[1]=0;
//! value.bits_8[2]=0;
//! value.bits_8[3]=0;
value.bits_16[0]=232;
value.bits_16[1]=11;
printf("H=%lu ",value.bits_32);
printf("H=%lu ",value.bits_16[1]);
printf("L=%lu ",value.bits_16[0]);
printf("0=%u ",value.bits_8[0]);
printf("1=%u ",value.bits_8[1]);
printf("2=%u ",value.bits_8[2]);
printf("3=%u ",value.bits_8[3]);
printf("\r\n");
while(TRUE)
{
}
}
|
This functions correctly on all the PIC processors (and in fact works on things like the 80486 as well - not using CCS).
Because the values are now all stored as unsigned integers, when 0xE8, is converted to int16, it becomes 0x00E8, and will then print correctly. |
|
|
jeremiah
Joined: 20 Jul 2010 Posts: 1348
|
|
Posted: Fri May 13, 2016 5:25 am |
|
|
Ttelmah wrote: | If (for instance), you declared a structure of two 8 bit integers, for the LSB/MSB, these would not work, unless packed. So for anything where stuff is being declared to align data for something else, it is worth defaulting to packed.
|
Sorry for the slight sidetrack here, but wanted to ask if you could show an example here. I was trying to understand the technical need for packed in a struct with two 8 bit integers. I know the general desire for packed but from a technical standpoint, unless I am misunderstanding what you are referencing, a structure with 2 8 bit ints will align correctly without packed.
Here is what I am talking about:
Test Code
Code: |
#case
#include <24fj64ga004.h>
typedef struct{
unsigned int8 b0;
unsigned int8 b1;
} test_struct_t;
typedef union{
unsigned int16 value;
struct{
unsigned int8 b0;
unsigned int8 b1;
};
} test_union_t;
void main()
{
test_struct_t s1;
test_union_t u1;
s1.b0 = 0x03;
s1.b1 = 0x85;
u1.value = 0x1234;
u1.b0 = 0x67;
u1.b1 = 0x01;
while(TRUE);
}
|
SYM excerpt
Code: |
630.6 C1OUT
630.7 C2OUT
802-803 main.s1
804-805 main.u1
2780-27FF Stack
|
This shows both variables with sizes of 2 bytes, which is what I would expect.
LST excerpt
Code: |
.................... test_struct_t s1;
.................... test_union_t u1;
....................
.................... s1.b0 = 0x03;
020C: MOV.B #3,W0L
020E: MOV.B W0L,802
.................... s1.b1 = 0x85;
0210: MOV.B #85,W0L
0212: MOV.B W0L,803
....................
.................... u1.value = 0x1234;
0214: MOV #1234,W4
0216: MOV W4,804
.................... u1.b0 = 0x67;
0218: MOV.B #67,W0L
021A: MOV.B W0L,804
.................... u1.b1 = 0x01;
021C: MOV.B #1,W0L
021E: MOV.B W0L,805
|
This shows that both b0 and b1 correctly call byte aligned instructions at the correct addresses.
Is this the same scenario you are discussing? If not, could you lay out a small example? Sorry about me being so dense about this. I am probably just reading something incorrectly.
EDIT: I am not saying you shouldn't use packed. I'm just trying to understand the mechanics correctly here. |
|
|
Ttelmah
Joined: 11 Mar 2010 Posts: 19510
|
|
Posted: Fri May 13, 2016 9:31 am |
|
|
May well depend on compiler.....
I've had it in the past _not_ align the structures used in the CCS default USB code correctly, unless I declared them to be packed. Suspect CCS may have fixed it.
Having been 'once bitten', I default to using packed (no reason not to, and it _ensures_ that the alignment does not get upset by the compiler...). |
|
|
jeremiah
Joined: 20 Jul 2010 Posts: 1348
|
|
Posted: Fri May 13, 2016 11:20 am |
|
|
Ok, makes sense. I've never run into it before, but now I know what to look for. I originally avoided ((packed)) because I wanted to make sure I didn't screw up and access misaligned data (16bit value on an odd address) and the default behavior of the compiler was shrinking them down to 8bits anyways.
I'm surprised CCS didn't make use of more __attribute__ directives. They tend to go with precompiler directives on a lot of stuff other compilers use the __attribute__ on. |
|
|
Ttelmah
Joined: 11 Mar 2010 Posts: 19510
|
|
Posted: Fri May 13, 2016 11:58 am |
|
|
What you have to be careful about is any pointers to 16bit or 32bit values. 8bit values are always byte accessed. |
|
|
|
|
You cannot post new topics in this forum You cannot reply to topics in this forum You cannot edit your posts in this forum You cannot delete your posts in this forum You cannot vote in polls in this forum
|
Powered by phpBB © 2001, 2005 phpBB Group
|