Incrementing a seven segment display in a state machine for de1 board

Question

I am using a state machine to create a countdown timer that on startup, displays 00:00 and when Key1 is pressed, you can input a time by incrementing/decrementing minutes by 1 and if the up/down button is held for 5 cycles it will go up/down by 5. Thanks to some awesome help (@DavidKoontz) I have finished the code. There is no need to debounce the buttons in my code b/c my altera board seems to pick up the low signals just fine.Since I am only using one clock the buttons react slowly b/c the Clock is set to 1Hz.

Library ieee;
USE ieee.std_logic_1164.ALL;
ENTITY CountDownTimer IS
PORT(
CLK,RESET: IN STD_LOGIC;
a1, b1, c1, d1, e1, f1, g1 : OUT STD_LOGIC;
a2, b2, c2, d2, e2, f2, g2 : OUT STD_LOGIC;
a3, b3, c3, d3, e3, f3, g3 : OUT STD_LOGIC;
a4, b4, c4, d4, e4, f4, g4 : OUT STD_LOGIC;
--All 4 buttons for timer
BUTTON0, BUTTON1, BUTTON2, BUTTON3: IN STD_LOGIC;
--LEDR9
ON_OFF_LED: OUT BIT;
--LEDR9-R6
INPUT_LED1, INPUT_LED2, INPUT_LED3, INPUT_LED4: OUT BIT;
--LEDR0
DONE_LED: OUT BIT);
END CountdownTimer;
ARCHITECTURE Counter OF CountDownTimer IS
--Define state machine
TYPE STATE_TYPE IS (A_ON_OFF, B_INPUT, C_COUNTDOWN, D_DONE);
SIGNAL State : STATE_TYPE;
--Buttons produce 0 when pressed, signal for 1 when pressed
SIGNAL B3D0, B3D1, B3D2, B3D3: STD_LOGIC := '0';
SIGNAL B2D0, B2D1, B2D2, B2D3: STD_LOGIC := '0';
SIGNAL CLOCK: STD_LOGIC := '0';
SIGNAL Count: INTEGER:= 1;
--SIGNAL for range of integer values
SIGNAL Minute1 : INTEGER RANGE 0 TO 6;
SIGNAL Minute2 : INTEGER RANGE 0 TO 9;
SIGNAL Second1 : INTEGER RANGE 0 TO 5;
SIGNAL Second2 : INTEGER RANGE 0 TO 9;
--Output for the seven segment displays
SIGNAL OUTPUT_HEX0 :  STD_LOGIC_VECTOR(6 DOWNTO 0);
SIGNAL OUTPUT_HEX1 :  STD_LOGIC_VECTOR (6 DOWNTO 0);
SIGNAL OUTPUT_HEX2 :  STD_LOGIC_VECTOR (6 DOWNTO 0);
SIGNAL OUTPUT_HEX3 :  STD_LOGIC_VECTOR (6 DOWNTO 0);
SIGNAL B3_HOLD: STD_LOGIC := '0'; --Gets 1 if button3 was held
SIGNAL B2_HOLD: STD_LOGIC := '0'; --Gets 1 is button2 was held

BEGIN

             --Segment 1 display pins
             a1 <= OUTPUT_HEX0(6);
             b1 <= OUTPUT_HEX0(5);
             c1 <= OUTPUT_HEX0(4);
             d1 <= OUTPUT_HEX0(3);
             e1 <= OUTPUT_HEX0(2);
             f1 <= OUTPUT_HEX0(1);
             g1 <= OUTPUT_HEX0(0);
             --Segment 2 display pins
             a2 <= OUTPUT_HEX1(6);
             b2 <= OUTPUT_HEX1(5);
             c2 <= OUTPUT_HEX1(4);
             d2 <= OUTPUT_HEX1(3);
             e2 <= OUTPUT_HEX1(2);
             f2 <= OUTPUT_HEX1(1);
             g2 <= OUTPUT_HEX1(0);
             --Segment 3 display pins
             a3 <= OUTPUT_HEX2(6);
             b3 <= OUTPUT_HEX2(5);
             c3 <= OUTPUT_HEX2(4);
             d3 <= OUTPUT_HEX2(3);
             e3 <= OUTPUT_HEX2(2);
             f3 <= OUTPUT_HEX2(1);
             g3 <= OUTPUT_HEX2(0);
             --Segment 4 display pins
             a4 <= OUTPUT_HEX3(6);
             b4 <= OUTPUT_HEX3(5);
             c4 <= OUTPUT_HEX3(4);
             d4 <= OUTPUT_HEX3(3);
             e4 <= OUTPUT_HEX3(2);
             f4 <= OUTPUT_HEX3(1);
             g4 <= OUTPUT_HEX3(0);

            WITH Second2 SELECT
            --One's second place, 0 to 9
            OUTPUT_HEX0 <= "0000001" WHEN 0,
                                "1001111" WHEN 1,
                                "0010010" WHEN 2,
                                "0000110" WHEN 3,
                                "1001100" WHEN 4,
                               "0100100" WHEN 5,
                                "0100000" WHEN 6,
                                "0001101" WHEN 7,
                                "0000000" WHEN 8,
                                "0001100" WHEN 9,
                                "0000001" WHEN OTHERS;
            WITH Second1 SELECT
            --Tens second place, 0 to 5
            OUTPUT_HEX1 <= "0000001" WHEN 0,
                                "1001111" WHEN 1,
                                "0010010" WHEN 2,
                                "0000110" WHEN 3,
                                "1001100" WHEN 4,
                               "0100100" WHEN 5,
                                "0000001" WHEN OTHERS;
            WITH Minute2 SELECT
            --Ones minute place, 0 to 9
            OUTPUT_HEX2 <= "0000001" WHEN 0,
                                "1001111" WHEN 1,
                                "0010010" WHEN 2,
                                "0000110" WHEN 3,
                                "1001100" WHEN 4,
                               "0100100" WHEN 5,
                               "0100000" WHEN 6,
                                "0001101" WHEN 7,
                                "0000000" WHEN 8,
                                "0001100" WHEN 9,
                                "0000001" WHEN OTHERS;
            WITH Minute1 SELECT
            --Tens minute place, 0 to 6
            OUTPUT_HEX3 <= "0000001" WHEN 0,
                                "1001111" WHEN 1,
                                "0010010" WHEN 2,
                                "0000110" WHEN 3,
                                "1001100" WHEN 4,
                               "0100100" WHEN 5,
                               "0100000" WHEN 6,
                                "0000001" WHEN OTHERS;


PROCESS(CLK)
    BEGIN
        IF RISING_EDGE(CLK) THEN 
            Count <= Count + 1;
            IF (Count = 30000000) THEN
                CLOCK <= NOT(CLOCK);
                Count <= 1;
            END IF;
        END IF; 
END PROCESS;

PROCESS(CLOCK)
BEGIN
    IF RISING_EDGE(CLOCK) THEN
        B3D0 <= BUTTON3;
        B3D1 <= NOT B3D0;
        B3D2 <= B3D1;
        B3D3 <= B3D2;
        B2D0 <= BUTTON2;
        B2D1 <= NOT B2D0;
        B2D2 <= B2D1;
        B2D3 <= B2D2;
        B3_HOLD <= B3D1 AND B3D2 AND B3D3;
        B2_HOLD <= B2D1 AND B2D2 AND B2D3;
    END IF;
    END PROCESS;

PROCESS(CLOCK)
BEGIN       
IF RESET = '1' THEN --Async Reset
            State <= A_ON_OFF;
ELSIF RISING_EDGE(CLOCK) THEN

    CASE State IS
---------------------------------A_ON_OFF---------------------------------  
        WHEN A_ON_OFF =>
            --Red LED9
            ON_OFF_LED <= '1';
            Minute1 <= 0;
            Minute2 <= 0;
            Second1 <= 0;
            Second2 <= 0;
            IF (BUTTON0 = '0') THEN
                                    ON_OFF_LED <= '0';
                                    State <= B_INPUT;
            END IF; 
---------------------------------B_INPUT/PAUSE---------------------------------                 

        WHEN B_INPUT =>

            --Light up LEDs
            INPUT_LED1 <= '1';
            INPUT_LED2 <= '1';
            INPUT_LED3 <= '1';
            INPUT_LED4 <= '1';
            IF (Minute1 = 6) THEN
                Minute2 <= 0;
                Second1 <= 0;
                Second2<= 0;
                State <= B_INPUT;
            END IF;
            --Count up button   
            IF (BUTTON3 = '0' AND B3_HOLD = '0') THEN   
                IF (Minute1 = 6 AND Minute2 >= 0) THEN
                    Minute1 <= 0;
                    Minute2 <= 1;
                    Second1 <= 0;
                    Second2 <= 0;
                        State <= B_INPUT;
                ELSIF (Minute2 < 9) THEN
                    Minute2 <= (Minute2 + 1);
                    State <= B_INPUT;
                ELSIF (Minute2 = 9) THEN
                    Minute1 <= (Minute1 + 1);
                    Minute2 <= 0;
                    State <= B_INPUT;
                END IF;
            END IF;
            IF (BUTTON3 = '0' AND B3_HOLD = '1') THEN   
                IF (Minute1 = 6 AND Minute2 >= 0) THEN
                        Minute1 <= 0;
                        Minute2 <= 5;
                        Second1 <= 0;
                        Second2 <= 0;
                        State <= B_INPUT;
                ELSIF (Minute2 < 5) THEN
                    IF (Minute2 = 0) THEN
                        Minute2 <= (Minute2 + 5);
                        State <= B_INPUT;
                    ELSE
                        Minute2 <= (Minute2 + 1);
                        State <= B_INPUT;
                    END IF;
                ELSIF (Minute2 = 5) THEN
                    Minute2 <= 0;
                    Minute1 <= (Minute1 + 1);
                    State <= B_INPUT;
                ELSIF (Minute2 > 5) THEN
                    IF (Minute2 = 9) THEN
                        Minute2 <= 0;
                        Minute1 <= (Minute1 + 1);
                        State <= B_INPUT;
                    ELSE
                        Minute2 <= (Minute2 + 1);
                        State <= B_INPUT;
                    END IF;
                END IF;
            END IF;
            --Count down button
            IF (BUTTON2 = '0' AND B2_HOLD = '0') THEN
                IF ((Minute1 = 0) AND (Minute2 = 0)) THEN
                    Minute1 <= 6;
                    Minute2 <= 0;
                    Second1 <= 0;
                    Second2 <= 0;
                ELSIF (Minute2 = 0) THEN
                    Minute2 <= 9;
                    Minute1 <= (Minute1 - 1);
                    ELSE 
                        Minute2 <= (Minute2 - 1);
                END IF;
                State <= B_INPUT;
            END IF;
            IF (BUTTON2 = '0' AND B2_HOLD = '1') THEN
                IF (Minute1 = 0 AND Minute2 = 0) THEN
                        Minute1 <= 6;
                        Minute2 <= 0;
                        Second1 <= 0;
                        Second2 <= 0;
                        State <= B_INPUT;
                ELSIF (Minute2 = 0 AND Minute1 > 0) THEN
                    Minute1 <= (Minute1 - 1);
                    Minute2 <= 5;
                    State <= B_INPUT;
                ELSIF (Minute2 < 5) THEN
                    IF (Minute2 = 0) THEN
                        Minute1 <= (Minute1 - 1);
                        Minute2 <= 5;
                        State <= B_INPUT;
                    ELSE
                        Minute2 <= (Minute2 - 1);
                        State <= B_INPUT;
                    END IF;
                ELSIF (Minute2 = 5) THEN
                    Minute2 <= (Minute2 - 5);
                    State <= B_INPUT;
                ELSIF (Minute2 > 5) THEN
                    Minute2 <= (Minute2 - 1);
                    State <= B_INPUT;
                END IF;
            END IF;
            --Clear button
            IF (BUTTON1 = '0') THEN
                Minute1 <= 0;
                Minute2 <= 0;
                Second1 <= 0;
                Second2 <= 0;
                State <= B_INPUT;
            END IF;
            --Start Button
            IF (BUTTON0 = '0') THEN
                                    --Turn off LEDs
                                    INPUT_LED1 <= '0';
                                    INPUT_LED2 <= '0';
                                    INPUT_LED3 <= '0';
                                    INPUT_LED4 <= '0';
                                    State <= C_COUNTDOWN;
            END IF;
---------------------------------C_COUNTDOWN---------------------------------                       

            WHEN C_COUNTDOWN =>

            IF (Second2 > 0) THEN
                Second2 <= (Second2 - 1);
            ELSIF (Second2 = 0) THEN
                IF (Second1 > 0) THEN
                    Second2 <= 9;
                    Second1 <= (Second1 - 1);
                ELSIF (Second1 = 0) THEN
                    IF (Minute2 > 0) THEN
                        Second1 <= 5;
                        Minute2 <= (Minute2 - 1);
                        IF (Second2 = 0) THEN
                            Second2 <= 9;
                        END IF;
                    ELSIF (Minute2 = 0) THEN
                            IF (Minute1 > 0) THEN
                                IF (Second1 = 0) THEN
                                    Second1 <= 5;
                                    Minute2 <= (Minute2 - 1);
                                    IF (Second2 = 0) THEN
                                        Second2 <= 9;
                                    END IF;
                                END IF;
                                Minute2 <= 9;
                                Minute1 <= (Minute1 - 1);
                            ELSIF (Minute1 <= 0) THEN
                                                        State <= D_DONE;
                            END IF;
                    END IF;
                END IF;
            END IF;
            --Reset Button
            IF (BUTTON1 = '0') THEN
                                        STATE <= A_ON_OFF;
            --Input button
            ELSIF (BUTTON0 = '0') THEN
                                            STATE <= B_INPUT;
            END IF;
---------------------------------D_DONE---------------------------------                        

            WHEN D_DONE =>
            --LEDR0
            DONE_LED <= '1';
                --Reset button
                IF (BUTTON1 = '0') THEN
                                            DONE_LED <= '0';
                                            STATE <= A_ON_OFF;

                --Input button
                ELSIF (BUTTON0 = '0') THEN
                                                DONE_LED <= '0';
                                                STATE <= B_INPUT;
                END IF;                  
        END CASE;
END IF;
END PROCESS;

END Counter;                  

Answer 1

In choice when b_input in the case statement you'll increment minute2 for every clk button3 is low, it's an enable. It needs to be converted to an event unless your counting on a real slow clk (noting you're also producing a slow clock). This enable isn't synchronous to clk, it can cause metastability for a setup or hold time violation.

In the process

PROCESS(BUTTON3)
BEGIN   
IF RISING_EDGE(BUTTON3) THEN
FF3 <= NOT(BUTTON3);
END IF;
END PROCESS;

This will assign ff3 low but never high. Rising edge means a transition from '0' to '1' on button3. In other words your driving ff3 low only.

This is why your counter doesn't change. Besides the potential metastability issue the enable should occur for a single clk.

For simulation integer range bound counters (e.g. minute2) will overflow. VHDL doesn't do range bound modular arithmetic.

signal minute2 : integer range 0 to 9;

When minute2 reaches 10 in state b_input simulation will quit because of a range error.

That increment in choice b_input should look something like:

                    if minute2 = 9 then
                        minute2 <= 0;
                    else
                        minute2 <= minute2 + 1;
                    end if;

(And there's a subtle hint you should simulate this thing.)

You need to use a single clk enable from the button(s) for incrementing counters.

The way to do this is a) debounce the buttons, b) detect the rising edge in the clk domain for one clk.

All this is complicated because you're using both 'clkandclock` as a clock.

A recommendation on how to implement de-bounce, edge detect and single clk enables is dependent on what your clock rate actually is.

Alternatively you could contemplate a bunch of clock ORing where you use buttons to increment things and gate clock for c_countdown.

You didn't include the context clause before the entity declaration:

library ieee;   
use ieee.std_logic_1164.all;

One question, should I use Falling_Edge of button 3 for ff3 because doing that my code also had issues

It would seem likely that if you had falling edge issues you'd also have rising edge issues. The root of all problems would likely be contact bounce.

And no I don't advocate using the falling edge of button 3 in this case.

The issue here is to produce a single clock enable from button 3 (or alternatively use it as a clock). It also needs debouncing. The duration of a switch or button bounce is dependent on several things - mass of the contacts, how hard it's operated and how springy the switch arm is come to mind.

There are several ways to get rid of bounce. For instance you can use a normally open and normally closed pair of contacts to operate an RS latch, requiring two switch or button signals to be in opposite binary states. You can temporally filter (with a clock interval in the 10's of milliseconds range) requiring N number of stable samples. This also filters out metastability from going from an asynchronous domain to a synchronous (clocked) domain.

To use the equivalent of FF3 as an enable for clk, you need to debounce the button, and detect the rising edge in the 'clk` domain.

Because your buttons are almost guaranteed to be single pole switches you need some sort of temporal filtering. If the clock for that is related to 'clk' you could simply use a long interval enable produced by a counter to sample a button successively. This allows you to use an additional flip flop in the clk domain to detect it used to be low and it's now high, the output of that gate used where FF3 is used now.

If clk is 50 MHz as you say, debouncing could entail generating an enable from the Count counter.

Let's call that debounce_en.

Note this is setup to sample some time after the falling edge of button3. An old high and either one or two succesive lows. This is milliseconds after the fall of button3 so it should be safe.

50 Mhz divided by 4000000 gives a clock baud of the reciprocal of 12.5 or 8 ms. That's a good starting rate for a temporal filter for debouncing button3:

    signal debounce_en: std_logic;
    signal button3_d0, button3_d1, button3_d2:

    begin

...

UNLABELLED1:
    process(clk)
    begin
        if rising_edge(clk) then 
            debounce_en <= '0';  -- one ping only.
            count <= count + 1;
            if count = 4000000 then
                clock <= not clock;
                debounce_en <= '1';  -- for one clk every 4000000
                count <= 1;
            end if;
        end if; 

    end process;

We can use debounce_en to sample button3 instead of producing FF3:

process (clk)
begin
    if rising_edge(clk)  and debounce_en = '1' then
        button3_d0 <= button3;
        button3_d1 <= button3_d0;
        button3_d2 <= button3_d1;
     end if;


button3_enable <= not button3_d1 and button3_d2 and debounce_en;

And the reason this would work is because more than likely 8 ms is long enough an interval to debounce one of these switches. (And if it's not add another stage and:

button3_enable <= not_button3_d1 and not button3_d2 and button3_d3 and debounce_en;

Which requires no 'ringing' be captured by three successive flip flops (30 ms).

'button3_enable would be used in place of FF3, all three (or more) of the new signals are type std_logic and the flip flops are used as a shift register.

Using a temporal filter requiring known values for so long can potentially filter out short pulses on the buttons, you'd be hard pressed to pulse a button that fast.

And as far as operating `minute2 in the case statement:

            when b_input =>
                if button3_enable = '1' then  -- count up button 
                    if minute2 = 9 then
                        minute2 <= 0;
                    else
                        minute2 <= minute2 + 1;
                    end if;
                    input_led <= '1';  --green led7
                end if;

And how you were to derive debounce_en from Count might be subject to change should you change clock's clock rate (I'd be tempted to get it to run real time myself). You don't have C_COUNTDOWN finished, but it looks like ideally it would operate with a one second rate enable and have the seconds, and minutes counters cascaded.

Notes on DE1 claim of debouncing buttons

I found a DE1-SoC board manual and in figure 3-14 and section 3.6.1 it claims the use of a 74HC245 schmidt trigger buffer (nominally bidirectional, undoubtedly used unidirectionally) is sufficient to use buttons as clocks. That would depend on board layout, capacitance and inductance of one of the buttons, the size of the pull up resistors, and if they say so... (and it's likely it works, they've offered it in several generations of board designs), how does this impact the above description?

Well you still need to synchronize to clk and produce a one clk period enable. So not much.