cancel
Showing results for 
Search instead for 
Did you mean: 
Highlighted
Participant
Participant
1,332 Views
Registered: ‎05-05-2016

case statement confusion

Jump to solution

I have a debounce for the buttons on a Basys3 board.  Im tyring to count button presses.  My count signal is getting set back to zero and I don't understand why.

 

----------------------------------------------------------------------------------
-- Company: 
-- Engineer: 
-- 
-- Create Date: 01/03/2017 04:47:26 PM
-- Design Name: 
-- Module Name: FSM_ila_test_top - Behavioral
-- Project Name: 
-- Target Devices: 
-- Tool Versions: 
-- Description: 
-- 
-- Dependencies: 
-- 
-- Revision:
-- Revision 0.01 - File Created
-- Additional Comments:
-- 
----------------------------------------------------------------------------------


library IEEE;
use IEEE.STD_LOGIC_1164.ALL;
use IEEE.NUMERIC_STD.ALL;


entity FSM_ila_test_top is
    Port (    
           clk 		: in    STD_LOGIC;
           btn      : in    STD_LOGIC_VECTOR (4 downto 0)
          );
end FSM_ila_test_top;

architecture Behavioral of FSM_ila_test_top is

signal button       : std_logic_vector(4 downto 0); 
signal next_button	: std_logic_vector(4 downto 0);
signal cnt 			: unsigned(7 downto 0) := "00000000";
signal next_cnt		: unsigned(7 downto 0) := "00000000";
signal cnt_delay	: unsigned(7 downto 0) := "00000000";
signal Q1, Q2, Q3 	: STD_LOGIC_VECTOR(4 downto 0) := "00000";


COMPONENT button_ila

PORT (
	clk 	: IN STD_LOGIC;
	probe0 	: IN STD_LOGIC_VECTOR(4 DOWNTO 0); 
	probe1 	: IN STD_LOGIC_VECTOR(4 DOWNTO 0); 
	probe2 	: IN STD_LOGIC_VECTOR(7 DOWNTO 0);
	probe3 	: IN STD_LOGIC_VECTOR(7 DOWNTO 0)
);
END COMPONENT  ;


begin
                         
process(clk)
begin
    if rising_edge(clk) then
         Q1 <= btn;
         Q2 <= Q1;
         Q3 <= Q2;
   end if;
end process;
 
next_button <= Q1 and Q2 and (not Q3);  
        
process(clk)
begin
	if rising_edge(clk) then
		cnt			<= next_cnt;
		button		<= next_button;
	end if;
end process;
                
process(button)
    begin
		
		case button is
	
		when "00010" 	=>	--up
			next_cnt	<=	cnt + 1;
			
		when "00100"   =>	--down	
			next_cnt	<=	cnt - 1;
			
		when "00001" 	=>	--center	
			next_cnt	<=	TO_UNSIGNED(0,8);
			
        when others =>  
      	end case;  
      	      
      end process;      
 
button_press : button_ila
      PORT MAP (
      	clk 	=> clk,
      	probe0 	=> next_button, 
      	probe1 	=> button, 
      	probe2 	=> std_logic_vector(next_cnt),
      	probe3 	=> std_logic_vector(cnt)
      );        
               

end Behavioral;

I press the up button and my ILA shows the cnt set to 1 for a single clock and then back to 0.

 

comb_process.JPG

 

Thanks

Tags (1)
0 Kudos
1 Solution

Accepted Solutions
Highlighted
Guide
Guide
1,615 Views
Registered: ‎01-23-2009

Re: case statement confusion

Jump to solution

I expected was the blank "when others => " option in the case statement to do nothing. 

 

And what would "do nothing" actually do? The process in question is not a synchronous process (i.e. not a flip-flop_. As the "next state" generator for the "cnt" variable, it would normally be expected to be a combinatorial process. So how does a combinatorial process "do nothing". By the rules of RTL, this means that next_cnt should "maintain the value it had previously" - a combinatorial process can't do that. So what you have here is a ????? (the answer is latch).

 

I have next_cnt initialized to 0 so I thought the next_cnt value would either be 0 or one of the values written to it from one of the other case options. 

 

The initialization value done during the declaration only affects the value of the signal at time 0 - after that it has no effect.

 

Are you tell me that I can't trust Xilinx provided HDL?  That is unsettling.

 

In this case, I guess not. This code template was probably written more than a dozen years ago, and is in the depths of "things people rarely use as a code template". So yes, it is incorrect as well as being out of date.

 

Avrum

View solution in original post

0 Kudos
5 Replies
Highlighted
Guide
Guide
1,318 Views
Registered: ‎01-23-2009

Re: case statement confusion

Jump to solution

Your case statement tells the tool what to set "next_cnt" to in 3 out of 32 possible conditions. In the remaining (including 00000) you do not set next_cnt... What do you expect it to be?

 

And, by the way:

   - you do not "debounce" the switches - you simply synchronize and edge detect them (this is not the same as debouncing)

   - your synchronizer is illegal - the Q1 and Q2 flip-flops are supposed to be metastability flip-flops

      - they should have the ASYNC_REG property set on them

      - you must never use the output of anything but the last flip-flop in a synchronizer chain

          - so you can use 'next_button <=  Q2 and (not Q3);' but you may not include Q1 in this combinatorial logic

 

Avrum

Highlighted
Moderator
Moderator
1,244 Views
Registered: ‎07-21-2014

Re: case statement confusion

Jump to solution

@waidcsuf Please free to post if you have any further queries, else please close the thread by accepting above post as an accepted solution.

 

Thanks

Anusheel 

0 Kudos
Highlighted
Participant
Participant
1,239 Views
Registered: ‎05-05-2016

Re: case statement confusion

Jump to solution

I expected was the blank "when others => " option in the case statement to do nothing.  I have next_cnt initialized to 0 so I thought the next_cnt value would either be 0 or one of the values written to it from one of the other case options.  Why does next_cnt get set to zero when the "when others =>" is selected?

 

As for the circuit that I believed was a debounce, I got that VHDL from the Vivado language template by typing debounce in the search window.  The code is in the language template under VHDL\Synthesis Constructs\Coding Examples\Misc\Debounce circuit.

 

The Verilog version is called "One-shot, Debounce Circuit"

 

Below is a cut-n-paste exactly from the Vivado language template

 

--  Provides a one-shot pulse from a non-clock input, with reset
--**Insert the following between the 'architecture' and
---'begin' keywords**
signal Q1, Q2, Q3 : std_logic;

--**Insert the following after the 'begin' keyword**
process(<clock>)
begin
   if (<clock>'event and <clock> = '1') then
      if (<reset> = '1') then
         Q1 <= '0';
         Q2 <= '0';
         Q3 <= '0';
      else
         Q1 <= D_IN;
         Q2 <= Q1;
         Q3 <= Q2;
      end if;
   end if;
end process;

Q_OUT <= Q1 and Q2 and (not Q3);

Are you tell me that I can't trust Xilinx provided HDL?  That is unsettling.

 

0 Kudos
Highlighted
Scholar
Scholar
1,223 Views
Registered: ‎08-01-2012

Re: case statement confusion

Jump to solution

You code has a couple of issues around latches and simulation missmatches. I suggest checking the warnings for latches created on "next_cnt" value. You didnt specify what next_cnt should be in all cases of button, so it will create a memory (latch) when it is not one of the value choices. Really not good in an FPGA. You are also missing "cnt" from the sensitivity list of the process, meaning your simulation is unlikely to match the behaviour of the real circuit (as sensitivity lists are ignored for syntheis - check warnings again.

 

And Q1 will have a high probability of meta-stability. So it is best if you do not use it for anything other than transfering the button value. Do not read it as it could be anything (including X on real hardware - causing more meta stability). Much better to shift everything and do your edge detect on those, and ignore Q1. 

0 Kudos
Highlighted
Guide
Guide
1,616 Views
Registered: ‎01-23-2009

Re: case statement confusion

Jump to solution

I expected was the blank "when others => " option in the case statement to do nothing. 

 

And what would "do nothing" actually do? The process in question is not a synchronous process (i.e. not a flip-flop_. As the "next state" generator for the "cnt" variable, it would normally be expected to be a combinatorial process. So how does a combinatorial process "do nothing". By the rules of RTL, this means that next_cnt should "maintain the value it had previously" - a combinatorial process can't do that. So what you have here is a ????? (the answer is latch).

 

I have next_cnt initialized to 0 so I thought the next_cnt value would either be 0 or one of the values written to it from one of the other case options. 

 

The initialization value done during the declaration only affects the value of the signal at time 0 - after that it has no effect.

 

Are you tell me that I can't trust Xilinx provided HDL?  That is unsettling.

 

In this case, I guess not. This code template was probably written more than a dozen years ago, and is in the depths of "things people rarely use as a code template". So yes, it is incorrect as well as being out of date.

 

Avrum

View solution in original post

0 Kudos