title: STM32 uart interrupt handler missing rx byte
body: <p>I am writing a small application on a STM F446:</p>
  <ul>
  <li>freertos (latest from git)</li>
  <li>lwip (pppos) (latest from git)</li>
  <li>LTE modem connected to uart2 (both rx and tx on interrupt, prio is 5)</li>
  <li>PC connected to uart3 (for logging) (only tx is used, also on interrupt prio 5)</li>
  </ul>
  <p>The amount of bytes that are received varies. So every received byte gets stored in the ring buffer on the interrupt. A dedicated lwip rx task is reading data from that task on highest prio and consumes the data from the ring buffer.</p>
  <p>Occasionally I ran into the problem that lwip is dropping a packet. When I started to compare received bytes vs logic analyzer I finally noticed the problem. I missed 1 byte in the cases where lwip drops the packet (due to bad fcs, which makes perfect sense then).</p>
  <p>I am rather new to this microcontroller world, so I am sure I am doing something wrong. I was hoping that somebody can give me some pointers.</p>
  <ul>
  <li>Are my interrupt handlers too bloated?</li>
  <li><strong>Must</strong> I use different priorities for each peripheral?</li>
  </ul>
  <p>The problem doesn't appear when I set uart3 to prio 6 (so one priority level lower than the uart connected to the modem). That's where I started to worry. Is it really a bad idea to have the same priority used for both uarts? Or is this a <em>clear sign</em> that something else is wrong in my code (specifically the interrupt handlers) which I should fix/improve?</p>
  <p>The interrupt handlers :</p>
  <pre><code>extern &quot;C&quot; void HAL_UART_RxCpltCallback(UART_HandleTypeDef *uartHandle)
  {
      if (uartHandle == &amp;uart2Handle)
      {
          uart2.RxHalInterruptCallback();
      }
  
      if (uartHandle == &amp;uart3Handle)
      {
          uart3.RxHalInterruptCallback();
      }
  }
  
  extern &quot;C&quot; void HAL_UART_TxCpltCallback(UART_HandleTypeDef *uartHandle)
  {
      if (uartHandle == &amp;uart2Handle)
      {
          uart2.TxHalInterruptCallback();
      }
  
      if (uartHandle == &amp;uart3Handle)
      {
          uart3.TxHalInterruptCallback();
      }
  }
  </code></pre>
  <p>And the implementation in uart class:</p>
  <pre><code>void RxHalInterruptCallback()
  {
      BaseType_t xHigherPriorityTaskWoken = pdFALSE;
  
      _rxRingBuffer.Store(_receivedByte);
  
      // Re-enable interrupt in HAL
      HAL_UART_Receive_IT(_handle, &amp;_receivedByte, 1);
  
      // Allow blocking read to continue, there is new data available
      xSemaphoreGiveFromISR(_rxSemaphore, &amp;xHigherPriorityTaskWoken);
  }
  
  void TxHalInterruptCallback()
  {
      uint16_t readBytes = 0;
      _txRingBuffer.ReadAll(256, _txBuffer, &amp;readBytes);
  
      if (readBytes)
      {
          HAL_UART_Transmit_IT(_handle, (uint8_t*)_txBuffer, readBytes*sizeof(uint8_t));
      }
  }
  </code></pre>
  <p>And finally, the ring buffer implementation:</p>
  <pre><code>class RingBuffer
  {
      public:
      RingBuffer(uint16_t size) : _size(size)
      {
          _head = 0;
          _tail = 0;
  
          _buffer = new uint8_t[size];        
      }
  
      virtual ~RingBuffer() 
      {
          delete [] _buffer;
      }
  
      virtual void Store(uint8_t byte)
      {
          // Store head and tail in atomic action to local variables
          volatile uint16_t head = _head;
          volatile uint16_t tail = _tail;
  
          _buffer[head++] = byte;
          head %= _size;
  
          // If head is equal to tail after store, we no longer know where our data is
          if (tail == head)
          {
              __disable_irq();
              while (1) 
              {
                  GPIOB-&gt;ODR |= LED_RED;
              }
          }
  
          // Restore head back to member
          _head = head;
      }
  
      virtual void Store(uint8_t *data, uint16_t length)
      {
          volatile uint16_t head = _head;
          volatile uint16_t tail = _tail;
  
          for (volatile uint16_t i = 0; i &lt; length; i++)
          {
              _buffer[head++] = data[i];
              head %= _size;
  
              // If head is equal to tail after store, we no longer know where our data is
              if (tail == head)
              {
                  __disable_irq();
                  while (1) 
                  {
                      GPIOB-&gt;ODR |= LED_RED;
                  }
              }
          }
  
          // Restore head back to member
          _head = head;
  
      }
  
      virtual void ReadAll(size_t maxLength, uint8_t *data, uint16_t *actualReadBytes)
      {
          // Store head and tail in atomic local variable
          volatile uint16_t tail = _tail;
          volatile uint16_t head = _head;
          
          // Keep grabbing bytes until we have all bytes or until we read the maximum amount of desired bytes
          while (tail != head &amp;&amp; (*actualReadBytes) &lt; maxLength)
          {
              data[(*actualReadBytes)++] = _buffer[tail++];
              tail %= _size;
          }
  
          // Restore tail back to member
          _tail = tail;
      }
  
      private:
  
      volatile uint16_t _head;
      volatile uint16_t _tail;
      volatile uint16_t _size;
      uint8_t *_buffer;
  };
  </code></pre>
  <p>PS: As experienced programmers will notice, I am still struggling when to use <code>volatile</code>. I don't know if that can rough up performance so hard that it will contribute to this problem. I am reading more on that in parallel. Again, guidance is appreciated.</p>