Skip to content
This repository has been archived by the owner on Jan 7, 2019. It is now read-only.

[driver] Explicitly cast enum class assignments #152

Merged
merged 1 commit into from
Jun 29, 2016
Merged

[driver] Explicitly cast enum class assignments #152

merged 1 commit into from
Jun 29, 2016

Conversation

salkinium
Copy link
Member

@salkinium salkinium commented Jun 29, 2016

Assigning an enum class value with another enum class value is not
type compatible, and must be explicitly cast before assignment.

This bug seems to have been ignored by GCC until the 6.1 release.
Clang (used for hosted unit tests) does not ignore this bug.

Thanks to @daniel-k for reporting this bug in #151.
cc @ekiwi @strongly-typed

@salkinium
Copy link
Member Author

Another UX fail is the inablity to use the overloaded flags at assignment time:

Given this initial register (from the LIS3DSH driver here):

/// CTRL_REG4 default value is 0x07
enum class
Control4 : uint8_t
{
    ODR3 = Bit7,
    ODR2 = Bit6,
    ODR1 = Bit5,
    ODR0 = Bit4,
    ODR_Mask = Bit7 | Bit6 | Bit5 | Bit4,

    BDU = Bit3,     ///< Block data update.
    ZEN = Bit2,     ///< Z axis enable
    YEN = Bit1,     ///< Y axis enable
    XEN = Bit0,     ///< X axis enable
};
XPCC_FLAGS8(Control4);

You must then cast each flag individually to int before ORing, instead of being able to use the operator overloading:

enum class
MeasurementRate : uint8_t
{
    Off = 0,
    Hz3_125 = int(Control4::ODR0),
    Hz6_25 = int(Control4::ODR1),
    Hz12_5 = int(Control4::ODR1) | int(Control4::ODR0),
    Hz25 = int(Control4::ODR2),
    Hz50 = int(Control4::ODR2) | int(Control4::ODR0),
    Hz100 = int(Control4::ODR2) | int(Control4::ODR1),
    Hz400 = int(Control4::ODR2) | int(Control4::ODR1) | int(Control4::ODR0),
//  Obviously it's not possible to OR first, then cast!1!!
//  Hz400 = int(Control4::ODR2 | Control4::ODR1 | Control4::ODR0),
    Hz800 = int(Control4::ODR3),
    Hz1600 = int(Control4::ODR3) | int(Control4::ODR0),
};

On the other hand, semantically speaking, the above example is pointless.
The user is never supposed to use the FifoControl::FMx bit manually, that's what the FifoMode is for.
Therefore using the xpcc::Bitx directly in FifoMode seems like a better idea.
That would translate to this:

/// CTRL_REG4 default value is 0x07
enum class
Control4 : uint8_t
{
//  No ODR bits!
    BDU = Bit3,     ///< Block data update.
    ZEN = Bit2,     ///< Z axis enable
    YEN = Bit1,     ///< Y axis enable
    XEN = Bit0,     ///< X axis enable
};
XPCC_FLAGS8(Control4);

enum class
MeasurementRate : uint8_t
{
    Off = 0,
    Hz3_125 = Bit4,
    Hz6_25 = Bit5,
    Hz12_5 = Bit5 | Bit4,
    Hz25 = Bit6,
    Hz50 = Bit6 | Bit4,
    Hz100 = Bit6 | Bit5,
    Hz400 = Bit6 | Bit5 | Bit4,
    Hz800 = Bit7,
    Hz1600 = Bit7 | Bit4,
};
// For completeness
using MeasurementRate_t = Configuration<Control4_t, MeasurementRate, 0b11110000>;

Assigning an enum class value with another enum class value is not
type compatible, and must be explicitly cast before assignment.

This bug seems to have been ignored by GCC until the 6.1 release.
Clang (used for hosted unit tests) does not ignore this bug.
@salkinium salkinium merged commit 82c60bc into roboterclubaachen:develop Jun 29, 2016
@salkinium salkinium deleted the fix/enum_class_casting branch June 29, 2016 22:38
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant