tags:

views:

170

answers:

4

First off, I’m not concerned with portability, and can safely assume that the endianness will not change. Assuming I read a hardware register value, I would like to overlay that register value over bitfields so that I can refer to the individual fields in the register without using bit masks.

EDIT: Fixed problems pointed out by GMan, and adjusted the code so it's clearer for future readers.

SEE: Anders K. & Michael J's answers below for a more eloquent solution.

#include <iostream>

/// \class HardwareRegister
/// Abstracts out bitfields in a hardware register.
/// \warning  This is non-portable code.
class HardwareRegister
{
   public:
      /// Constructor.
      /// \param[in]  registerValue - the value of the entire register. The
      ///                             value will be overlayed onto the bitfields
      ///                             defined in this class.
      HardwareRegister(unsigned long registerValue = 0)
      {
         /// Lots of casting to get registerValue to overlay on top of the
         /// bitfields
         *this = *(reinterpret_cast<HardwareRegister*>(&registerValue));
      }


      /// Bitfields of this register.
      /// The data type of this field should be the same size as the register
      /// unsigned short for 16 bit register
      /// unsigned long for 32 bit register.
      ///
      /// \warning Remember endianess! Order of the following bitfields are
      ///          important.
      ///          Big Endian    - Start with the most signifcant bits first.
      ///          Little Endian - Start with the least signifcant bits first.
      unsigned long field1: 8;
      unsigned long field2:16;
      unsigned long field3: 8;
}; //end class Hardware


int main()
{

   unsigned long registerValue = 0xFFFFFF00;
   HardwareRegister  testRegister(registerValue);

   // Prints out for little endianess machine
   // Field 1 = 0
   // Field 2 = 65535
   // Field 3 = 255
   std::cout << "Field 1 = " << testRegister.field1 << std::endl;
   std::cout << "Field 2 = " << testRegister.field2 << std::endl;
   std::cout << "Field 3 = " << testRegister.field3 << std::endl;
}
A: 
HW_Register(unsigned char registerValue) : field1(0), field2(0), field3(0) 
Chubsdad
@Potatoswatter: I goofed it up with the hex codes. Have cleaned it as your response is the best and correct too!.
Chubsdad
+4  A: 

Bitfields don't work that way. You can't assign a scalar value to a struct full of bitfields. It looks like you already know this since you used reinterpret_cast, but since reinterpret_cast isn't guaranteed to do very much, it's just rolling the dice.

You need to encode and decode the values if you want to translate between bitfield structs and scalars.

    HW_Register(unsigned char value)
      : field1( value & 3 ),
        field2( value >> 2 & 3 ),
        field3( value >> 4 & 7 )
        {}

Edit: The reason you don't get any output is that the ASCII characters corresponding to the numbers in the fields are non-printing. Try this:

std::cout << "Field 1 = " << (int) testRegister.field1 << std::endl;
std::cout << "Field 2 = " << (int) testRegister.field2 << std::endl;
std::cout << "Field 3 = " << (int) testRegister.field3 << std::endl;
Potatoswatter
If you're going to go through a slow (but safe) SHIFT-and-AND conversion, then you probably wouldn't bother with the bitfields. Seems implicit to me that the question was about reinterpreting the data in-place, for which Anders union is on the right track, but has implementation flaws.
Tony
@Tony: I wouldn't call 2-4 bitwise operations slow. Anyway, it has to be decoded at some point, so a better policy is to avoid bitfields inside C++ objects and just use `int`.
Potatoswatter
@Potatoswatter: slow's very context sensitive - the question does talks of hardware registers, and wanted to use reinterpret_cast - both suggestive of high performance sensitivity. Decoding up-front versus only - but repeatedly - when/if each field is used is a design trade-off, neither is inherently "better".
Tony
+3  A: 

don't do this

 *this = *(reinterpret_cast<HW_Register*>(&registerValue));

the 'this' pointer shouldn't be fiddled with in that way:

HW_Register reg(val)
HW_Register *reg = new HW_Register(val)

here 'this' is in two different places in memory

instead have an internal union/struct to hold the value, that way its easy to convert back and forth (since you are not interested in portability)

e.g.

union
{
   struct {
     unsigned short field1:2;
     unsigned short field2:4;
     unsigned short field3:2;
    ...
   } bits;
   unsigned short value;
} reg

edit: true enough with the name 'register'

Anders K.
There's nothing wrong with assigning to `this`. The `reinterpret_cast` is the entire problem. And `union` isn't a particularly safe way to handle this either. Such code is non-compliant.
Potatoswatter
This is the time-honoured approach for prioritising speed over safety or portability. You want "value" to be the same size as the register... not especially likely to be an unsigned short. For the bitfields, no need to say "short" - they're sized by the number of bits anyway, and you can't call it register - that's a reserved word.
Tony
@Potatoswatter yes it is wrong assigning to 'this' because its not a lvalue.
Anders K.
I like the union approach. Thanks.
Dennis Miller
@Anders: `*this` is, obviously that is what I'm referring to.
Potatoswatter
+1  A: 

Try this:

class HW_Register
{
public:
    HW_Register(unsigned char nRegisterValue=0)
    {
        Init(nRegisterValue);
    }
    ~HW_Register(void){};

    void Init(unsigned char nRegisterValue)
    {
        nVal = nRegisterValue;
    }

    unsigned Field1() { return nField1; }
    unsigned Field2() { return nField2; }
    unsigned Field3() { return nField3; }

private:
    union
    {
        struct 
        {
            unsigned char nField1:2;
            unsigned char nField2:4;
            unsigned char nField3:2;
        };
        unsigned char nVal;
    };
};


int main()
{
    unsigned char registerValue = 0xFF;
    HW_Register  testRegister(registerValue);

    std::cout << "Field 1 = " << testRegister.Field1() << std::endl;
    std::cout << "Field 2 = " << testRegister.Field2() << std::endl;
    std::cout << "Field 3 = " << testRegister.Field3() << std::endl;

    return 0;
}
Michael J
I like the code, looks very clean.
Dennis Miller
@Dennis -- Thanks. I try. :-)
Michael J