How to modify safely component library functions?

Altera_Forum · ‎05-03-2006

I have realized I need a special behaviour of my UART read/write functions.

What I need is to modify uart access in blocking mode to add a timeout for the blocking 'read'. I do not mind write function blocking task indifinitely (I do not use handshaking so there is no rick for a deadloop).

I have found the C module called altera_avalon_uart.c in the \altera\kits\nios2_v5\components\altera_avalon_uart\HAL\src folder where all this action take place. I see they block on a FLAG signal idifinitelly - I would like to modify this code to block on a variable timeout. Preferably timeout should be set for each read function access.

My question is:

HOW do I modify standard component library functions to make them become a part of my project and overwrite the standard library ? How can I deal with passing this extra timeout argument to the read function, but only if read applies to serial communication and not to the other file system accesses?

Probably adding some members like this timeout to the alt_fd struct would do the trick - is there a way I can get access to this object from my task code ?

Anybody here with an experience in modifying standard component library ?

Altera_Forum · ‎05-03-2006

OK, not knowing a good way to modify library I have decided to work around the issue using original code and make all changes in my own functions.

Here is what I have done:

1. I used OSTaskDel() to delay my task for one tick in the event there was nothing in the circular buffer. Then I read again and compared number of ticks elapsed with the timeout. If I could wait longer I did, if not - I returned with timeout. Here is the code for the version 1

INT32S ReadUARTwithTimeout(struct _reent * r, INT32S uart, const char* uart_name, INT8U * ch, INT8U ms_timeout)
{
  INT32S result, need_to_close_uart=1;
  INT8U ticks = OS_TICKS_PER_SEC * ((INT32U)ms_timeout+500L/OS_TICKS_PER_SEC) / 1000;
  if(uart == 0)
    uart = _open_r(r, uart_name, O_RDONLY | O_NONBLOCK | O_NOCTTY, 0); 
  else
    need_to_close_uart = 0;
  
  if(uart == 0)
    {
    need_to_close_uart = 0;
    printf("Unable to open UART in the ReadUARTwithTimeout(%s)\n", uart_name);
    }
  else
    {
    INT32U start_time, current_time;
    start_time = OSTimeGet();
    while ((result = _read_r(r, uart, ch, 1)) == 0)
      {
      OSTimeDly(1); // delay task one tick
      current_time = OSTimeGet();
      // current_time might be smaller than start_time
      // when the tick counter overlaps, but we will
      // still get correct value working with INT32U types.
      if(current_time-start_time >= ticks)
        break;
      }
    }
  if(need_to_close_uart)
    _close_r(r, uart);
  return result;
}

This version of the function worked fine, but I did not like quite long and undetermined delay caused with lack of synchronisaton between uart receiver and the system timer interrupt. With a system clock in a range of 10ms the time was too much unpredictable. I could shorten the system clock interval but I felt it is not the way to go anyway... So I wrote 2nd version.

2. Second version is written under the inspiration of reading altera driver code. I tried to use the same system flags the original code in blocking mode uses to wait for received character. Instead waiting on flag indifinitelly, like the original code does, I added a timeout in my code. Here is the modified function:

 # include "altera_avalon_uart.h"# define ALT_UART_READ_RDY  0x1
extern alt_fd alt_fd_list;
INT32S ReadUARTwithTimeout(struct _reent * r, INT32S uart, const char* uart_name, INT8U * ch, INT8U ms_timeout)
{
  INT32S result, need_to_close_uart=1;
  INT8U ticks = OS_TICKS_PER_SEC * ((INT32U)ms_timeout+500L/OS_TICKS_PER_SEC) / 1000;
  if(uart == 0)
    uart = _open_r(r, uart_name, O_RDONLY | O_NONBLOCK | O_NOCTTY, 0); 
  else
    need_to_close_uart = 0;
  
  if(uart == 0)
    {
    need_to_close_uart = 0;
    printf("Unable to open UART in the ReadUARTwithTimeout(%s)\n", uart_name);
    }
  else
    {
    // Get access to UART code internal guts... :-)
    alt_fd* fd = &alt_fd_list;
    alt_avalon_uart_dev* dev = (alt_avalon_uart_dev*) fd->dev;
    result = _read_r(r, uart, ch, 1);
    if(result == 0)
      { // we need to wait on UART_READ_RDY flag for character
      result = ALT_FLAG_PEND (dev->events, 
                      ALT_UART_READ_RDY,
                      OS_FLAG_WAIT_SET_ANY + OS_FLAG_CONSUME,
                      ticks);
      if(result == OS_NO_ERR)
        {
        // The desired bits have been set within the specified &#39;timeout&#39;
        result = _read_r(r, uart, ch, 1);
        }
      else     // The bit(s) have not been set in the specified &#39;timeout&#39;
        result = 0;
      }
    }
  if(need_to_close_uart)
    _close_r(r, uart);
  return result;
}

The function worked great and I was amazed how predictable code can be compared to the 1st version... No uncertainity from the OSTaskDel()!

Unfortunatelly - there is a problem... From time to time I get an unexpected error. I debugged the code and I have found that the ALT_UART_READ_RDY system flag is set prematurely or without the reason at all! I am getting OS_NO_ERR result from ALT_FLAG_PEND indicating the flag was set without timeout but consecutive read() returns 0 like there was no character waiting! I have checked errno value after read() returned and it was not even EWOULDBLOCK. I am puzzled, but inspection of the altera RX IRQ handler revealed the reason for the false wake up call from the OS. It would be in the case of receiving a characeter with a parity or frame error.

Here is a fragment of altera_avalon_uart.c code showing RX IRQ handler:

static void alt_avalon_uart_rxirq (alt_avalon_uart_dev* dev,
                                   alt_u32              status)
{
  alt_u32 next;
  /*
   * In a multi-threaded environment, set the read event flag to indicate
   * that there is data ready. This is only done if the circular buffer was
   * previously empty.
   */
  if (dev->rx_end == dev->rx_start)
  {
    ALT_FLAG_POST (dev->events, ALT_UART_READ_RDY, OS_FLAG_SET);
  }
  /* Determine which slot to use next in the circular buffer */
  next = (dev->rx_end + 1) & ALT_AVALON_UART_BUF_MSK;
  /* Transfer data from the device to the circular buffer */
  dev->rx_buf = IORD_ALTERA_AVALON_UART_RXDATA(dev->base);
  /* If there was an error, discard the data */
  if (status & (ALTERA_AVALON_UART_STATUS_PE_MSK | 
                  ALTERA_AVALON_UART_STATUS_FE_MSK))
  {
    return;
  }
  dev->rx_end = next;
  next = (dev->rx_end + 1) & ALT_AVALON_UART_BUF_MSK;
  /*
   * If the cicular buffer was full, disable interrupts. Interrupts will be
   * re-enabled when data is removed from the buffer.
   */
  if (next == dev->rx_start)
  {
    dev->ctrl &= ~ALTERA_AVALON_UART_CONTROL_RRDY_MSK;
    IOWR_ALTERA_AVALON_UART_CONTROL(dev->base, dev->ctrl); 
  }   
}

As you can see, the task is awaken with a call to ALT_FLAG_POST before the received character is even checked for error and potentially discarded. I do not reject the posibility of receiving errors on my board but I tried to set the breakpoint in the RX IRW handler on the line with return; after detecting the parity/frame error but debugger never stopped there...

3. Because of this mistery, I have decided to hack my code in a way to detect a false wake up call from the OS and give it another chance http://forum.niosforum.com/work2/style_emoticons/<#EMO_DIR#>/smile.gif

Here is "improved" code:

#include "altera_avalon_uart.h"# define ALT_UART_READ_RDY  0x1
extern alt_fd alt_fd_list;
INT32S ReadUARTwithTimeout(struct _reent * r, INT32S uart, const char* uart_name, INT8U * ch, INT8U ms_timeout)
{
  INT32S result, need_to_close_uart=1;
  INT8U ticks = OS_TICKS_PER_SEC * ((INT32U)ms_timeout+500L/OS_TICKS_PER_SEC) / 1000;
  if(uart == 0)
    uart = _open_r(r, uart_name, O_RDONLY | O_NONBLOCK | O_NOCTTY, 0); 
  else
    need_to_close_uart = 0;
  
  if(uart == 0)
    {
    need_to_close_uart = 0;
    printf("Unable to open UART in the ReadUARTwithTimeout(%s)\n", uart_name);
    }
  else
    {
    // Get access to UART code internal guts... :-)
    alt_fd* fd = &alt_fd_list;
    alt_avalon_uart_dev* dev = (alt_avalon_uart_dev*) fd->dev;
    result = _read_r(r, uart, ch, 1);
    if(result == 0)
      { // we need to wait on UART_READ_RDY flag for character
      result = ALT_FLAG_PEND (dev->events, 
                      ALT_UART_READ_RDY,
                      OS_FLAG_WAIT_SET_ANY + OS_FLAG_CONSUME,
                      ticks);
      if(result == OS_NO_ERR)
        {
        // The desired bits have been set within the specified &#39;timeout&#39;
        result = _read_r(r, uart, ch, 1);
        if(result == 0)
          { // False wake up call from the OS!!! Wait again.
          result = ALT_FLAG_PEND (dev->events, 
                          ALT_UART_READ_RDY,
                          OS_FLAG_WAIT_SET_ANY + OS_FLAG_CONSUME,
                          ticks);
          if(result == OS_NO_ERR)
            // The desired bits have been set within the specified &#39;timeout&#39;
            result = _read_r(r, uart, ch, 1);
            // If result == 0 here than give up
            // 2nd False wake up call from the OS!!!
          else // The bit(s) have not been set in the specified &#39;timeout&#39;
            result = 0;
          }
        }
      else     // The bit(s) have not been set in the specified &#39;timeout&#39;
        result = 0;
      }
    }
  if(need_to_close_uart)
    _close_r(r, uart);
  return result;
}

My function in version 3. works fine, but unfortunatelly I have unwanted random delay of about 200-300us added from time to time - probably due to the bug in the Altera code. It is much better than the function version 1 but still not what I would like to have.

Of course - it could be a problem with my code, but in this case I do not see it and I would appreciate if you could point it out to me.

Thanks!

Altera_Forum · ‎05-05-2006

I have found a simple way to modify library: copy altera_avalon_uart.c module from the original folder to my project folder, rename it to mycorp_avalon_uart.c. Now I can modify the receiver ISR in my local module to wake up the task AFTER checking the parity/frame error and job would be done. My code is linking this way, that functions from my local module overwrite library functions.

The problem is that I still have read() returning zero in my readuartwithtimeout sometimes when called just after receiving OS flag set notification. Cannot fully understand the reason behind this behaviour when inspecting the code...

Any ideas?