We are running a survey to help us improve the experience for all of our members. If you see the survey appear, please take the time to tell us about your experience if you can.
I have a somewhat complex structure in my C code, and driver functions to read or write data with it.
The simpler parts of the struct (such as ioflags2 below) and their driver functions work just fine. However, the more complex parts of the struct (such as ioflags3) do not.
The symptom is that only the drivers for the first part of the more complex parts of the structure work right, and the rest don't appear to change the "word" part of the structure.
However, if I step through the code with the simulator, all of the individual parts of the more complex parts of the struct work just fine with the driver functions, but the "word" part doesn't change (well, except for the 1st part of the more complex parts, i.e., module1 works, but module2 - module5 don't).
Finally, I cut/paste the whole struct and drivers to a Visual C++ program and stepped through the code with their debugger. *ALL* of the driver functions had the expected effect on *ALL* of the parts of the whole struct.
Can anyone tell me what, if anything, is wrong with this code, and how to make it work with KEIL's ARM tools?
union{ u16 word; struct{ unsigned sensor1:1; unsigned sensor2:1; unsigned sensor3:1; unsigned sensor4:1; unsigned sensor5:1; unsigned sensor6:1; unsigned sensor7:1; unsigned sensor8:1; unsigned sensor9:1; unsigned sensor10:1; unsigned reserved_bit_10:1; unsigned reserved_bit_11:1; unsigned reserved_bit_12:1; unsigned reserved_bit_13:1; unsigned reserved_bit_14:1; unsigned reserved_bit_15:1; }bit; }ioflags2; union{ u16 word; struct{ struct{ struct{ unsigned run:1; unsigned direction:1; }stepper_motor; }module1; union{ struct{ unsigned extend:1; unsigned retract:1; }ioctl; struct{ unsigned run:1; unsigned direction:1; }dc_motor; }module2; union{ struct{ unsigned open:1; unsigned close:1; }ioctl; struct{ unsigned run:1; unsigned direction:1; }dc_motor; }module3; union{ struct{ unsigned open:1; unsigned close:1; }ioctl; struct{ unsigned run:1; unsigned direction:1; }dc_motor; }module4; union{ struct{ unsigned reserved_bit_0:1; unsigned fan:1; }ioctl; struct{ unsigned direction:1; unsigned run:1; }dc_motor; }module5; unsigned reserved_bit_10:1; unsigned reserved_bit_11:1; unsigned reserved_bit_12:1; unsigned reserved_bit_13:1; unsigned reserved_bit_14:1; unsigned reserved_bit_15:1; }bit; }ioflags3;
And here's how my drivers manipulate the structure.
int set_module2_retract(int state){ int previous_state; ASSERT(module2_type() == TYPE_IOCTL); if(module2_type() != TYPE_IOCTL){ return(-1); } if(state){ set_module1_extend(0); // protect the hardware! } previous_state = variable.ioflags3.bit.module2.ioctl.retract; variable.ioflags3.bit.module2.ioctl.retract = state ? 1u : 0u; return(previous_state); }
And... getting back to the original question, could I have used the __packed attribute to make the original code work like I expected it to (i.e., to shrink the number of bytes required to implement the ioflags variables to the min size possible)?
I believe what Per was saying (although if you get lost in the discussion it has the distinct possibility of being lost). Do not rely on the compiler to deal with the hardware in a certain manner. This is VERY important especially if you change processors (which I have done numerous tmies) or compilers (same there).
What I garnered is that he was suggesting using the compiler for what is known to work with normal C/C++ conventions in ALL compilers. That is use the 'ugly' standard C '&' '|' operators so that it will work irregardless of the compilers arbitrary support methods for bit fields.
The assumption that the compiler you have available (IE Turbo C++ Borland C++ GCC {in it's numerous incarnations} ) will work the same irregardless of what target or version you are using is a bad one.
Summary make your code blatantly obvious what it is doing, and comment it heavily. So that no one can guess what is being modified. Do not rely on any feature of a compiler (again bit fields is the example) to make the code work. Readability also includes understanding what is being done and why, not just having nice looking code (that hides lots of potential problems, and sins, EX one small change is made such as recompiling the project with a different version of the compiler).
A good resource is http://www.ganssle.com/ and http://www.embedded.com for this, and numerous other problems you might encounter.
Stephen
No. If you pack the data each union will shrink to 1 byte and you will need a 6th byte for the 6 reserved bits you have last.
That means that the 2 bytes of the u16 will be overlaid with 6 bytes.
Your unions module1, module2, module3, module4 and module5 will not magically merge into a single 16-bit container.
Paragraph 6.7.2.1 of the C standard says in section 13 that "Within a structure object, the non-bit-field members and the units in which bit-fields reside have addresses that increase in the order in which they are declared."
In 14, the standard specifies that "A pointer to a union object, suitably converted, points to each of its members (or if a member is a bitfield, then to the unit in which it resides), and vice versa."
The language standard will not merge your bit fields since that would mean that the struct directly following the u16 would have multiple members having the same address. But only a union may have multiple members with the same address.