HiveBrain v1.2.0
Get Started
← Back to all entries
patterncMinor

Toggle four relays on/off based on bitmap stored in uint8_t

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
storedfouruint8_tbasedbitmaprelaysofftoggle

Problem

I am using a one-byte bitmap to control four relays. I am writing a function which will take the bitmap stored as uint8_t, look to see which bits are set/cleared, then turn on/off the relays appropriately. The input value to my function could be anything from 0 to 15.

This is in a controls application, so I'd like this to thing to work pretty fast (not insane optimization, since I still have to wait 10ms for my relays to latch).

Here's the function so far (I stripped out the actual relay switching to simplify):

void 
relayToggle(uint8_t relayBitmap)
{
    if ( 0b00000001 & relayBitmap == 0b00000001 ) { relayOn(1); }
    else if ( 0b00000001 & relayBitmap == 0 ) { relayOff(1); }
    else if ( 0b00000010 & relayBitmap == 0b00000010 ) { relayOn(2); }
    else if ( 0b00000010 & relayBitmap == 0 ) { relayOff(2); }
    else if ( 0b00000100 & relayBitmap == 0b00000100 ) { relayOn(3); }
    else if ( 0b00000100 & relayBitmap == 0 ) { relayOff(3); }
    else if ( 0b00001000 & relayBitmap == 0b00001000 ) { relayOn(4); }
    else if ( 0b00001000 & relayBitmap == 0 ) { relayOff(4); }
}

Solution

Use a loop to eliminate all that copy-pasta and get rid of some superfluous tests, branches and hard-coded constants:

void relayToggle(uint8_t relayBitmap)
{
    for (int r = 1; r >= 1;
    }
}

Code Snippets

void relayToggle(uint8_t relayBitmap)
{
    for (int r = 1; r <= 4; ++r)
    {
         if (relayBitmap & 1)
             relayOn(r);
         else
             relayOff(r);
         relayBitmap >>= 1;
    }
}

Context

StackExchange Code Review Q#112279, answer score: 2

Revisions (0)

No revisions yet.