Skip to content

Avoid stalling if uart buffer becomes full #260

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

Closed
wants to merge 1 commit into from

Conversation

facchinm
Copy link
Member

@facchinm facchinm commented Sep 7, 2017

The idea is borrowed from AVR core https://github.com/arduino/Arduino/blob/master/hardware/arduino/avr/cores/arduino/HardwareSerial.cpp#L236

This should allow other interrupts to fire while the buffer is getting (slowly) emptied.

@ArduinoBot
Copy link

✅ Build completed.

⬇️ Build URL: http://downloads.arduino.cc/PR/samd/package_samd-b165_index.json

ℹ️ To test this build:

  1. Open the Preferences of the Arduino IDE.
  2. Add the Build URL above in the Additional Boards Manager URLs field, and click OK.
  3. Open the Boards Manager (menu Tools->Board->Board Manager...)
  4. Install Arduino SAMD core - Pull Request Avoid stalling if uart buffer becomes full #260
  5. Select one of the boards under SAMD Pull Request Avoid stalling if uart buffer becomes full #260 in Tools->Board menu
  6. Compile/Upload as usual

@cmaglie
Copy link
Member

cmaglie commented Sep 7, 2017

Why it stalls? Are you trying to write from inside an ISR?

On the AVR side the IRQ handler is called only if the global interrupts are disabled, and only if the UART already emptied the internal shift register:

  // If the output buffer is full, there's nothing for it other than to 
  // wait for the interrupt handler to empty it a bit
  while (i == _tx_buffer_tail) {
    if (bit_is_clear(SREG, SREG_I)) {      <----- here
      // Interrupts are disabled, so we'll have to poll the data
      // register empty flag ourselves. If it is set, pretend an
      // interrupt has happened and call the handler to free up
      // space for us.
      if(bit_is_set(*_ucsra, UDRE0))    <------ and here
	_tx_udr_empty_irq();
    } else {
      // nop, the interrupt handler will free up space for us
    }
  }

I think that we should implement a similar check here

@facchinm
Copy link
Member Author

facchinm commented Sep 7, 2017

Yes, you caught me 😄 Indeed I was printing from an ISR; the additional check doesn't seem to fix the situation, nor forcefully rearming the DataRegisterEmptyInterruptUART() interrupt.

@ArduinoBot
Copy link

✅ Build completed.

⬇️ Build URL: http://downloads.arduino.cc/PR/samd/package_samd-b174_index.json

ℹ️ To test this build:

  1. Open the Preferences of the Arduino IDE.
  2. Add the Build URL above in the Additional Boards Manager URLs field, and click OK.
  3. Open the Boards Manager (menu Tools->Board->Board Manager...)
  4. Install Arduino SAMD core - Pull Request Avoid stalling if uart buffer becomes full #260
  5. Select one of the boards under SAMD Pull Request Avoid stalling if uart buffer becomes full #260 in Tools->Board menu
  6. Compile/Upload as usual

@sandeepmistry
Copy link
Contributor

Closing in favour of #262.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants