1

I am working with the Renesas RA2A1 using their Flexible software package, trying to send data over a uart.

I am sending ints and floats over the uart, so I created a union of a float and a 4 byte uint8_t array, same for ints.

I put a few of these in a struct, and then put that in a union with an array that is the size of all the data contained in the struct.

I can't get it to work by passing the array in the struct to the function.. If I create an array of uint8_t, that passes in and works OK... I'm not sure what's wrong with trying to pass the array as I am.

It is failing an assert in R_SCI_UART_WRITE that checks the size, which is failing because it is 0.

typedef union{
    float num_float;
    uint32_t num_uint32;
    int32_t num_int32;
    uint8_t num_array[4];
} comms_data_t;

typedef struct{
    comms_data_t a;
    comms_data_t b;
    comms_data_t c;
    comms_data_t d;
    comms_data_t e;
    uint8_t lr[2];
} packet_data_t;

typedef union{
    packet_data_t msg_packet_data;
    uint8_t packet_array[22];
}msg_data_t;


/* Works */
    uint8_t myData[10] = "Hi Dave!\r\n";

    uart_print_main_processor_msg(myData);

/* Doesn't work */
msg_data_t msg_data;

/* code removed that puts data into msg_data,ex below */
msg_data.msg_packet_data.a.num_float = 1.2f;

uart_print_main_processor_msg(msg_data.packet_array);

// Functions below

 /****************************************************************************************************************/
fsp_err_t uart_print_main_processor_msg(uint8_t *p_msg)
{
    fsp_err_t err   = FSP_SUCCESS;
    uint8_t msg_len = RESET_VALUE;
    uint32_t local_timeout = (DATA_LENGTH * UINT16_MAX);
    char *p_temp_ptr = (char *)p_msg;

    /* Calculate length of message received */
    msg_len = ((uint8_t)(strlen(p_temp_ptr)));

    /* Reset callback capture variable */
    g_uart_event = RESET_VALUE;

    /* Writing to terminal */
        err = R_SCI_UART_Write (&g_uartMainProcessor_ctrl, p_msg, msg_len);
        if (FSP_SUCCESS != err)
        {
            APP_ERR_PRINT ("\r\n**  R_SCI_UART_Write API Failed  **\r\n");

            return err;
        }

        /* Check for event transfer complete */
        while ((UART_EVENT_TX_COMPLETE != g_uart_event) && (--local_timeout))
        {
            /* Check if any error event occurred */
            if (UART_ERROR_EVENTS == g_uart_event)
            {
                APP_ERR_PRINT ("\r\n**  UART Error Event Received  **\r\n");
                return FSP_ERR_TRANSFER_ABORTED;
            }
        }
        if(RESET_VALUE == local_timeout)
        {
            err = FSP_ERR_TIMEOUT;
        }


    return err;

}

fsp_err_t R_SCI_UART_Write (uart_ctrl_t * const p_api_ctrl, uint8_t const * const p_src, uint32_t const bytes)
{
#if (SCI_UART_CFG_TX_ENABLE)
    sci_uart_instance_ctrl_t * p_ctrl = (sci_uart_instance_ctrl_t *) p_api_ctrl;
 #if SCI_UART_CFG_PARAM_CHECKING_ENABLE || SCI_UART_CFG_DTC_SUPPORTED
    fsp_err_t err = FSP_SUCCESS;
 #endif

 #if (SCI_UART_CFG_PARAM_CHECKING_ENABLE)
    err = r_sci_read_write_param_check(p_ctrl, p_src, bytes);
    FSP_ERROR_RETURN(FSP_SUCCESS == err, err);
    FSP_ERROR_RETURN(0U == p_ctrl->tx_src_bytes, FSP_ERR_IN_USE);
 #endif

    /* Transmit interrupts must be disabled to start with. */
    p_ctrl->p_reg->SCR &= (uint8_t) ~(SCI_SCR_TIE_MASK | SCI_SCR_TEIE_MASK);

    /* If the fifo is not used the first write will be done from this function. Subsequent writes will be done
     * from txi_isr. */
 #if SCI_UART_CFG_FIFO_SUPPORT
    if (p_ctrl->fifo_depth > 0U)
    {
        p_ctrl->tx_src_bytes = bytes;
        p_ctrl->p_tx_src     = p_src;
    }
    else
 #endif
    {
        p_ctrl->tx_src_bytes = bytes - p_ctrl->data_bytes;
        p_ctrl->p_tx_src     = p_src + p_ctrl->data_bytes;
    }

 #if SCI_UART_CFG_DTC_SUPPORTED

    /* If a transfer instance is used for transmission, reset the transfer instance to transmit the requested
     * data. */
    if ((NULL != p_ctrl->p_cfg->p_transfer_tx) && p_ctrl->tx_src_bytes)
    {
        uint32_t data_bytes    = p_ctrl->data_bytes;
        uint32_t num_transfers = p_ctrl->tx_src_bytes >> (data_bytes - 1);
        p_ctrl->tx_src_bytes = 0U;
  #if (SCI_UART_CFG_PARAM_CHECKING_ENABLE)

        /* Check that the number of transfers is within the 16-bit limit. */
        FSP_ASSERT(num_transfers <= SCI_UART_DTC_MAX_TRANSFER);
  #endif

        err = p_ctrl->p_cfg->p_transfer_tx->p_api->reset(p_ctrl->p_cfg->p_transfer_tx->p_ctrl,
                                                         (void const *) p_ctrl->p_tx_src,
                                                         NULL,
                                                         (uint16_t) num_transfers);
        FSP_ERROR_RETURN(FSP_SUCCESS == err, err);
    }
 #endif

 #if SCI_UART_CFG_FLOW_CONTROL_SUPPORT
    if ((((sci_uart_extended_cfg_t *) p_ctrl->p_cfg->p_extend)->uart_mode == UART_MODE_RS485_HD) &&
        (p_ctrl->flow_pin != SCI_UART_INVALID_16BIT_PARAM))
    {
        R_BSP_PinAccessEnable();
        R_BSP_PinWrite(p_ctrl->flow_pin, BSP_IO_LEVEL_HIGH);
        R_BSP_PinAccessDisable();
    }
 #endif

    /* Trigger a TXI interrupt. This triggers the transfer instance or a TXI interrupt if the transfer instance is
     * not used. */
    p_ctrl->p_reg->SCR |= SCI_SCR_TIE_MASK;
 #if SCI_UART_CFG_FIFO_SUPPORT
    if (p_ctrl->fifo_depth == 0U)
 #endif
    {
        /* On channels with no FIFO, the first byte is sent from this function to trigger the first TXI event.  This
         * method is used instead of setting TE and TIE at the same time as recommended in the hardware manual to avoid
         * the one frame delay that occurs when the TE bit is set. */
        if (2U == p_ctrl->data_bytes)
        {
            p_ctrl->p_reg->FTDRHL = *((uint16_t *) (p_src)) | (uint16_t) ~(SCI_UART_FIFO_DAT_MASK);
        }
        else
        {
            p_ctrl->p_reg->TDR = *(p_src);
        }
    }

    return FSP_SUCCESS;
#else
    FSP_PARAMETER_NOT_USED(p_api_ctrl);
    FSP_PARAMETER_NOT_USED(p_src);
    FSP_PARAMETER_NOT_USED(bytes);

    return FSP_ERR_UNSUPPORTED;
#endif
}



Dave S
  • 25
  • 4
  • `uint8_t myData[10] = "Hi Dave!\r\n";` is wrong because you didn't allocate enough memory to include the `'\0'`. – Fiddling Bits Jul 13 '20 at 14:51
  • I appreciate the comment.... but that's not really the point... – Dave S Jul 13 '20 at 14:57
  • 1
    Calculating the length requires `NUL` termination, right? `msg_len = ((uint8_t)(strlen(p_temp_ptr)));` `packet_array[0]` is probably `0`. – Fiddling Bits Jul 13 '20 at 15:03
  • Perhaps the first byte in the array is zero? This would matter because a zero byte is the end of a string. – user253751 Jul 13 '20 at 15:07
  • so any 0 in the array would terminate it and I'd lose the rest of the data in the array? Then how do you transmit data that is 0? I can't use sprintf or snprintf to convert it to a string because I am doing it in a freertos timer, I don't have the stack depth to convert it to a string. – Dave S Jul 13 '20 at 15:17
  • 1
    Most of this code is undefined behavior. Unions are also UB if used for aliasing, but pretty much all C compilers tend to allow it, but even then I would rather use a `char[]` for the array used for aliasing. Where is `msg_data_t msg_data;` declared? If it's a local variable, it won't be initialized to `{0}`, so there is no reason why `strlen` will work. And why do you thing `strlen` will work correctly in the general case? If you pass a floating point value which has a single zero byte in its IEEE 754 representation, it will mark the end of this "string". – vgru Jul 13 '20 at 15:17
  • @Groo Let me try to answer your questions / comments. ```msg_data_t msg_data``` is file scope. The function using strlen is not my function, but I can certainly re-write it. I'm not worried about the communication protocol part... I don't have it all shown, but I have it covered. I guess I'm still confused how the uart write function would take the struct and treat it like a uint8_t pointer to transmit it out of the buffer. – Dave S Jul 13 '20 at 15:36
  • Hmm... ok... Thanks for all of the comments.... I'll go play with it and see how it goes – Dave S Jul 13 '20 at 16:01
  • @DaveS. You can send a 0 byte. `R_SCI_UART_Write (&g_uartMainProcessor_ctrl, p_msg, msg_len);` <- msg_len is how many bytes you want it to send. But pay attention to how **your** code calculates msg_len! **Your** code cannot process a 0 byte! – user253751 Jul 14 '20 at 09:56

1 Answers1

1

There are several issues with this program. A large part of this code relies on undefined behavior. Unions are also UB if used for aliasing, even if pretty much all C compilers tend to allow it, but if you are using a union I would still prefer using a char[] for the array used for aliasing. As mentioned in the comments, "Hi Dave!\r\n"; actually takes up 11 bytes with the null-character. It's safer to use uint8_t myData[] = "Hi Dave!\r\n"; or const * uint8_t = "Hi Dave!\r\n"; and spare yourself the trouble.

Second problem is that strlen cannot work correctly for binary data. strlen works by searching for the first occurrence of the null-character in the string, so it's not applicable for binary data. If you pass a floating point value which has a single zero byte in its IEEE 754 representation, it will mark the end of this "string".

Plain and simple, your function should be declared as fsp_err_t uart_write(const char * msg, size_t msg_len); and be called using uart_write(data_array, sizeof data_array);. If you want to transmit messages of variable size over the UART, you will also have to define a certain communication protocol, i.e. create a message that can be unambiguously parsed. This will likely mean: 1) some cookie at the beginning, 2) length of the transmitted data, 3) actual data, 4) crc -- but this is outside the scope of this question.

So, strlen won't tell you the length of the data, you will pass it to the function yourself, and you don't need unions at all. If you choose not to properly serialize the data (e.g. using protobuf or some other protocol), you can simply pass the pointer to the struct to the function, i.e. call the above mentioned uart_write((char*)&some_struct, sizeof some_struct); and it will work as if you passed an array.

Note that char in this case doesn't mean "ascii character", or "character in a string". The point with using the char* is that it's the only pointer which is legally allowed to alias other pointers. So, you acquire a pointer to your struct (&str), cast it to a char*, and pass it to a function which can then read its representation in memory. I am aware that R_SCI_UART_Write is likely generated by your IDE, and unfortunately these blocks often use uint8_t* instead of char*, so you will probably have to cast to uint8_t* at some point.

vgru
  • 49,838
  • 16
  • 120
  • 201