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;
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.
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?
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.
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.
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.
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.