Comments in code are very useful. But not as good as executable comments…
I write image processing code at work. One of my FPGAs has a piece of code which generates a signal which has a hard-coded number of clock cycles that it is low for. This is fine in the application – it never needs to change, it just has to match what the camera does, and the software programs that up the same every time.
So, in the (detailed) comments for this module, I made a note that this was the case. However, recently, I needed to change the value that the software sends to the camera to give a bit more time for the processing. So I changed my VHDL tests so that the represented the new value the camera would be using, and ran my regression suite. No problem, all works fine.
We pushed the code into the FPGA and tried it on the bench. All works fine except that this particular signal doesn’t match the camera any more. And my testbenches don’t model the chip at the other end of the link in that level of detail. What I should have done, as well as writing the comment was add some code to check that it was being obeyed.
If I assert something must be true in the comments (ie This signal should match this other signal timing) then I should add some code to tell me off if I make it untrue! The word assert is key – use a VHDL assertion to make sure that the two signals match:
process (clk, camera_sig, mysig) is
variable camera_low, mysig_low:natural:=0;
begin — process
if falling_edge(mysig) then
assert camera_low = mysig_low
report "Camera sig timing doesn’t match mysig timing"
severity error;
end if;
if rising_edge(camera_sig) then
camera_low := 0;
end if;
if rising_edge(mysig) then
mysig_low := 0;
end if;
if rising_edge(clk) then
if camera_sig = ’0′ then
camera_low := camera_low + 1;
end if;
if mysig_low = ’0′ then
mysig_low := mysig_low + 1;
end if;
end if;
end process;
The key assert is at the top of that process. The rest simply counts clock ticks while the relevant signal is low. You could also do it without the clocks and capture the time at the start and the end of the pulses to compare…
And that’s an executable comment!
Hi,
is your vhdl code correct? where is mysig coming from? It is not in the sensitivity list, even though it is tested asynchronously… while camera_sig is in the sensitivity list, even though it is tested at the edge of the clock…
Heh, that’s what I get for rewriting code on the fly, rather than copy and pasting it :) Well spotted – hopefully it looks better now! I also spotted that I was testing for a `rising_edge` on one of the integer variables – note to self – try compiling code before submittng it!
For the record (I’ve made it right in the posting), it used to look like this:
[vhdl]
process (clk, camera_sig) is
variable camera_low, mysig_low:natural:=0;
begin — process
if falling_edge(mysig) then
assert camera_low = mysig_low
report “Camera sig timing doesn’t match mysig timing”
severity error;
end if;
if rising_edge(camera_low) then
camera_low := 0;
end if;
if rising_edge(mysig) then
mysig_low := 0;
end if;
if rising_edge(clk) then
if camera_sig = ‘0’ then
camera_low := camera_low + 1;
end if;
if mysig_low = ‘0’ then
mysig_low := mysig_low + 1;
end if;
end if;
end process;
[/vhdl]
There are signals being used in the rising_edge line such as “if rising_edge(camera_low)†instead of using the main clock. I always leaned that it is bad practice to use signals like this and instead should use your main clock clk. Can you comment?
Jeff, you’re quite right to use that rule for synthesisable code – however, that example is from some testbench code. When not contrained by the abilities of my synthesiser and/or the target hardware, I make full use of the abilities of VHDL to express what I need to accomplish. Including using the `real` type, timeouts, many varied edges, time delays etc…