Need help in code

[Altera_Forum]

Hello 

I'm currently working on my assignment in Spartan-3E.My assignment is: While button is clicked show values of switches in low 4 bit of LED,when button is released move from left to right.Switching speed 2Hz. But i haven't got any success so far.It not even showing values of switch. And here is my code: library IEEE; use IEEE.STD_LOGIC_1164.ALL; use ieee.std_logic_unsigned.all; use ieee.std_logic_arith.all;   entity daalgavar_13 is Port ( clk : in STD_LOGIC;  led : out STD_LOGIC_VECTOR (7 downto 0);  button : in std_logic;  switch : in STD_LOGIC_VECTOR (3 downto 0)); end daalgavar_13; architecture Behavioral of daalgavar_13 is signal clk_2G: std_logic; signal tooluur: integer range 0 to 25000000; signal data : STD_LOGIC_VECTOR (7 downto 0);  begin  --led<=data; process(clk_2G) --, switch) variable temp:std_logic_vector(7 downto 0);   begin --if (rst = '0') then if clk_2G'event and clk_2G='1' then  if button='1' then  data <="0000" & switch(3 downto 0); else    temp := data;  data <= temp (6 downto 0)& temp(7) ;   end if; end if; end process; led<=data; process(clk) begin  if clk'event and clk='1' then  if tooluur=25000000 then --2Hz tooluur<=0; clk_2G<='1'; else  tooluur<=tooluur+1; clk_2G<='0'; end if; end if; end process; end Behavioral;   Thank you

[Altera_Forum]

Best not mention Spartan on this Altera forum. However, I think we can overlook that... 

 You're code, whilst it could be improved, looks fundamentally there even if it doesn't employ best practice. I think it should do roughly what's been asked of you. So, a couple of questions back to you.  What isn't it doing? Anything? Nothing?  Have you constrained the I/O to the correct pins on the device?  Are any of the input or output signals active low - e.g. does the button generate a logic LOW on the pin when it's pressed (rather than a logic HIGH as your code is expecting).  Have you simulated any of this?  You don't need the 'temp' variable. Anything unnecessary should be removed to keep any code clear, simple and uncluttered.  Cheers, Alex

[Altera_Forum]

1.  

You should used a reset signal to have a known initial state.  In your code, the critical uninitialized signals are 'data' and 'tooluur', you use them with unknown values  2.  You have 2 clocks for a very simple design. Best practice is to have the main clock and an enable signal for the 2Hz clock  if clk'event and clk = '1' then  if en_2G = '1' then -- pulse at clk frequency each 0.5s  ...   3. If you want to make a more complex design with the signal button, you should debounce it.

[Altera_Forum]

Note, it's helpful to use the forums CODE tags. It's hard to read code with no indentation. 

 Just a few notes:  You don't say if nothing at all shows up on the LED's or if it is just wrong. Most boards turn an LED on when the value is '0'. So you may want to use:  led <= not data;  Have you tried just driving "led" with a fixed bit pattern to make sure everything is hooked up and functioning? When developing code, it is helpful to do it a small step at a time and make sure things work at each step.   Do you know the value of "switch"? With Quartus, SignalTap would probably show your issue very quickly. Xilinx has ChipScope, but, unless something has changed, it is a pain to use.   Your clk_2G is more of a pulse. Have you checked that your tools are interpreting it as a clock? You set it to 1 for one clock cycle of clk every 25,000,000 cycles. A more common way to do this would be:   process(clk) begin if clk'event and clk='1' then if tooluur = 12_500_000 then --2Hz -- I'm assuming clk is at 50 Mhz. tooluur <= 1; -- You're not at exactly 2 Hz. Minor in this case. clk_2G <= not clk_2G; -- Just invert each time. else tooluur <= tooluur+1; end if; end if; end process;   However, I would not use your clk_2G at all. I'd get rid of it and replace it with a pulse say "check_button". Set check_button to '1' every time tooluur gets to 25,000,000 (in other words, exactly what you were doing). Then in the first process, now clocked by 'clk', put the body of the process inside "if check_button = '1' then". This assumes you really only want to check the button state twice a second as opposed to continuously. Clocks and clock nets are special and scarce resources. Don't use them unless you really need to.      You're only checking "button" twice a second. I assume it is pressed and held?   Asynchronous inputs, which I assume "button" is, should have a few synchronization stages to avoid metastability issues. And it should be debounced (but probably not with a 2 Hz clock).   You could use VHDL "rol" operator on "data" instead of "temp".   I notice you're using std_logic_arith instead of numeric_std. Is there a reason for that?  It's best to replace magic numbers with constants.  As others have mentioned, you should initialize all signals and variables. For synthesis, uninitialized values are probably zero. But simulators will complain.   And yes, I had nothing else to do today.