Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

UART Class write Method is Blocking and Makes no Use of the SafeRingBuffer #307

Open
delta-G opened this issue May 6, 2024 · 1 comment · May be fixed by #304
Open

UART Class write Method is Blocking and Makes no Use of the SafeRingBuffer #307

delta-G opened this issue May 6, 2024 · 1 comment · May be fixed by #304
Labels
topic: code Related to content of the project itself type: enhancement Proposed improvement

Comments

@delta-G
Copy link
Contributor

delta-G commented May 6, 2024

The UART class write method currently blocks waiting on a flag that will not be set until the transmission is complete. For low baud rates this can be a long time. There is currently a SafeRingBuffer instance allocated in the class, but no attempt has been made to make use of it. The code as written now submits a pointer to the HAL and then hangs until the transmission ends.

/* -------------------------------------------------------------------------- */
size_t UART::write(uint8_t c) {
/* -------------------------------------------------------------------------- */  
  if(init_ok) {
    tx_done = false;
    R_SCI_UART_Write(&uart_ctrl, &c, 1);
    while (!tx_done) {}
    return 1;
  }
  else {
    return 0;
  }
}

size_t  UART::write(uint8_t* c, size_t len) {
  if(init_ok) {
    tx_done = false;
    R_SCI_UART_Write(&uart_ctrl, c, len);
    while (!tx_done) {}
    return len;
  }
  else {
    return 0;
  }
}

The flag tx_done is only set in the interrupt handler after transmission ends.

/* -------------------------------------------------------------------------- */
void UART::WrapperCallback(uart_callback_args_t *p_args) {
/* -------------------------------------------------------------------------- */  

  uint32_t channel = p_args->channel;
  
  UART *uart_ptr = UART::g_uarts[channel];

  if(uart_ptr == nullptr) {
    return;
  }
  

  switch (p_args->event){
      case UART_EVENT_ERR_PARITY:
      case UART_EVENT_ERR_FRAMING:
      case UART_EVENT_ERR_OVERFLOW:
      case UART_EVENT_RX_COMPLETE: // This is called when all the "expected" data are received
      {
          break;
      }
      case UART_EVENT_TX_COMPLETE:
      case UART_EVENT_TX_DATA_EMPTY:
      {
        //uint8_t to_enqueue = uart_ptr->txBuffer.available() < uart_ptr->uart_ctrl.fifo_depth ? uart_ptr->txBuffer.available() : uart_ptr->uart_ctrl.fifo_depth;
        //while (to_enqueue) {
        uart_ptr->tx_done = true;
        break;
      }
      case UART_EVENT_RX_CHAR:
      {
        if (uart_ptr->rxBuffer.availableForStore()) {
          uart_ptr->rxBuffer.store_char(p_args->data);
        }
        break;
      }
      case UART_EVENT_BREAK_DETECT:
      {
          break;
      }
  }

}

Additionally, by not making use of the buffer, a pointer to the users data is passed into the HAL. This means that any changes made to the data behind that pointer while it's still being sent will corrupt the output. Using the txBuffer will solve that problem as well.

@delta-G
Copy link
Contributor Author

delta-G commented May 6, 2024

I submitted PR #304 for this issue.

@per1234 per1234 linked a pull request May 9, 2024 that will close this issue
@per1234 per1234 added type: enhancement Proposed improvement topic: code Related to content of the project itself labels May 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: code Related to content of the project itself type: enhancement Proposed improvement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants