AVR (ATmega328P) - Reading input from PORTB to display hex digits on 7-Seg display

Go To Last Post
44 posts / 0 new
Author
Message
#1
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I'm trying to write a C program that will read the binary input on PORTB of the AVR and display the corresponding Hex digit on a single 7-segment display.

A bank of 4 DIP switches is connected to digital pins 8-11 (PB0->PB3).  I have 5v from the arduino connected to switches 5-8.  Switches 1-4 are connected to gnd on the arduino.

Here is a picture of the circuit:

 

circuit

 

The 7-seg display is correctly connected as I've tested it, but I'm not sure whether I've made a mistake in terms of the DIP switch and power/gnd connections.  Or, if my code is wrong.

Here is the code:

 

 

 

Can you please help to point me in the right direction, thanks.

 

Last Edited: Sun. Oct 8, 2017 - 04:26 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

How about a diagram of the extra wiring?

John Samperi

Ampertronics Pty. Ltd.

www.ampertronics.com.au

* Electronic Design * Custom Products * Contract Assembly

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Make sure you use common cathode display which common connected to ground.
And your switch bank common connected to vcc and each pin of PORTB have pull down resistor. Or activate the PORTB internal pull up and connect switch bank common to ground.
.
MG

I don't know why I'm still doing this hobby

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

js wrote:

How about a diagram of the extra wiring?

 

What do you mean js?

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

MicroGyro wrote:
Make sure you use common cathode display which common connected to ground. And your switch bank common connected to vcc and each pin of PORTB have pull down resistor. Or activate the PORTB internal pull up and connect switch bank common to ground. . MG

 

Thanks MicroGyro, the 7-seg is common cathode and is connected to gnd on arduino - it's working fine.  The switch bank common connected to vcc/ground, I don't understand.. Any further guidance would be appreciated.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

stevesy wrote:

js wrote:

How about a diagram of the extra wiring?

 

What do you mean js?

 

John was probably still asleep when he wrote that. He probably meant to ask for a circuit diagram. I would also, especially as I don't see any current limiting/setting resistors for the 7 segment display. Your photograph, while illustrative of your physical layout, does not define the interconnections the way a circuit diagram would do.

Ross McKenzie ValuSoft Melbourne Australia

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

valusoft wrote:

stevesy wrote:

js wrote:

How about a diagram of the extra wiring?

 

What do you mean js?

 

John was probably still asleep when he wrote that. He probably meant to ask for a circuit diagram. I would also, especially as I don't see any current limiting/setting resistors for the 7 segment display. Your photograph, while illustrative of your physical layout, does not define the interconnections the way a circuit diagram would do.

 

I'm doing this for a college assignment.  We aren't using resistors for the 7-segment display (not sure why).  If possible, can you or someone else explain to me in further detail where I'm going wrong with my wiring/code? 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

you need to enable pullups on your switch inputs - DDR = 0, PORT =1. If you are unsure of what I'm suggesting, refer to the datasheet. Because the AVR only has internal pullups ( as opposed to pull downs), you need to wire the switches between 0V and the port pin.

Because the AVR is implemented in CMOS, the inputs will 'float' if they are not pulled up or down.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

stevesy wrote:
Thanks MicroGyro, the 7-seg is common cathode and is connected to gnd on arduino - it's working fine.  The switch bank common connected to vcc/ground, I don't understand.. Any further guidance would be appreciated.

 

Forgive my big picture, no rezise option here.

 

You MUST use series resistor for your segment display. Unless you do with scanning.(I won't explain that further). Or you will fry the display.(maybe it already fried?)

 

Look at the schematic for the SW bank (DSW1). White represent the sw tips (position). That position represent 0b00000110 or 0x06 and your display should show number 6 (SIX).

But you must add to your code (under DDRB setup):

PORTB= 0xFF;

 

Hope it helps

 

MG

I don't know why I'm still doing this hobby

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Kartman wrote:

you need to enable pullups on your switch inputs - DDR = 0, PORT =1. If you are unsure of what I'm suggesting, refer to the datasheet. Because the AVR only has internal pullups ( as opposed to pull downs), you need to wire the switches between 0V and the port pin.

Because the AVR is implemented in CMOS, the inputs will 'float' if they are not pulled up or down.

 

Thanks Kartman I'll check out the datasheet, I'm unsure of pullups/pulldowns and will need to look into it.  May I just ask, are you saying I need to wire switches 5-8 to ground rather than 5v and enable the internal pullups on the AVR?

Last Edited: Sun. Oct 8, 2017 - 03:48 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

stevesy wrote:
My thinking was that when turned on and off they'd provide high/low input to PORTB
Unless there are pullups, when the switch is open the input to the uC will "float" and may be interpreted as not being "high". The internal pullups would provide the "drive" to insure the open switch is interpreted as a high.

David (aka frog_jr)

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

frog_jr wrote:

stevesy wrote:
My thinking was that when turned on and off they'd provide high/low input to PORTB
Unless there are pullups, when the switch is open the input to the uC will "float" and may be interpreted as not being "high". The internal pullups would provide the "drive" to insure the open switch is interpreted as a high.

 

Thanks frog_jr I think this is beginning to make a little more sense.  If the AVR happened to have internal pulldowns, this would ensure that the open switch was interpreted as low?

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

MicroGyro wrote:

stevesy wrote:
Thanks MicroGyro, the 7-seg is common cathode and is connected to gnd on arduino - it's working fine.  The switch bank common connected to vcc/ground, I don't understand.. Any further guidance would be appreciated.

 

Forgive my big picture, no rezise option here.

 

You MUST use series resistor for your segment display. Unless you do with scanning.(I won't explain that further). Or you will fry the display.(maybe it already fried?)

 

Look at the schematic for the SW bank (DSW1). White represent the sw tips (position). That position represent 0b00000110 or 0x06 and your display should show number 6 (SIX).

But you must add to your code (under DDRB setup):

PORTB= 0xFF;

 

Hope it helps

 

MG

 

Many thanks MicroGyro, it all helps! :)

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

stevesy wrote:
If the AVR happened to have internal pulldowns, this would ensure that the open switch was interpreted as low?
Yes. Although the AVRs do not have pulldowns, some other uCs do provide pullup/pulldown resistor configuration.

David (aka frog_jr)

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

frog_jr wrote:

Yes. Although the AVRs do not have pulldowns, some other uCs do provide pullup/pulldown resistor configuration.

 

Thank you frog_jr.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

stevesy wrote:

I'm doing this for a college assignment.  We aren't using resistors for the 7-segment display (not sure why). 

 

So tell me how you knew which pins to connect to which components if you don't have a circuit diagram? Seems implausible.

 

If your prof didn't include the current limiting resistors in the display section maybe s/he is expecting to read comments in your report along the lines of ...

 

"Hey prof, why does the power rail drop down to around 2 volts every time I send a number to my display? And why does my mcu chip get hot?"

Ross McKenzie ValuSoft Melbourne Australia

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

valusoft wrote:

stevesy wrote:

I'm doing this for a college assignment.  We aren't using resistors for the 7-segment display (not sure why). 

 

So tell me how you knew which pins to connect to which components if you don't have a circuit diagram? Seems implausible.

 

If your prof didn't include the current limiting resistors in the display section maybe s/he is expecting to read comments in your report along the lines of ...

 

"Hey prof, why does the power rail drop down to around 2 volts every time I send a number to my display? And why does my mcu chip get hot?"

 

That is true valusoft, and I have noticed the mcu chip get quite hot before.  Though I hadn't tested the power rail.. 

And apologies, I do have a circuit diagram for the the AVR chip and 7-seg display:

 

 

The is also a diagram in my lecture notes (about PINB) related to a switches and pullups:

 

                                                                          

 

Last Edited: Sun. Oct 8, 2017 - 10:09 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I'm still having difficulty getting this circuit and program to work.  Here is the circuit as it is now:

 

circuit

 

 

I have 0v connected from arduino to the 4 switches (5-8), and these are connected to digital pins 8-11 (PB0-PB3).

I've enabled the AVR's internal pullup resistors. 

 

DSW 8 (Connected to PB0) is ON

DSW 7 (Connected to PB0) is OFF

DSW 6 (Connected to PB0) is OFF

DSW 5 (Connected to PB0) is ON

 

So I was hoping to see a "6" on the display, similar to MicroGyro's example (I'm only using four of the eight switches).

 

Here are the alterations I've made in the code:

 

code

 

Should I be setting PullUps to 0x0F, as I'm only using four switches? Or should I be using "masking"?

e.g. x = PINB;

       x = x & 0x0F;

 

I've tried the masking approach above, and am starting to get some numbers but they're all over the place..

 

If you could help me to get this issue sorted I'd be greatful.  Thanks for all your help so far.

 

Last Edited: Sun. Oct 8, 2017 - 12:21 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

You may want to provide your updated code.

Have you added the code to enable the pullups as shown by MicroGyro?

MicroGyro wrote:
But you must add to your code (under DDRB setup): PORTB= 0xFF;

 

Edit: wrong quote assignment

stevesy wrote:
Should I be using "masking"?
Yes

David (aka frog_jr)

Last Edited: Sun. Oct 8, 2017 - 12:23 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

frog_jr wrote:

You may want to provide your updated code.

Have you added the code to enable the pullups as shown by stevesy?

stevesy wrote:
But you must add to your code (under DDRB setup): PORTB= 0xFF;

 

Hi frog_jr, I was having a problem uploading images (413 Request Entity too large)..

Updated code is up now.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Yes

 

Ok great frog_jr.  It seems to be the only thing giving me numbers on the display.  Here is the code as it is right now:

 

 

 

I'm a bit stumped..

 

 

 

Last Edited: Sun. Oct 8, 2017 - 12:59 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

stevesy wrote:
Should I be setting PullUps to 0x0F
 

The upper bits of PORTB do not need to be inputs if you are not using them; however, if they are inputs and you are not using them, placing the pullups on them is a good idea to keep them from floating.

Inputs that are driven by active logic (probably) should not have pullups active.

 

David (aka frog_jr)

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

frog_jr wrote:

Inputs that are driven by active logic (probably) should not have pullups active.

 

... but as has been said several times already, he has manually activated switches instead of active logic devices. So without pullups, his inputs will be swinging in the breeze.

 

And he still hasn't inserted the resistors in the display segments.

 

I'm out.

 

Ross McKenzie ValuSoft Melbourne Australia

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

valusoft wrote:

 

And he still hasn't inserted the resistors in the display segments.

 

I'm out.

 

 

My tutor didn't say to use resistors, which is why I haven't included them.  I'll include them now and post a pic.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

@valusoft, I was referring to the upper bits of the port that are not connected to the switches at all.

David (aka frog_jr)

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I have added a 470Ω current limiting resistor to display segment a, b, c, d, e, f, g.

 

current limiting resistors

 

 

 

 

 

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

First test your display to ensure it is not fried.
Apparently pins 3 and 8 are the common cathode and should be grounded.
One at a time, connect the anodes to Vcc through a resistor.
Next test your CPU to make sure it is not fried.
Run code you know works.

 

SOP for unused pins is to enable pullups.
Making them outputs can also work, but is more vulnrable to shorts/

 

As noted by others, for common cathode,
the straightforward way to limit current
is to use a resistor for each anode.
Other ways have drawbacks.

 

If one is feeling cheap, a resistor just for the common cathode will work,
but the limit will be per display, not per segment.
Since only one resistor is out of the package,
this seems to be what you are doing.
Current-hogging is likely.

 

If you can, reducing your CPU voltage to 3.3 might help.  No promises.
Similarly, adding a diode or four between
ground and the common cathode might work.
Again, no promises.

 

Test to ensure your connections are how you want them.
Write code that turns on one segment
at a time for five seconds in order.

 

Check your switches.
Copy switch values directly to the display.
If you include the decimal point,
you can copy each switch value and its complement to the display.
Floating pins might be visible as two lit segments.

 

Rewrite your previous code to use an array instead of 16 if's.
A #define for each segment might be useful.

 

International Theophysical Year seems to have been forgotten..
Anyone remember the song Jukebox Band?

Last Edited: Sun. Oct 8, 2017 - 04:05 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

skeeve wrote:

First test your display to ensure it is not fried.
Apparently pins 3 and 8 are the common cathode and should be grounded.
One at a time, connect the anodes to Vcc through a resistor.
Next test your CPU to make sure it is not fried.
Run code you know works.

 

SOP for unused pins is to enable pullups.
Making them outputs can also work, but is more vulnrable to shorts/

 

As noted by others, for common cathode,
the straightforward way to limit current
is to use a resistor for each anode.
Other ways have drawbacks.

 

If one is feeling cheap, a resistor just for the common cathode will work,
but the limit will be per display, not per segment.
Since only one resistor is out of the package,
this seems to be what you are doing.
Current-hogging is likely.

 

If you can, reducing your CPU voltage to 3.3 might help.  No promises.
Similarly, adding a diode or four between
ground and the common cathode might work.
Again, no promises.

 

Test to ensure your connections are how you want them.
Write code that turns on one segment
at a time for five seconds in order.

 

Check your switches.
Copy switch values directly to the display.
If you include the decimal point,
you can copy each switch value and its complement to the display.
Floating pins might be visible as two lit segments.

 

Rewrite your previous code to use an array instead of 16 if's.
A #define for each segment might be useful.

 

 

circuit

 

 

Many thanks for your advice skeeve, much appreciated. Once the current limiting resistors were added I got most of the hex digits to display, except for 6, 5 and 4.  But then as I continued testing, digits that had been displaying correctly began to display incorrectly!  I ran a program to test each segment individually and they all work.  I checked for loose connections etc..  This was for an assignment so I submitted the code I had - hopefully it was just an issue with the display/hardware.  Can you see anything "wrong" in my code?

Last Edited: Sun. Oct 8, 2017 - 09:45 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

stevesy wrote:
Many thanks for your advice skeeve, much appreciated. Once the current limiting resistors were added I got most of the hex digits to display, except for 6, 5 and 4. But then as I continued testing, digits that had been displaying correctly began to display incorrectly! I ran a program to test each segment individually and they all work. I checked for loose connections etc.. This was for an assignment so I submitted the code I had - hopefully it was just an issue with the display/hardware. Can you see anything "wrong" in my code?
My best guess is current-hogging.

You seem to be using only one resistor.

One way to check this is to use scan mode.

It turns on one segment at a time in quick succession.

International Theophysical Year seems to have been forgotten..
Anyone remember the song Jukebox Band?

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

skeeve wrote:

My best guess is current-hogging.

You seem to be using only one resistor.

One way to check this is to use scan mode.

It turns on one segment at a time in quick succession.

 

I'm using seven 470ohm resistors, one for each segment.  I'll look into scan mode skeeve, thank you.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

He probably meant to ask for a circuit diagram

Isn't that what I said? Not of the Arduino but all the "extra wiring" on the bread board.

John Samperi

Ampertronics Pty. Ltd.

www.ampertronics.com.au

* Electronic Design * Custom Products * Contract Assembly

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

stevesy wrote:
I got most of the hex digits to display, except for 6, 5 and 4.  But then as I continued testing, digits that had been displaying correctly began to display incorrectly!

What happened to 6,5,4?
.
Try this:
Use PORTC as input switch. IIRC PORTC only bit 0-5? Bit 6 is reset? So for 7 segment it short 1 bit.
Then use PORTB as output to display. Don't forget the series resistor!
This check to make sure that your PORTD not defected.
.
If the problem still exist then you had software issue?
.
MG

I don't know why I'm still doing this hobby

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

MG

Quote:

What happened to 6,5,4? . Try this: Use PORTC as input switch. IIRC PORTC only bit 0-5? Bit 6 is reset? So for 7 segment it short 1 bit. Then use PORTB as output to display. Don't forget the series resistor! This check to make sure that your PORTD not defected. . If the problem still exist then you had software issue? . MG

 

Incorrect segment(s) illuminated..  The code for displaying each digit is correct though.  As I mentioned, most of the digits displayed correctly but as I continued testing the errors began to happen on digits which had previously been working!  I did not change the code or anything.  Maybe it was a hardware issue with the display/switch?  I don't know.  I was doing this for a college assignment and sumbitted the code I had, as my time was limited.. Thanks for your advice as regards the PORTD deficiency test, and for your help with this project MicroGyro I really appreciate it :)

 

stevesy

Last Edited: Mon. Oct 9, 2017 - 06:34 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

If you really have 7 resistors, current-hogging should not be an issue.

Which segments lit for 4, 5 and 6?

Have you eliminated switch issues?

Have you written code to step though 0..0xF without the switches?

 

Are you able to change the processor board?

 

If your switches are flaky enough to give you intermittent connections even in "steady" state,

the result would be some extra segments dimly lit.

International Theophysical Year seems to have been forgotten..
Anyone remember the song Jukebox Band?

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Sort of lends itself to a solution such as:

uint8_t digits[] = { ZERO, ONE, TWO, THREE, FOUR, FIVE, SIX, SEVEN, EIGHT, NINE, A, B, C, D, E, F };

x = PINB;
x = x & 0x0F;

PORTD = digits[x];

(which replaces that LONG sequence!).

 

Can't say I am that keen on the redefinition of A, B, C, D, E and F either - couldn't you call them something like DISPLAY_A, DISPLAY_B or something ?

Last Edited: Mon. Oct 9, 2017 - 09:00 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

clawson wrote:
Sort of lends itself to a solution such as: uint8_t digits[] = { ZERO, ONE, TWO, THREE, FOUR, FIVE, SIX, SEVEN, EIGHT, NINE, A, B, C, D, E, F }; x = PINB; x = x & 0x0F; PORTD = digits[x]; (which replaces that LONG sequence!). Can't say I am that keen on the redefinition of A, B, C, D, E and F either - couldn't you call them something like DISPLAY_A, DISPLAY_B or something ?
OP didn't seem to notice when I suggested an array.

I'm not keen on OP's choice of things to #define.

I'd have gone with display segments.

The combinations of segments should be stable across hardware changes.

The segment #defines represent design decisions and should probably be in a separate header file,

possibly with other design decisions.

 

Also, I'd use a static_assert to check the array size.

 

OP's code does not have a main loop.

International Theophysical Year seems to have been forgotten..
Anyone remember the song Jukebox Band?

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

skeeve wrote:

clawson wrote:
Sort of lends itself to a solution such as: uint8_t digits[] = { ZERO, ONE, TWO, THREE, FOUR, FIVE, SIX, SEVEN, EIGHT, NINE, A, B, C, D, E, F }; x = PINB; x = x & 0x0F; PORTD = digits[x]; (which replaces that LONG sequence!). Can't say I am that keen on the redefinition of A, B, C, D, E and F either - couldn't you call them something like DISPLAY_A, DISPLAY_B or something ?
OP didn't seem to notice when I suggested an array.

I'm not keen on OP's choice of things to #define.

I'd have gone with display segments.

The combinations of segments should be stable across hardware changes.

The segment #defines represent design decisions and should probably be in a separate header file,

possibly with other design decisions.

 

Also, I'd use a static_assert to check the array size.

 

OP's code does not have a main loop.

 

I'm new to this stuff skeeve and clawson.  Thank you both very much for your help and advice, I'm not anywhere close to your level.. We're now moving on to LCDs..

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Nobody is missing while(1) ?

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Visovian wrote:

Nobody is missing while(1) ?

 

In post#36, Skeeve said...

OP's code does not have a main loop.

 

Ross McKenzie ValuSoft Melbourne Australia

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

stevesy wrote:
We're now moving on to LCDs..
You will likely need a main loop.

International Theophysical Year seems to have been forgotten..
Anyone remember the song Jukebox Band?

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I've attached my updated code, please let me know what you think.  And thank you to you all for your continued help, much appreciated.

 

 

Attachment(s): 

Last Edited: Tue. Oct 17, 2017 - 09:19 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Previously, uint_8 was suggested, but you used int. Whilst it does make much difference to your code, it is a difference that might have an impact in other code. The reason being that the AVR is 8bit and the ports are 8bit, so using an 8bit variable is the more economical choice. With other architectures like the arm, it may work the other way - using an int can be more economical on code space.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Kartman wrote:
uint
Kartman wrote:
Previously, uint_8 was suggested, but you used int. Whilst it does make much difference to your code, it is a difference that might have an impact in other code. The reason being that the AVR is 8bit and the ports are 8bit, so using an 8bit variable is the more economical choice. With other architectures like the arm, it may work the other way - using an int can be more economical on code space.

 

Thanks Kartman, I don't really understand the difference between the two at this time tbh.  Besides uint_8, does the code look ok to you?

 

Regards,

 

stevesy

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

stevesy wrote:
Besides uint_8, does the code look ok to you?
The segment defining array is never going to change. Whenever you see something in a C program that is never going to change you should apply the "const" modifier to it. For AVR the architecture means that flash and RAM are split into separate memories. C does not natively know this so it will put all variables you define into RAM. For something that never changes that is a waste of RAM as you don't need it in the (scarce!) modifiable memory. It would be enough for it to reside in flash. Recently the GCC compiler for AVR has had the ability to easily place data in flash added. Such data must be defined globally. Putting that all together gets you to:

const __flash uint8_t digits[] = {DISP_ZERO, DISP_TWO, DISP_THREE, DISP_FOUR, DISP_FIVE, DISP_SIX, DISP_SEVEN, DISP_EIGHT, DISP_NINE, DISP_A, DISP_b, DISP_C, DISP_d, DISP_E, DISP_F};

int main(void) {

	uint8_t x;

	DDRD = PDOUTPUTS; // set all PORTD pins to outputs
	DDRB = PBINPUTS;  // set all PORTB pins to inputs
	PORTB = PULLUPS;  // enable internal pull-ups on PORTB


	while(1)
	{
		//apply masking
		x = PINB;
		x = x & 0x0F; // read first 4 pins and mask out last 4 pins
		PORTD = digits[x]; // write corresponding value of x from array to PORTD
	}
}

BTW I took the opportunity to capitalize your macro names. Pretty much all C and C++ programmers everywhere stick to the convention of using all upper case for macro names and use mixed or lower case for function and symbol names.

 

(in fact you can tell from the names "PINB" and "PORTD" that those things must also be macros - not variables).

 

If you want to make "readable words" from your macro names use something like:

	DDRD = PD_OUTPUTS; // set all PORTD pins to outputs
	DDRB = PB_INPUTS;  // set all PORTB pins to inputs
	PORTB = PULL_UPS;  // enable internal pull-ups on PORTB