logic operators & bit separation calculation in C (PIC programming)

Question

I am programming a PIC18F94K20 to work in conjunction with a MCP7941X I2C RTCC ship and a 24AA128 I2C CMOS Serial EEPROM device. Currently I have code which successfully intialises the seconds/days/etc values of the RTCC and starts the timer, toggling a LED upon the turnover of every second.

I am attempting to augment the code to read back the correct data for these values, however I am running into trouble when I try to account for the various 'extra' bits in the values. The memory map may help elucidate my problem somewhat:

Taking, for example, the hours column, or the 02h address. Bit 6 is set as 1 to toggle 12 hour time, adding 01000000 to the hours bit. I can read back the entire contents of the byte at this address, but I want to employ an if statement to detect whether 12 or 24 hour time is in place, and adjust accordingly. I'm not worried about the 10-hour bits, as I can calculate that easily enough with a BCD conversion loop (I think).

I earlier used the bitwise OR operator in C to augment the original hours data to 24. I initialised the hours in this particular case to 0x11, and set the 12 hour control bit which is 0x64. When setting the time:

WriteI2C(0x11|0x64);

which as you can see uses the bitwise OR.

When reading back the hours, how can I incorporate operators into my code to separate the superfluous bits from the actual time bits? I tried doing something like this:

current_seconds = ReadI2C();
current_seconds = ST & current_seconds;

but that completely ruins everything. It compiles, but the device gets 'stuck' on this sequence.

How do I separate the ST / AMPM / VBATEN bits from the actual data I need, and what would a good method be of implementing for loops for the various circumstances they present (e.g. reading back 12 hour time if bit 6 = 0 and 24 hour time if bit6 = 1, and so on).

I'm a bit of a C novice and this is my first foray into electronics so I really appreciate any help. Thanks.

Answer 1

To remove (zero) a bit, you can AND the value with a mask having all other bits set, i.e., the complement of the bits that you wish to zero, e.g.:

value_without_bit_6 = value & ~(1<<6);

To isolate a bit within an integer, you can AND the value with a mask having only those bits set. For checking flags this is all you need to do, e.g.,

if (value & (1<<6)) {
    // bit 6 is set
} else {
    // bit 6 is not set
}

To read the value of a small integer offset within a larger one, first isolate the bits, and then shift them right by the index of the lowest bit (to get the least significant bit into correct position), e.g.:

value_in_bits_4_and_5 = (value & ((1<<4)|(1<<5))) >> 4;

For more readable code, you should use constants or #defined macros to represent the various bit masks you need, e.g.:

#define BIT_VBAT_EN (1<<3)

if (value & BIT_VBAT_EN) {
   // VBAT is enabled
}

Another way to do this is to use bitfields to define the organisation of bits, e.g.:

typedef union {
    struct {
        unsigned ones:4;
        unsigned tens:3;
        unsigned st:1;
    } seconds;
    uint8_t byte;
} seconds_register_t;

seconds_register_t sr;
sr.byte = READ_ADDRESS(0x00);
unsigned int seconds = sr.seconds.ones + sr.seconds.tens * 10;

A potential problem with bitfields is that the code generated by the compiler may be unpredictably large or inefficient, which is sometimes a concern with microcontrollers, but obviously it's nicer to read and write. (Another problem often cited is that the organisation of bit fields, e.g., endianness, is largely unspecified by the C standard and thus not guaranteed portable across compilers and platforms. However, it is my opinion that low-level development for microcontrollers tends to be inherently non-portable, so if you find the right bit layout I wouldn't consider using bitfields “wrong”, especially for hobbyist projects.)

Yet you can accomplish similarly readable syntax with macros; it's just the macro itself that is less readable:

#define GET_SECONDS(r) ( ((r) & 0x0F) + (((r) & 0x70) >> 4) * 10 )
uint8_t sr = READ_ADDRESS(0x00);
unsigned int seconds = GET_SECONDS(sr);

Answer 2

Regarding the bit masking itself, you are going to want to make a model of that memory map in your microcontroller. The simplest, cudest way to do that is to #define a number of bit masks, like this:

#define REG1_ST          0x80u
#define REG1_10_SECONDS  0x70u
#define REG1_SECONDS     0x0Fu

#define REG2_10_MINUTES  0x70u
...

And then when reading each byte, mask out the data you are interested in. For example:

bool    st          = (data & REG1_ST) != 0;
uint8_t ten_seconds = (data & REG1_10_SECONDS) >> 4;
uint8_t seconds     = (data & REG1_SECONDS);

The important part is to minimize the amount of "magic numbers" in the source code.

Writing data:

reg1 = 0;
reg1 |= st ? REG1_ST : 0;
reg1 |= (ten_seconds << 4) & REG1_10_SECONDS;
reg1 |= seconds & REG1_SECONDS;

Please note that I left out the I2C communication of this.