- Mark as New
- Bookmark
- Subscribe
- Mute
- Subscribe to RSS Feed
- Permalink
- Report Inappropriate Content
Hi all,
I'm a rank beginner, so expect beginner errors. I am using Quartus v9.1. I'm getting the famous "Error (10028): Can't resolve multiple constant drivers for net "button1" at keyfob_top.vhd(65)" I know what it is happening, but I'm unable to see _why_ it is happening. Button1 is declared as a SIGNAL, therefore there can only be ONE line of code that changes its actual value. This is why PROCEDURE set_button1() exists, it acts as a multiplexer, reducing the number of code instances that change button1 to one. The problem starts when I add a second call to set_button1(), which gives the error above. From what I can tell, the compiler sees each call to set_button1() as a unique and different piece of code, where the intention is exactly the opposite!:( How can I solve this problem? Is there another way to approach this? Can someone point me to documents that explain in detail how to deal with this problem? The complete code follows. Yours, Robert Hodkinson. P.S. The formatting of my code was thrown out of the window by the forums, so how to people place their code into postings and keep the formatting? (edit: this has been sorted out now).:)
library ieee;
use ieee.std_logic_1164.all;
entity keyfob_top is
port
(
pin_clk: in std_logic; -- our input clock
pin_button1: in std_logic; -- user button, fires a signal1
pin_led_output: buffer std_logic; -- user sees that a button has been pushed
pin_ir_output: buffer std_logic -- IR output signal
);
end keyfob_top;
architecture led_behaviour of keyfob_top is
constant led_off : std_logic :='0'; -- led boolean off value
constant led_on : std_logic :='1'; -- led boolean on value
constant button_up: std_logic :='0'; -- button has not been pressed
constant button_down: std_logic :='1'; -- button has been pressed
constant baud_rate : integer := 5; -- delay number
signal baud_tick : std_logic := '0'; -- baud rate delay
signal button1 : std_logic := '0'; -- internal button1
signal button1_bits_left : integer; -- bits left to transmit for button1 press
-- apply the correct value to button1 (which is a signal)
procedure set_button1(constant status : std_logic) is
variable tmp : std_logic;
begin
if status = button_up then
tmp := button_up; -- has just been pressed
else
tmp := button_down; -- has just been released
end if;
button1 <= tmp;
end procedure set_button1;
begin
-- create our baud rate
-- runs permanently in the background generating our baud rate.
baud_clock : process(pin_clk, baud_tick)
variable count : integer range 0 to 99;
begin
if count > baud_rate then
count := 0; -- reset our counter
if baud_tick = '1' then
baud_tick <= '0'; -- toggle our indicator
else
baud_tick <= '1'; -- toggle our indicator
end if;
else
count := count + 1; -- incrament the counter
end if;
end process baud_clock;
-- triggered when pin_button1 has been pressed
trigger_button1: process
begin
wait until pin_button1 = '1'; -- wait until pin_button1 is pressed
set_button1(button_down); -- pin_button1 has just been pressed.
button1_bits_left <= 8;
end process trigger_button1;
-- do we have anything to send out
data_out: process
begin
wait until baud_tick = '1'; -- line 65 -- wait on rising edge
-- does button1 need to be serviced
if button1 = button_down then
button1_bits_left <= button1_bits_left - 1; -- decrament our counter
case button1_bits_left is -- choose which bit to send out
when 7 => pin_ir_output <= led_off;
pin_led_output <= led_on; -- show user what is going on
when 6 => pin_ir_output <= led_on;
when 5 => pin_ir_output <= led_off;
when 4 => pin_ir_output <= led_off;
when 3 => pin_ir_output <= led_on;
when 2 => pin_ir_output <= led_off;
when 1 => pin_ir_output <= led_on;
when 0 => pin_ir_output <= led_off;
set_button1(button_up); -- reset our button
pin_led_output <= led_off; -- show user what is going on
when others => null;
end case;
end if;
-- list other button presses here.
end process data_out;
end led_behaviour;
Link Copied
- Mark as New
- Bookmark
- Subscribe
- Mute
- Subscribe to RSS Feed
- Permalink
- Report Inappropriate Content
You should think of a procedure (or function) to be executed 'in-line' i.e. inside the process where you make the call to it. So you end up with two processes assigning a value to the signal 'button1', hence the compiler's complaint. The same applies to signal ' button1_bits_left'.
To solve this in your code you have to change the approach to handle every assignment to signal 'button1' in a single process. The simplest approach is to handle everything in the clocked process.- Mark as New
- Bookmark
- Subscribe
- Mute
- Subscribe to RSS Feed
- Permalink
- Report Inappropriate Content
The easy bit first, to keep your code format insert it between the two gates:
[%code] your code [%/code]} without the percentage sign. So format your code and we will take risk. For now I believe your procedure wouldn't work. Button1 is not visible to a procedure, or is it?- Mark as New
- Bookmark
- Subscribe
- Mute
- Subscribe to RSS Feed
- Permalink
- Report Inappropriate Content
let me go through the code, bit by bit. You have other problems other than what you're asking about:
port
(
pin_clk: in std_logic; -- our input clock
pin_button1: in std_logic; -- user button, fires a signal1
pin_led_output: buffer std_logic; -- user sees that a button has been pushed
pin_ir_output: buffer std_logic -- IR output signal
);
end keyfob_top;
Buffers are not the prefered option of doing things in VHDL now, most people prefer to use an internal signal and assign that to an output. But, I leave it up to you.
-- apply the correct value to button1 (which is a signal)
procedure set_button1(constant status : std_logic) is
variable tmp : std_logic;
begin
if status = button_up then
tmp := button_up; -- has just been pressed
else
tmp := button_down; -- has just been released
end if;
button1 <= tmp;
end procedure set_button1;
Two things. 1. why dont you do this instead? On real hardware, "status" can only be '0' or '1'. button1 <= status; 2. Its generally not a good idea to assign a signal thats not in the scope of the procedure. You can do it, but I suspect you'll be getting yourself into trouble, like you already have. Usually, its better to create a signal output on the procedure and connect the signal to that.
procedure set_button1(constant status : std_logic; signal output : out std_logic) is
begin
output <= status;
end procedure set_button1;
.... then call it:
set_button1(status => input, output => button1);
-- triggered when pin_button1 has been pressed
trigger_button1: process
begin
wait until pin_button1 = '1'; -- wait until pin_button1 is pressed
set_button1(button_down); -- pin_button1 has just been pressed.
button1_bits_left <= 8;
end process trigger_button1;
Again, using waits is not the prefered method of clocking logic. You are also using logic as a clock, which is highly inadvisable.
-- do we have anything to send out
data_out: process
begin
wait until baud_tick = '1'; -- line 65 -- wait on rising edge
-- does button1 need to be serviced
if button1 = button_down then
....
set_button1(button_up); -- reset our button
pin_led_output <= led_off; -- show user what is going on
......
end process data_out;
There is your problem. You already assigned button1 in the "trigger_button1" process. Signals can only be assigned from a single process, otherwise you get multple driver error like you did. Its because "trigger_button1" is always driving "button_down" onto the signal, while "data_out" always drives "button_up". So they are in conflict.
- Mark as New
- Bookmark
- Subscribe
- Mute
- Subscribe to RSS Feed
- Permalink
- Report Inappropriate Content
josyb:
What you say makes a great deal of sense. What I am struggling with is the how to structure all the code dealing with assigning values to a signal in a process. I'm coming from a 'C' background, with all of the associated language. I can see I'm heading in the right direction, but I'm not there yet! The real problem I'm running into is how to pass information between processes. At the moment the only thing i can see that can perform this are things defined as SIGNAL. I need to go back and rethink the functionality distribution of the code. kaz: Formatting of my code is now been sorted, thanks. tricky: I'm not surprised I have lots of problems in my code. Buffers: I will return to using SIGNALS as opposed to BUFFERS. I used BUFFERS to solve problems, but not in the correct way. 1. button1<=status; duh! obvious! 2. I don't understand the sentence starting "Usually, its better to create...." I get the feeling that it is something fairly fundamental. waits: OK, so as a rule, try and use sensitivity lists, as opposed to waits. I'm learning that clocking of logic is not a good idea. Infact, it sounds like using any derived clocks is a poor idea. Part of my code is generating a baud rate, which is a form of a clock. Is there a preferred/recommended way of doing this? I have yet to find examples of a baud rate generators (and how to use them correctly). There is your problem: I'm not quite sure I understand this. The reason I created the set_button1() procedure was to ensure that there was only line of code that alters the value of button1. But it seems I've not solved the problem. It sounds like the compiler is treating a PROCEDURE or a FUNCTION as a method of inserting code (macro like), as opposed to calling a generic procedure (I think I've just let out my 'C' like thinking here). This bring me back to a comment I made answering josyb response: how to I pass information/triggers between processes that do not violate SIGNAL rules? Either I've not understood what I've been reading, or I've not read the relevant sections. thanks for all your responses, please keep them coming! Robert Hodkinson.- Mark as New
- Bookmark
- Subscribe
- Mute
- Subscribe to RSS Feed
- Permalink
- Report Inappropriate Content
The biggest problem is that you are treating VHDL like a programming language. VHDL stands for VHSIC (very high speed integrated circuit) Hardware description Language. You use the language to describe hardware. Unless you get this fundamental understood, any behavioural VHDL is going to fall to peices when you try and synthesise it.
So the best thing for you to do might be start over. First think about/draw out the logic circuit that you're trying to achieve. When you know what gates/registers you need, then write it up in VHDL. Clocks are an essential part of FPGA logic, but not derived clocks - system clocks. You can then use something like baud_tick as a register enable. This way the registers are always clocked with the same stable low skew clock, but the enable only lets data through at the correct time. Back to you not understanding my sentence: Procedures are almost like mini entities. Entries in a procedure can be 3 types: constant, variable or signal. Each entry can be of mode in, out or inout (like a entity). In signals default to constant, out or inout default to variable. So look at the following code:
procedure do_something( a : in integer; --This is a constant.
variable b : out integer;
c : inout integer;; --defaults to variable
signal d : inout integer;
signal e : out integer
) is
begin
b := a + 10;
c := c + a;
d <= c + a; --as c is a variable, d = c + a + a.
e <= d + a; --as d is a signal, e uses old value of d before the update above.
end procedure;
------------------------
--Call the procedure
------------------------
do_something( a => some_signal, --reads the value of "some_signal" when the procedure is called, internally treated as a constant
b => op_var, --op_var cannot be read inside the procedure, only written
c => some_var, --procedure can read "some_var" and write back to it
d => output --like b above, but with a signal
);
Generally, procedures are a good way of tidying up repeated code that assigns the same signals over and over again. Functions are great for tidying away complex assignemnets to a single signal/variable/constant.
- Mark as New
- Bookmark
- Subscribe
- Mute
- Subscribe to RSS Feed
- Permalink
- Report Inappropriate Content
Hi Tricky,
I realise i'm still thinking as a classic programmer, it will take time to separate myself from this. I have started again from scratch (3rd time), looking at is from a register point of view. The program compiles, but has thrown up quite a lot of warnings that need looking at. thanks. robert hodkinson.- Subscribe to RSS Feed
- Mark Topic as New
- Mark Topic as Read
- Float this Topic for Current User
- Bookmark
- Subscribe
- Printer Friendly Page