CODEWITHSUNDEEP
timervhdlcounterbcd
CmYang
CmYang

Reputation: 3

BCD Timer in VHDL

Just started out in VHDL not long ago,out of curiosity.

So I was trying to write BCD timer on spartan 3 board and

Somehow I couldn't find out why it keeps showing 'unexpected with' error.

So if I want to have such function as shown in the link of picture, how can I modify the code?any help would be grateful.

my initial sketch_picture link to imgur
(clickable)

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


entity w3 is
    Port ( clk : in  STD_LOGIC;
           rst : in  STD_LOGIC;
           stp : in  STD_LOGIC;
           an : out  STD_LOGIC_VECTOR (3 downto 0);
           c : out  STD_LOGIC_VECTOR (6 downto 0));
end w3;

architecture timer of w3 is
signal div1 : integer range 0 to 499999 :=0; -- 100Hz
signal ck100hz : std_logic; -- 100Hz output
signal div2 : integer range 0 to 249999 :=0; -- 200Hz
signal ck200hz : std_logic; -- 200Hz output
signal div3 : integer range 0 to 124999 :=0; -- 400Hz
signal ck400hz : std_logic; -- 400Hz output
signal index : integer range 0 to 9 :=0;
signal scan : std_logic_vector (3 downto 0);
signal S : std_logic;
signal disp : std_logic_vector (3 downto 0);

begin
process begin
wait until rising_edge(clk);
if div1 < 499999 then
    ck100hz <= '0';
    div1 <= div1+1;
else 
    ck100hz <= '1';
    div1 <= 0;
end if;
if div2 < 249999 then
    ck200hz <= '0';
    div2 <= div2+1;
else
    ck200hz <= '1';
    div2 <= 0;
end if;
if div3 < 124999 then
    ck400hz <= '0';
    div3 <= div3+1;
else
    ck400hz <= '1';
    div3 <= 0;
end if;
end process;

process begin
wait until rising_edge(clk);
if rst = '1' then 
    index <= 0; 
end if;
if stp = '1' then 
    index <= index; 
end if;

if ck100hz = '1' then
    if index < 3 then index <= index+1;
        else index <= 0;
        if index < 4 and index > 7 then index <= index+1;
            else index <= 0;
            if index < 8 and index > 11 then index <= index+1;
                else index <= 0;
                if index < 12 and index > 15 then index <= index+1;
                    else index <= 0;
   end if;
      end if;
          end if;
             end if;
end if;
end process;

process begin
wait until rising_edge(clk);
if ck400hz = '1' then
    With scan select -- error unexpected With
        an <= an(0) when "00",
              an(1) when "01",
              an(2) when "10",
              an(3) when others;
end if;
end process;

process begin
wait until rising_edge(clk);
if ck200hz = '1' then
    With S select   -- error unexpected With
    disp <= index integer range 0 to 3 when "00",
            index integer range 4 to 7 when "01",
            index integer range 8 to 11 when "10",
            index integer range 12 to 15 when others;
end if;
end process;




with index select
    C <= "1000000" when 0, 
          "1111001" when 1, 
          "0100100" when 2, 
          "0110000" when 3, 
          "0011001" when 4, 
          "0010010" when 5, 
          "0000010" when 6,
          "1111000" when 7,       
          "0000000" when 8,
          "0011000" when 9;

end timer;

Upvotes: 0

Views: 829

Answers (1)

Paebbels
Paebbels

Reputation: 16221

For low-active:
You should assign 0111111 to C in case of index = 0. You have to enable almost all segments. Now your internal calculation would be high-active. The display itself is low-active, because of the PCB layout, thus you should invert the whole C vector before assigning it to a Cathode_n port: Cathode_n <= not C; Note, I used _n to designate the low-active behavior of this port.

Old code:

with index select
  C <= "1000000" when 0, 
       "1111001" when 1,
       -- ...
       "0011000" when 9;

This should be the goal when writing purely high-active code:

with index select
  C <= "0111111" when 0, 
       "0000110" when 1,
       -- ...
       "1100111" when 9;

  Cathode_n <= not C;

High-active means: If a bit is high (1) then a thing is active. In your case an LED of a 7-segment display is activated. Based on the PCB design, you have to drive low (0) to activate a light. This is low-active, because a low value activates something.

The select statement needs to assign the position to 1 that should be enlightened, not the positions to turn off. More over low-active signals should be marked in to code to denote there different behavior. Only a top-level component should translate high-active signals to low-active signals and vice versa. This ensures, the inner parts of your design are purely high-active.

For no driver:
There is no assignment to S and scan. Both signals are 'U' or "UUUU", respectively.

Edit:
Each signal assignment creates a driver on a signal. Currently, your code never assigns any value to S nor scan. Thus the initial values of S and scan become the driving value of the signals. You should run a simulation und see a lot of Us in your waveform.

Synthesis tools might report: Signal S is read, but never assigned.

For using elsif:

I reformatted you horrible if-then-else construct:

if ck100hz = '1' then
  if index < 3 then
    index <= index+1;
  else
    index <= 0;
    if index < 4 and index > 7 then
      index <= index+1;
    else
      index <= 0;
      if index < 8 and index > 11 then
        index <= index+1;
      else
        index <= 0;
        if index < 12 and index > 15 then
          index <= index+1;
        else
          index <= 0;
        end if;
      end if;
    end if;
  end if;
end if;

Now we see, you code can not use elsif, because you have assignments in the else branch before the next if statement. On the otherhand, the index <= 0; assignment is redundant and can be removed:

if ck100hz = '1' then
  if index < 3 then
    index <= index+1;
  else
    if index < 4 and index > 7 then
      index <= index+1;
    else
      if index < 8 and index > 11 then
        index <= index+1;
      else
        if index < 12 and index > 15 then
          index <= index+1;
        else
          index <= 0;
        end if;
      end if;
    end if;
  end if;
end if;

Now we can transform it to use elsif branches:

if ck100hz = '1' then
  if index < 3 then
    index <= index + 1;
  elsif index < 4 and index > 7 then
    index <= index + 1;
  elsif index < 8 and index > 11 then
    index <= index + 1;
  elsif index < 12 and index > 15 then
    index <= index+1;
  else
    index <= 0;
  end if;
end if;

Much more readable, right?

Next, lets check your expressions in that statement:

elsif index < 4 and index > 7 then

index cannot be less than 4 and greater than 7 at the same time. So lets play synthesis and optimized away unreachable branches:

if ck100hz = '1' then
  if index < 3 then
    index <= index + 1;
  else
    index <= 0;
  end if;
end if;

Ok, other problems in the code:

if rst = '1' then 
  index <= 0; 
end if;
if stp = '1' then 
  index <= index; 
end if;

if ck100hz = '1' then
  -- ...
end if;

Reset should always have highest priority. In your case, e.g. stp has higher priority. In a good case you're only wasting FPGA resources, in a bad case synthesis can not translate your code to primitives in the FPGA. E.g. flip-flops with matching reset behavior.

Correct implementation:

if rst = '1' then 
  index <= 0; 
elsif stp = '1' then 
  index <= index; 
elsif ck100hz = '1' then
  -- ...
end if;

I think for now, you have enough input to fix your code.

Upvotes: 1

Related Questions