Forum Discussion

Altera_Forum's avatar
Altera_Forum
Icon for Honored Contributor rankHonored Contributor
15 years ago

VHDL Error (10028): Can't resolve multiple constant drivers for net...

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;

6 Replies

  • Altera_Forum's avatar
    Altera_Forum
    Icon for Honored Contributor rankHonored Contributor

    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.
  • Altera_Forum's avatar
    Altera_Forum
    Icon for Honored Contributor rankHonored Contributor

    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?
  • Altera_Forum's avatar
    Altera_Forum
    Icon for Honored Contributor rankHonored Contributor

    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.
  • Altera_Forum's avatar
    Altera_Forum
    Icon for Honored Contributor rankHonored Contributor

    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.
  • Altera_Forum's avatar
    Altera_Forum
    Icon for Honored Contributor rankHonored Contributor

    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.
  • Altera_Forum's avatar
    Altera_Forum
    Icon for Honored Contributor rankHonored Contributor

    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.