So i have this receiver code for the RS232 communication link, I'm supposed to send 8 bits with 1 start bit "0" and one stop bit "1", no parity check bit, I have tried with those code in most kinds of ways but the simulation never worked correctly, even though some people told me my problem is the testbench not the code but it never works on the FPGA implementation, the first signal i sent is always wrong where as any signal after that is correct.
here is the code below
entity Rs232Rxd is
port( Reset, Clock16x, Rxd: in std_logic;
DataOut1: out std_logic_vector (7 downto 0));
end Rs232Rxd;
architecture Rs232Rxd_Arch of Rs232Rxd is
attribute enum_encoding: string;
-- state definitions
type stateType is (stIdle, stData, stStop, stRxdCompleted);
attribute enum_encoding of statetype: type is "00 01 11 10";
signal iReset : std_logic;
signal iRxd1, iRxd2 : std_logic := '1';
signal presState: stateType;
signal nextState: stateType;
signal iClock1xEnable, iClock1x, iEnableDataOut: std_logic :='0' ;
signal iClockDiv: std_logic_vector (3 downto 0) := (others=>'0') ;
signal iDataOut1, iShiftRegister: std_logic_vector (7 downto 0):= (others=>'0');
signal iNoBitsReceived: std_logic_vector (3 downto 0):= (others=>'0') ;
begin
process (Clock16x) begin
if rising_edge(Clock16x) then
if Reset = '1' or iReset = '1' then
iRxd1 <= '1';
iRxd2 <= '1';
iClock1xEnable <= '0';
iClockDiv <= (others=>'0');
else
iRxd1 <= Rxd;
iRxd2 <= iRxd1;
end if;
if iRxd1 = '0' and iRxd2 = '1' then
iClock1xEnable <= '1';
end if;
if iClock1xEnable = '1' then
iClockDiv <= iClockDiv + '1';
end if;
end if;
end process;
iClock1x <= iClockDiv(3);
process (iClock1xEnable, iClock1x)
begin
if iClock1xEnable = '0' then
iNoBitsReceived <= (others=>'0');
presState <= stIdle;
elsif rising_edge(iClock1x) then
iNoBitsReceived <= iNoBitsReceived + '1';
presState <= nextState;
if iEnableDataOut = '1' then
iDataOut1 <= iShiftRegister;
--iShiftRegister <= (others=>'0');
else
iShiftRegister <= Rxd & iShiftRegister(7 downto 1);
end if;
end if;
end process;
DataOut1 <= iDataOut1;
process (presState, iClock1xEnable, iNoBitsReceived)
begin
-- signal defaults
iReset <= '0';
iEnableDataOut <= '0';
case presState is
when stIdle =>
if iClock1xEnable = '1' then
nextState <= stData;
else
nextState <= stIdle;
end if;
when stData =>
if iNoBitsReceived = "1000" then
iEnableDataOut <= '1';
nextState <= stStop;
else
iEnableDataOut <= '0';
nextState <= stData;
end if;
when stStop =>
nextState <= stRxdCompleted;
when stRxdCompleted =>
iReset <= '1';
nextState <= stIdle;
end case;
end process;
end Rs232Rxd_Arch;
Your question doesn't present a Minimal Complete and Verifiable Example. The problem cannot be duplicated without writing a testbench and your problem lacks specificity ('signal' and 'wrong' used here are imprecise).
There are some observations.
A stop bit followed by a start bit of a successive character leaves no room for state stRxdCompleted. Also iNoBitsReceived is not set to all '0's when iClock1xEnable goes invalid, meaning the sampling point is not determined by the falling edge of the start bit for successive characters:
This is a capital 'A' immediately followed by a lower case 'a', the stop bit immediately followed by the start bit of the second character (which is legal).
In the first character you see that the start bit is counted as one of the character bits.
You also see that the bit counter isn't reset when the enable goes invalid, which will cause sampling point drift (and may eventually cause sampling errors depending on clock differences or transmission distortion and lack of isosynchronous sampling point reset).
You also see presState is stStop during the last data bit of the first character and yet the second character is correct. Looking a bit closer we see the start bit of the first character occurs during stData and that doesn't occur for the second character.
There's a basic issue with number of states and state transitions when iClock1x is stopped.
You don't need the state machine, you have the counter named iNoBitsReceived that can store all the state should ishiftregister be long enough to accommodate start (and possibly stop) bits should you also detect framing errors.
Tying operations to specific counts without a separate state machine, along with clearing the bit counter when idle:
Gives us something that works with a bit less complexity:
library ieee;
use ieee.std_logic_1164.all;
use ieee.numeric_std.all;
entity Rs232Rxd is
port (
Reset,
Clock16x,
Rxd: in std_logic;
DataOut1: out std_logic_vector (7 downto 0)
);
end entity Rs232Rxd;
architecture foo of Rs232Rxd is
signal rxd1: std_logic;
signal rxd2: std_logic;
signal baudctr: unsigned (3 downto 0);
signal ctr16x: unsigned (3 downto 0);
signal enab1xstart: std_logic;
signal enable1x: std_logic;
signal ninthbit: std_logic;
signal sampleenab: std_logic;
signal shiftregister: std_logic_vector(7 downto 0);
begin
CLOCK_DOMAIN:
process (clock16x)
begin
if rising_edge(clock16x) then
rxd1 <= rxd;
rxd2 <= rxd1;
end if;
end process;
enab1xstart <= not rxd1 and rxd2 and not enable1x;
ENABLE_1X:
process (clock16x, reset)
begin
if reset = '1' then
enable1x <= '0';
elsif rising_edge(clock16x) then
if enab1xstart = '1' then
enable1x <= '1';
elsif ninthbit = '1' then
enable1x <= '0';
end if;
end if;
end process;
SAMPLE_COUNTER:
process (clock16x, reset, ninthbit)
begin
if reset = '1' or ninthbit = '1' then
ctr16x <= (others => '0'); -- for simulation
elsif rising_edge(clock16x) then
if enab1xstart = '1' or enable1x = '1' then
ctr16x <= ctr16x + 1;
end if;
end if;
end process;
sampleenab <= not ctr16x(3) and ctr16x(2) and ctr16x(1) and ctr16x(0);
BAUD_COUNTER:
process (clock16x, reset)
begin
if reset = '1' then
baudctr <= (others => '0');
elsif rising_edge(clock16x) and sampleenab = '1' then
if baudctr = 8 then
baudctr <= (others => '0');
else
baudctr <= baudctr + 1;
end if;
end if;
end process;
NINTH_BIT: -- one clock16x period long, after baudctr changes
process (clock16x, reset)
begin
if reset = '1' then
ninthbit <= '0';
elsif rising_edge(clock16x) then
ninthbit <= sampleenab and baudctr(3) and not baudctr(2) and
not baudctr(1) and not baudctr(0);
end if;
end process;
SHIFT_REG:
process (clock16x, reset)
begin
if reset = '1' then
shiftregister <= (others => '0'); -- for pretty waveforms
elsif rising_edge(clock16x) and sampleenab = '1' then
shiftregister <= rxd2 & shiftregister(7 downto 1);
end if;
end process;
OUTREG:
process (clock16x, reset)
begin
if reset = '1' then
dataout1 <= (others => '0');
elsif rising_edge(clock16x) and ninthbit = '1' then
dataout1 <= shiftregister;
end if;
end process;
end architecture;
VHDL basic identifiers are case insensitive, and the names weren't particularly enlightening. The format of the two above waveforms indicates name changes handily.
If you extend the shift register length by one or two you can detect framing errors during the stop bit. Changing the shift register length would require slicing the shift register output for writing to your data output.
Note this architecture is written to use package numeric_std and not Synopsys package std_logic_arith. You also didn't provide the context clause preceding the entity declaration.
This architecture also produces enables and uses the 16x clock instead of producing a 1x clock.
It was written after finding the amount of changes to correct issues in the original architecture seemed overwhelming. (When in doubt, start over.)
This testbench was used:
library ieee;
use ieee.std_logic_1164.all;
entity rs232rxd_tb is
end entity;
architecture foo of rs232rxd_tb is
signal reset: std_logic := '0';
signal clock16x: std_logic := '0';
signal rxd: std_logic := '1';
signal dataout1: std_logic_vector (7 downto 0);
begin
DUT:
entity work.rs232rxd
port map (
reset => reset,
clock16x => clock16x,
rxd => rxd,
dataout1 => dataout1
);
CLOCK:
process
begin
wait for 3.255 us; -- 16X clock divided by 2, 9600 baud 104.16 us
clock16x <= not clock16x;
if now > 2.30 ms then
wait;
end if;
end process;
STIMULI:
process
begin
wait for 6.51 us;
reset <= '1';
wait for 13.02 us;
reset <= '0';
wait for 13.02 us;
wait for 40 us;
rxd <= '0';
wait for 104.16 us; -- start bit
rxd <= '1';
wait for 104.16 us; -- first data bit, bit 0 = '1'
rxd <= '0';
wait for 104.16 us; -- second data bit, bit 1 = '0'
rxd <= '0';
wait for 104.16 us; -- third data bit, bit 2 = '0';
wait for 104.16 us; -- fourth data bit, bit 3 = '0';
wait for 104.16 us; -- fifth data bit, bit 4 = '0';
wait for 104.16 us; -- sixth data bit, bit 5 = '0';
rxd <= '1';
wait for 104.16 us; -- seventh data bit, bit 6 = '1';
rxd <= '0';
wait for 104.16 us; -- eigth data bit, bit 7 = '0';
rxd <= '1';
wait for 104.16 us; -- stop bit ( = '1')
--wait for 104.16 us; -- idle
rxd <= '0';
wait for 104.16 us; -- start bit
rxd <= '1';
wait for 104.16 us; -- first data bit, bit 0 = '1'
rxd <= '0';
wait for 104.16 us; -- second data bit, bit 1 = '0'
rxd <= '0';
wait for 104.16 us; -- third data bit, bit 2 = '0';
wait for 104.16 us; -- fourth data bit, bit 3 = '0';
wait for 104.16 us; -- fifth data bit, bit 4 = '0';
rxd <= '1';
wait for 104.16 us; -- sixth data bit, bit 5 = '1';
wait for 104.16 us; -- seventh data bit, bit 6 = '1';
rxd <= '0';
wait for 104.16 us; -- eigth data bit, bit 7 = '0';
rxd <= '1';
wait for 104.16 us; -- stop bit ( = '1')
wait;
end process;
end architecture;
You can see that new architecture has all the same essential elements, although clocked process elements are in separate process statements.
There is no state machine process.
The architecture is extensible to a full feature UART receiver by playing around with separating inputs to the shift register (for parity, two stop bits, 7 data bits, etc.). Parity can be performed serially.