FPGA Nixie Clock Code Issues

[FotatoPotato]

Hi all, I just recently got into FPGA's and after watching some tutorials I thought I would try to throw together my own Nixie Clock. After a few hours of writing code I finally went to synthesize it all in the Xilinx ISE 14.1 program but I ran into a few errors that for the life of me I cant seem to fix Basically every time I try to synthesize my design it will leave out 3 of my 4 denounce modules. Specifically the DBS, DBM, and the DBH modules. It also spits out a bunch of errors in the console and I have no Idea how to fix them. There are pics of the errors linked to this post btw.

In my design I have 6 buttons, 2 for the seconds, 2 for the minutes and 2 for the hours. These buttons set the time by increasing their corresponding tubes number by 1. Then I use the onboard 50Mhz clock and have it trigger a clock "variable" once per second to increment the number of each tube. Finally I have a hard reset button on the FPGA board that resets everything and a soft reset button that only reset's the numbers.

Just a warning to all the experienced VHDL programmers out there: I have been programming in VHDL for about 2 days now, so I am by no means good at it and while making this program I had no idea if It would work, and there is no example code that I could find for a nixie clock, so I was in the dark for this project. Anyway, I have linked a folder with all the ISE files if you want to check It out. I also have some pics of my top level block diagram and finally I have 2 links, one to the FPGA board I'm using and one to the data sheet for the Nixie tube drivers.

Here is the link to my code: http://www.mediafire.com/file/q3rovss6v1oz272/NixieTubeClock.rar
Here is the link to my FPGA Board: https://www.micro-nova.com/mercury/
Here is the link to the Nixie driver Data sheet: https://tubehobby.com/datasheets/k155id1.pdf

If you find the problem please let me know!

Thanks! 

[rstofer]

You really should be using 14.7...

Are you debouncing signals that you don't actually use?  Sometimes we define a register of some width and then only use a few bits.  XST will trim the register and complain.
It appears that some of your logic evaluates to a constant.

[FotatoPotato]

Thanks for the quick response!

I do want to use 14.7 but I don't like the fact that It has to run on a VM. I am working on getting Ubuntu installed so that I can run it on that. Also, I am using all the debounce modules, I just made them separate because if I were to put them all together it would be huge and quite confusing, as if my code wasn't confusing enough   

Also, could you point out to what logic evaluates to a constant?

[rstofer]

I don't see the .ucf file that assigns pins to signals.  If the signals don't really go to a pin, the logic will be scrapped.

[FotatoPotato]

I do Have the UCF file. Do I need it if I am just synthesizing to see the schematic?

Also, would I set the IO as (3 downto 0) and then assign that IO to 4 pins on the FPGA, or would I just set it as a standard logic and make 4 IO's for each tube ?

I added the UCF file if you need it.

[hamster_nz]

The tools are very good at finding and eliminating dead or redundant code - after all, everything has to resolve down to gates and wires in the end.

These warnings look to be the sort of "Hey, I've discovered that I can do this, I think you should know!" warnings, not the "This is really dumb, and will never work as intended" warnings.

Without seeing source code, it is hard to say what is going on. It might be that your debouncers all have n-bit counter, that all start at zero, and are lock-stepped so they always have the same state. or that they are connected to static inputs, so it can prove that the outputs will remain static and the logic can be removed.

[FotatoPotato]

Hey hamster, thanks for the reply. You can download the code if you want, I linked it in my first post. My problem is that I'm not skilled enough to understand exactly what you mean and fix it in the code So if it isn't too much of an inconvenience, could you take a look at my code and show me whats wrong?

Thanks!

[hamster_nz]

Got the project to open... stream of random comments:

1. When you create a project in ISE, set a working directory in the "Design Properties". That will stop your top level directory from being fulled with junk files.

2. clkDiv.vhd:
- Set default values for outputs. eg "clkTimeSlow : out  STD_LOGIC := '0';", otherwise the first cycle of simulation will have undefined values in it (unless you are using an explicit reset, and need to check your reset works)

- rather than "if CLK'event and CLK = '1' then" use "if rising_edge(CLK) then'.Doesn't change the generate low-level output but is much better style.

- You can use ":= (others => '0')" rather than ":= x"0000";" to initialize signals. Saves a lot of effort when initialising odd sizes or long values.

- I don't like using integers in synthesizable codes, so would have used "unsigned(25 downto 0)" rather than "integer range 0 to 49999999" for s_clkCounterTime.

- If you want to use a bit of a register as a source for a clock, you really should send it through a clock buffer Normally the tools will insert this for you, but it is good style to do this yourself.

Code: [Select]

Library UNISIM;
use UNISIM.vcomponents.all;

BUFG_1 : BUFG
   port map (
      O => clkSlow,     -- Clock buffer output
      I =>  s_clkSlow     -- Clock buffer input
   );

3. ResetDebounceHW.vhd
Although I see what you are trying to do, this is wrong on a few levels due to the hardware implementation. In softwarish terms there is a race between:
  "if HardReset = '1' then"
and
  "elsif(clkSL'event and clkSL = '1') then".

The signals take time to spread across the chip, so some if HardReset changes state at around the same time that the clock ticks then the values in "s_cleanOutReset" and "s_debounce" can be FUBARed.

A better form is

  if rising_edge(clk) then
    if HardReset = '1' then
      s_debounce <= (others => '1');
    else
      s_debounce <= s_debounce(s_debounce'high downto 0) & '0';
    end if;
    cleanOutReset <= s_debounce(s_debounce'high);
  end if;

As long as the HardReset pulse is long enough to trigger the async resets on all of s_debounce, then all works as expected

You might want to stretch the pulse width (from a few cycles to maybe 1,000,000 cycles with a counter, so you won't get multiple resets due to switch bounce.

... more to follow...

[FotatoPotato]

Wow, what a response  ! Thanks so much! This really helped. It’s getting late by me so I’ll make the changes tomorrow and let you know if any other errors pop up. Also as my first Hardware definition program written without any reference or tutorial, what do you think? I know there is room for improvement (duh) but is it as bad as I think it is?

Thanks again!

[rstofer]

There's one other requirement for the .ucf file:  The signal names need to match with the PORT list in the top level.  I imagine that what you have is a factory provided .ucf and it's probably fine.  What you need to do is rename the signals in one place or the other.  Given the factory nature of the .ucf file, I would change the VHDL file(s) to match.  You will have to comment out unused signals in the .ucf file.  Highlight the lines, right click and select Comment -> Selection

Even with the issues you can run just Synthesis and then View RTL Schematic.  Drill down by double-clicking on a block that interests you,

[rstofer]

This is a matter of style and almost irrelevant...

You have a Component declaration at the top of the file and an instantiation near the bottom (or wherever),  As a result, the PORT list needs to agree in 3 places, the declaration, the instantiation and the module itself.

Another way to do that is to not use the component declaration and simply tell the compiler to go find the module like this:

   

Code: [Select]

Inst_SPI: entity work.spi Port Map(
Clk => ClkIn,
SPI_Clk => SPI_Clk,
SPI_DataOut => SPI_DataOut,
SPI_CS_n => SPI_CS_n,
Reg_0 => IR,
Reg_1 => IAR,
Reg_2 => SBR,
Reg_3 => SAR);
[/font]

This tells the compiler to look in the work directory for the module.  This saves a lot of typing and removes one opportunity to mess up.

Later on, look in Help for 'instantiation template'.  The tool will build the instantiation code for you and you just copy and paste it into your own file.  This makes sure all the signals are included and spelled correctly.

ISE is a really nice tool.

[hamster_nz]

... following on from above...

The time counters and the display runs on different clock signals (clkST for time, and clkSL for Debounce). This is most likely an error in the high level design. It would be far simpler if this just had a single clock domain, and then a "clock enable" pulse for the two different uses. e.g.

if rising_edge(clk) then
   if pulse_per_second = '1' then
     ... advance the time...
   end if;
end if;

You have FSM's in the time counter "CounterMinutes" and so on. You could most likely do it with simpler logic that avoids that complexity - something like:

  if rising_edge(clk) then
    if pulse_per_minute = '1' and timeSetM = '1' then
        -- increase time_min by 2
        if  time_min = 8 then
           time_min <= 0;
       elsif time_min = 9 then
           time_min <= 1;
       else
           time_min <= time_min + 2;
       end if;
    elsif pulse_per_minute = '1' or timeSetM = '1' then
       -- increase time_min by 1
       if time_min = 9 then
           time_min <= 0;
       else
           time_min <= time_min + 1;
       end if;
   end if;

Depending on the type for time_min you might need to use "time_min <= to_unsigned(2,time_min'length);" or even "time_min <= std_logic_vector(to_unsigned(2,time_min'length));", or just "time_min <= x"2";" to assign the constant values to the signals.

You will then need some logic to change the buttons from a long pulse to a single pulse. Something like:

if rising_edge(clk) then
  if debounced_button_x = '1' and debounced_button_x_last = '0' then
     do_something_this_cycle_flag <= '1';
  else
     do_something_this_cycle_flag <= '0';
  end if;
  debounced_button_x_last <= debounced_button_x;
end if;

Don't fret too much. There are all common design patterns in any HDL, and you just haven't seen or written enough HDL yet to see the patterns :-)

The big thing is to try and not introduce too many clock domains.

[FotatoPotato]

Ok, I think I’m starting to get it, its still a bit confusing but I think I’ll be fine. One last question: I have two clocks running in this design, one at aout 762 Hz for the debouncers so that they had a 10 ms period between clock cycles and then the 1 hz clock for the actual time. Based on what you are saying it is best if I dont add any more clocks, so how would I get the ten’s seconds, minutes and hours to increment accordingly while only using the 1 hz clock. Also I saw in the code that you posted that you assigned a ‘2’ to the time_min value. I thought you could only assign a 1 or a 0 and that if you wanted to do a larger number you would have to use a binary value. Is this correct? And if so can I just say something like “st_ timeValue = ‘9’;” instead of “st_timeValue = “1001”; (1001 is 9 in binary)

Thanks 

[rstofer]

In broad terms, you have only one clock and you generate a bunch of enable signals that do things like update digit counters and so on.

You can have several clock dividers to generate the enable signals.  When the divider value reaches the terminal count, set an enable signal for just one clock cycle.

Use the enable signal in a clocked process to update whatever is required at that terminal value.

Code: [Select]

process(clk)
begin
  if rising_edge(clk) then
    if my_enable_signal = 1 then
      do something;
    end if;
  end if;
end process;
[/font]

[daveshah]

It's certainly not a big issue at this stage in your learning and for a project like this, but as you move on to larger projects it might be useful to start considering the use of VHDL Generics to create parameterisable modules which often turns out to be a very powerful feature.

This way, you could replace the separate hour/minute/second counters, where there is currently a lot of code duplication, with a single module that can be used three times. Maybe you could even do this to the two counters for each digit. This does tend to make maintenance, verification, etc easier.

[rstofer]

You are supposed to do arithmetic on STD_LOGIC_VECTORS by wearing out your keyboard typing 'casts'

Code: [Select]

library IEEE;
use IEEE.STD_LOGIC_1164.ALL;
use IEEE.STD_LOGIC_UNSIGNED.ALL;
use IEEE.NUMERIC_STD.ALL;
...
signal PC : std_logic_vector(15 downto 0);
...
PC <= std_logic_vector(unsigned(PC) + 1);
[/font]

You cast PC to unsigned, do the arithmetic, and cast it back to std_logic_vector.  This shouldn't generate any additional logic, there will just be an adder dedicated to the expression.

OR

PC could have been declared as unsigned(15 downto 0) but that may get in the way elsewhere.  I'm of the opinion that signals are STD_LOGIC or STD_LOGIC_VECTOR because that's what hardware looks like.  A single bit or a collection of bits (vector).

This is one of the common topics of debate.  Which libraries are REALLY in the IEEE standard and which are vendor specific and named as IEEE...  And, of course, the standards are changing from time to time.

[SiliconWizard]

@daveshah: Good advice. Use generics and cut the code duplication. It gets awful to read and maintain pretty fast, is a huge source of potential bugs and additionally will make you lose a lot of time, a lot more than the couple hours needed to learn how to use generics. Guaranteed.

I didn't inspect your code in details, but just for the record, what you're getting here are a lot of warnings. Those are not errors.
A lot of your logic will get pruned at the synthesis stage. Unless there is something fundamentally wrong with your design, this is often not a source of concern.

Additional tip: learn quickly how to write test benches and use them. Writing VHDL without using test benches is almost like taking a gun, closing your eyes and then shooting. Really. You may hit your target at first shot but it's not very likely.

Didn't follow all the answers in details either, but I have seen clock domains being mentioned.
As long as all the intermediate clocks are derived from (meaning, synchronized to) one clock only, you only have one clock domain, strictly speaking. The concrete pitfall of using a lot of different synchronized clocks in your design is that you'll end up running into limitations of your target device, which only has a limited number of internal clock trees (clock distribution). If your design has more clocks than supported by the device (FPGA here), some of your clocks will use general purpose logics instead of clock distribution structures and you might then run into timing problems. Those problems will usually get pinpointed at the final stage (place and route) and reported to you as errors (timing violations).