Again, "Magic Numbers" are best avoided - it would make your code clearer to give names to your LED patterns; eg,
#define LED01_PATTERN 0b1000000010000000
#define LED02_PATTERN 0b1000000001000000
#define LED03_PATTERN 0b1000000000100000
#define LED04_PATTERN 0b0100000010000000
:
:
#define LED24_PATTERN 0b0000000100100000
Combine that with earlier advice to pass the pattern as parameter - rather than via a global variable - to get:
if(input1==0)
{
HC4094write( LED01_PATTERN );
}
if(input2==0)
{
HC4094write( LED02_PATTERN );
}
if(input3==0)
{
HC4094write( LED03_PATTERN );
}
if(input4==0)
{
HC4094write( LED04_PATTERN );
}
Which is a lot clearer and more concise.
But this still means that, as soon as you have written the pattern for LED1, you immediately over-write it with the pattern for LED2.
I think a better approach would be something like:
#define LED01 (1UL<<0) // Set bit 0 for the 1st LED
#define LED02 (1UL<<1) // Set bit 1 for the 2nd LED
#define LED03 (1UL<<2) // Set bit 2 for the 3rd LED
#define LED04 (1UL<<3) // Set bit 3 for the 4th LED
:
#define LED24 (1UL<<23) // Set bit 4 for the 24th LED
uint32_t required_leds = 0; // This will be a bitmap of the LEDs required to be ON
// Now set the bits for each LED that is required to be ON
if(input1==0)
{
required_leds |= LED01;
}
if(input2==0)
{
required_leds |= LED02;
}
if(input3==0)
{
required_leds |= LED03;
}
if(input4==0)
{
required_leds |= LED04;
}
// required_leds now has one bit set for each LED required to be ON
// The HC4094 function turns this into the required pattern(s) to send
// to the shift registers
HC4094( required_leds );
As @unsigned_char_array pointed out earlier, your display consists of 8 groups of 3 LEDs.
So have the HC4094() function inspect the bitmap of which LEDs are required to be on, and then write to each group in turn.
That way you can write to 8 LEDs at once - instead of just one at a time.
Then you don't need to scan so fast - so your LEDs can be brighter.
