Click here to Skip to main content
15,900,418 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.

 
AnswerRe: Intergalactic cephalopods Pin
Eddy Vluggen7-Jan-22 13:26
professionalEddy Vluggen7-Jan-22 13:26 
GeneralRe: Intergalactic cephalopods Pin
Rob Grainger1-Feb-22 3:20
Rob Grainger1-Feb-22 3:20 
GeneralCuter by the day.... PinPopular
Peter_in_278028-Dec-21 17:05
professionalPeter_in_278028-Dec-21 17:05 
GeneralRe: Cuter by the day.... Pin
Super Lloyd7-Jan-22 12:08
Super Lloyd7-Jan-22 12:08 
GeneralRe: Cuter by the day.... Pin
Peter_in_27807-Jan-22 18:35
professionalPeter_in_27807-Jan-22 18:35 
GeneralRe: Cuter by the day.... Pin
Super Lloyd7-Jan-22 20:32
Super Lloyd7-Jan-22 20:32 
GeneralThis is actually cleaned up! Pin
honey the codewitch14-Dec-21 17:58
mvahoney the codewitch14-Dec-21 17:58 
GeneralRe: This is actually cleaned up! Pin
11917640 Member 14-Dec-21 21:49
11917640 Member 14-Dec-21 21:49 
GeneralRe: This is actually cleaned up! Pin
honey the codewitch15-Dec-21 1:43
mvahoney the codewitch15-Dec-21 1:43 
GeneralRe: This is actually cleaned up! Pin
Slacker00715-Dec-21 7:58
professionalSlacker00715-Dec-21 7:58 
GeneralRe: This is actually cleaned up! Pin
honey the codewitch15-Dec-21 9:17
mvahoney the codewitch15-Dec-21 9:17 
GeneralRe: This is actually cleaned up! Pin
Rick York15-Dec-21 6:30
mveRick York15-Dec-21 6:30 
GeneralRe: This is actually cleaned up! Pin
honey the codewitch15-Dec-21 6:38
mvahoney the codewitch15-Dec-21 6:38 
GeneralRe: This is actually cleaned up! Pin
Rick York15-Dec-21 6:55
mveRick York15-Dec-21 6:55 
GeneralRe: This is actually cleaned up! Pin
honey the codewitch15-Dec-21 6:58
mvahoney the codewitch15-Dec-21 6:58 
GeneralRe: This is actually cleaned up! Pin
jschell23-Jan-22 8:54
jschell23-Jan-22 8:54 
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 

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.