Sending 16 bit values on 8 bit UART

We're trying to send a 16-bit value from one PSoC to another. The essence of this operation should be pretty simple:

Split the value into two 8-bit values, one for MSB and one for LSB
Send the MSB and then LSB from system 1
Recieve MSB and then LSB on system 2
Merge the two bytes by bitshifting MSB and then OR-masking LSB.
Profit

So the way we do this is in practice is:

//Split and send
uint16 utest = (uint16)test;
uint8 hibyte = utest>>8;
uint8 lowbyte = utest;        
UART_PutChar(hibyte);
UART_PutChar(lowbyte);

System 2 (ISR on byte recieved):

//Recieve and merge
uint8 rx_msb = UART_GetByte();
uint8 rx_lsb = UART_GetByte();
rx_udata = ((uint16)rx_msb << 8) | rx_lsb;
sprintf(TransmitBufferUSB,"%d\n\r",rx_udata);
UART_USB_PutString(TransmitBufferUSB);

The problem is that this code isn't consistent. And we almost never seem to receive the same data that we send.

Another problem occurs when we try to send the data to a computer via the UART_USB_PutString function in system 2. We receive two sets of the %d\n\r in the putty terminal, one of which might be the right value sent, and another which seems rather random.

Other information

PSoC: CY8CKIT-059
IDE: Cypress PSoC Creator 4.1
Terminal: Putty
Sender code: https://pastebin.com/4SfYGuSw
Reciever code: https://pastebin.com/90c7p2zJ

Please keep in mind that this is our first time working with the UART in any setting, so we could also use tips and tricks if you have any. If you need any other information or have any ideas on how to fix this broken piece of shit please let us know.

Sincerely two noob electronics students working on PSoC

\de_rush

Solution

First of all using sprintf in the interrupt routine is a very bad idea even id you disable the interrupts. Even worse is sending the data there :) You probably are a very very beginner. Always keep the interrupt routines as short as possible.

Move your debug code outside the interrupt.

Secondly you can only read what you have received in the interrupt (which is one byte) and you read two.

Finally I do not think that the UART_GetByte was invented to be used in the interrupt routine. Just read the data register instead.

personally I prefer unions.

typedef union
{
    uint16_t u16;
    int16_t i16;
    uint8_t b[2];
}data16;

volatile data16 revcb, tb;  // tb can be local and not volatile
volatile int pointer = 0;
volatile int flag = 0;

CY_ISR(UART_ISR){
    Status_pin_Write(0xFF); //Used for debugging
    revcb.b[pointer] = dataregister;  // place the appripriate code here
    pointer = ! pointer;
    if(!pointer) flag = 1;
    Status_pin_Write(0x00);
}

//in the main function

while(1)
{
    if(flag) 
    {
        ISR_Rx_Disable();   // make it atomic
        tb = recv;
        flag = 0;
        ISR_Rx_Enable();
        sprintf(TransmitBufferUSB,"%d\n\r",tb.u16);
        UART_USB_PutString(TransmitBufferUSB);
    }
}

But remember - when you send the debug data - many other values may come and you may lost something. You need to implement a cycle buffer - but this is outside the scope of this question.