Click here to Skip to main content
15,885,952 members

The Weird and The Wonderful

   

The Weird and The Wonderful forum is a place to post Coding Horrors, Worst Practices, and the occasional flash of brilliance.

We all come across code that simply boggles the mind. Lazy kludges, embarrassing mistakes, horrid workarounds and developers just not quite getting it. And then somedays we come across - or write - the truly sublime.

Post your Best, your worst, and your most interesting. But please - no programming questions . This forum is purely for amusement and discussions on code snippets. All actual programming questions will be removed.

 
GeneralRe: This is actually cleaned up! Pin
honey the codewitch24-Jan-22 1:00
mvahoney the codewitch24-Jan-22 1:00 
GeneralRe: This is actually cleaned up! Pin
jschell30-Jan-22 6:13
jschell30-Jan-22 6:13 
GeneralRe: This is actually cleaned up! Pin
honey the codewitch30-Jan-22 9:02
mvahoney the codewitch30-Jan-22 9:02 
GeneralRe: This is actually cleaned up! Pin
jschell17-Feb-22 10:33
jschell17-Feb-22 10:33 
JokeRe: This is actually cleaned up! Pin
Nelek15-Dec-21 10:59
protectorNelek15-Dec-21 10:59 
GeneralRe: This is actually cleaned up! Pin
Fueled By Decaff15-Dec-21 6:30
Fueled By Decaff15-Dec-21 6:30 
GeneralRe: This is actually cleaned up! Pin
honey the codewitch15-Dec-21 6:32
mvahoney the codewitch15-Dec-21 6:32 
GeneralRe: This is actually cleaned up! Pin
Al_Brown31-Dec-21 1:44
Al_Brown31-Dec-21 1:44 
I realise you're not asking for suggestions but I would certainly be minded to simplify this, particularly if this is for general purpose use. If you're in a situation where this is for an embedded platform where the hardware configuration is fixed you should gain both better readability and performance if you can spare a little RAM. Something like this:

C++
const int NUM_BUS_BITS = 8;     // Dealing with an eight bit bus
const int NUM_GPIO_PINS = 40;   // ESP32 has GPIOs 0 to 39

// Last read state of the GPIO bus
static uint32_t sGPIOLow, sGPIOHigh;

// Map data bus to GPIO bits
static struct
{
    uint32_t* pSource;          // Which GPIO variable to use
    unsigned shift;             // Shift to apply for this bit
} sDataMap[NUM_BUS_BITS];

// Initialise mapping between input data bit and GPIO pin - call for each data bit prior to readInput()
static void mapInputPin(unsigned dataBit, unsigned gpioPin)
{
    if (dataBit < NUM_BUS_BITS && gpioPin < NUM_GPIO_PINS)
    {
        sDataMap[dataBit].pSource = gpioPin >= 32 ? &sGPIOHigh : &sGPIOLow;
        sDataMap[dataBit].shift = gpioPin & 31;
    }
}

// Read our eight bit bus value from the appropriate GPIO pins
static uint8_t readInput(void)
{
    // Read the hardware bus (the need for 3x reads should be investigated)
    sGPIOLow = gpio_input_get();
    sGPIOLow = gpio_input_get();
    sGPIOLow = gpio_input_get();
    sGPIOHigh = gpio_input_get_high();

    // Decode from GPIO to eight bit bus
    uint8_t dataRead = 0;
    dataRead |= (((*sDataMap[0].pSource) >> sDataMap[0].shift) & 1) << 0;
    dataRead |= (((*sDataMap[1].pSource) >> sDataMap[1].shift) & 1) << 1;
    dataRead |= (((*sDataMap[2].pSource) >> sDataMap[2].shift) & 1) << 2;
    dataRead |= (((*sDataMap[3].pSource) >> sDataMap[3].shift) & 1) << 3;
    dataRead |= (((*sDataMap[4].pSource) >> sDataMap[4].shift) & 1) << 4;
    dataRead |= (((*sDataMap[5].pSource) >> sDataMap[5].shift) & 1) << 5;
    dataRead |= (((*sDataMap[6].pSource) >> sDataMap[6].shift) & 1) << 6;
    dataRead |= (((*sDataMap[7].pSource) >> sDataMap[7].shift) & 1) << 7;

    return dataRead;
}

int main(void)
{
    // Initialise mapping for GPIO allocation
    // (Use defines or named constants for a real implementation)
    mapInputPin(0, 4);
    mapInputPin(1, 5);
    mapInputPin(2, 6);
    mapInputPin(3, 7);
    mapInputPin(4, 36);
    mapInputPin(5, 37);
    mapInputPin(6, 38);
    mapInputPin(7, 39);

    // Read input
    uint8_t readData = readInput();

    return readData;
}


The readInput() function is basically what you have above but doesn't require the conditionals since you've defined ahead of time where data should be read from and how it should be shifted. It reads both low and high GPIO registers but I doubt that will be noticeable but could be simplified prior to the decoding if absolutely necessary.

The multiple reads from the GPIO bus is a bit of a red flag, however. I am not familiar with the platform (I'm guessing ESP32) but that would usually suggest to me that a hardware clock hasn't been set up correctly or that your timing is marginal. If it's the latter then I'd suggest you need a more robust delay mechanism as you may be lucky on this board but it'll be intermittent on others.
GeneralRe: This is actually cleaned up! Pin
honey the codewitch31-Dec-21 3:14
mvahoney the codewitch31-Dec-21 3:14 
GeneralRe: This is actually cleaned up! Pin
Al_Brown31-Dec-21 4:38
Al_Brown31-Dec-21 4:38 
GeneralMessage Closed Pin
13-Dec-21 14:55
mvahoney the codewitch13-Dec-21 14:55 
GeneralRe: IME, It's a lot harder to shop for people when they get pregnant Pin
Brisingr Aerowing13-Dec-21 15:01
professionalBrisingr Aerowing13-Dec-21 15:01 
GeneralRe: IME, It's a lot harder to shop for people when they get pregnant Pin
honey the codewitch13-Dec-21 15:06
mvahoney the codewitch13-Dec-21 15:06 
GeneralI'm not sure how I feel about Forth implemented on a Cortex M-0 Pin
honey the codewitch13-Dec-21 0:09
mvahoney the codewitch13-Dec-21 0:09 
GeneralRe: I'm not sure how I feel about Forth implemented on a Cortex M-0 Pin
trønderen13-Dec-21 16:55
trønderen13-Dec-21 16:55 
GeneralRe: I'm not sure how I feel about Forth implemented on a Cortex M-0 Pin
Richard Deeming13-Dec-21 21:58
mveRichard Deeming13-Dec-21 21:58 
JokeRe: I'm not sure how I feel about Forth implemented on a Cortex M-0 Pin
englebart1-Sep-22 15:12
professionalenglebart1-Sep-22 15:12 
GeneralRe: I'm not sure how I feel about Forth implemented on a Cortex M-0 Pin
Rick York14-Dec-21 19:45
mveRick York14-Dec-21 19:45 
GeneralRe: I'm not sure how I feel about Forth implemented on a Cortex M-0 Pin
Gary Wheeler31-Dec-21 4:07
Gary Wheeler31-Dec-21 4:07 
GeneralRe: I'm not sure how I feel about Forth implemented on a Cortex M-0 Pin
Rick York31-Dec-21 7:44
mveRick York31-Dec-21 7:44 
GeneralPreparing for the future... PinPopular
Richard Deeming8-Dec-21 1:13
mveRichard Deeming8-Dec-21 1:13 
JokeRe: Preparing for the future... Pin
Daniel Pfeffer8-Dec-21 1:30
professionalDaniel Pfeffer8-Dec-21 1:30 
GeneralRe: Preparing for the future... Pin
PIEBALDconsult8-Dec-21 3:09
mvePIEBALDconsult8-Dec-21 3:09 
JokeRe: Preparing for the future... PinPopular
Sander Rossel8-Dec-21 3:42
professionalSander Rossel8-Dec-21 3:42 
GeneralRe: Preparing for the future... Pin
Greg Utas9-Dec-21 14:09
professionalGreg Utas9-Dec-21 14:09 

General General    News News    Suggestion Suggestion    Question Question    Bug Bug    Answer Answer    Joke Joke    Praise Praise    Rant Rant    Admin Admin   

Use Ctrl+Left/Right to switch messages, Ctrl+Up/Down to switch threads, Ctrl+Shift+Left/Right to switch pages.