Eliminare goto da HardwareSerial.cpp

Il codice originale del begin() della HardwareSerial.cpp é

void HardwareSerial::begin(unsigned long baud)
{
  uint16_t baud_setting;
  bool use_u2x = true;

#if F_CPU == 16000000UL
  // hardcoded exception for compatibility with the bootloader shipped
  // with the Duemilanove and previous boards and the firmware on the 8U2
  // on the Uno and Mega 2560.
  if (baud == 57600) {
    use_u2x = false;
  }
#endif

try_again:
  
  if (use_u2x) {
    *_ucsra = 1 << _u2x;
    baud_setting = (F_CPU / 4 / baud - 1) / 2;
  } else {
    *_ucsra = 0;
    baud_setting = (F_CPU / 8 / baud - 1) / 2;
  }
  
  if ((baud_setting > 4095) && use_u2x)
  {
    use_u2x = false;
    goto try_again;
  }

  // assign the baud_setting, a.k.a. ubbr (USART Baud Rate Register)
  *_ubrrh = baud_setting >> 8;
  *_ubrrl = baud_setting;

  transmitting = false;

  sbi(*_ucsrb, _rxen);
  sbi(*_ucsrb, _txen);
  sbi(*_ucsrb, _rxcie);
  cbi(*_ucsrb, _udrie);
}

in cui compare un’etichetta e un goto.

Credo che sia facilmente risolvibile con una variabile di stato e un do while in questo modo

void HardwareSerial::begin(unsigned long baud)
{
  uint16_t baud_setting;
  bool use_u2x = true, 
  bool try_again;

#if F_CPU == 16000000UL
  // hardcoded exception for compatibility with the bootloader shipped
  // with the Duemilanove and previous boards and the firmware on the 8U2
  // on the Uno and Mega 2560.
  if (baud == 57600) {
    use_u2x = false;
  }
#endif

 do {
   try_again = false;
   if (use_u2x) {
     *_ucsra = 1 << _u2x;
     baud_setting = (F_CPU / 4 / baud - 1) / 2;
   } else {
     *_ucsra = 0;
     baud_setting = (F_CPU / 8 / baud - 1) / 2;
   }
  
   if ((baud_setting > 4095) && use_u2x)
   {
     use_u2x = false;
     try_again = true;
   }
 } while(try_again);

  // assign the baud_setting, a.k.a. ubbr (USART Baud Rate Register)
  *_ubrrh = baud_setting >> 8;
  *_ubrrl = baud_setting;

  transmitting = false;

  sbi(*_ucsrb, _rxen);
  sbi(*_ucsrb, _txen);
  sbi(*_ucsrb, _rxcie);
  cbi(*_ucsrb, _udrie);
}

Cosa ne pensate?

Con le ottimizzazioni del compilatore i binari risultanti dovrebbero essere uguali.

Mia opinione: a me non dà fastidio un goto nelle librerie del core.

Il significato di quel pezzo di codice è di provare a configurare la seriale affinché usi una velocità doppia. Se questo non è possibile, viene riprovata la configurazione a velocità normale.
Secondo me la soluzione di Paolo è ridondante, a me pare che si possa risolvere più semplicemente così:

if (use_u2x) {
    *_ucsra = 1 << _u2x;
    baud_setting = (F_CPU / 4 / baud - 1) / 2;
    if ((baud_setting > 4095) && use_u2x) {
        use_u2x = false;
        *_ucsra = 0;
        baud_setting = (F_CPU / 8 / baud - 1) / 2;
    }
}

Senza variabili aggiuntive o do…while.

@Leo, il tuo pezzo tiene conto di quella forzatura nell'#if ?

#if F_CPU == 16000000UL
  if (baud == 57600)  {
    use_u2x = false;
  }
#endif

Scusate…
Stiamo discutendo per niente.
Nella versione 1.5.6 in sviluppo è stato già risolto. :sweat_smile:

void HardwareSerial::begin(unsigned long baud, byte config)
{
  // Try u2x mode first
  uint16_t baud_setting = (F_CPU / 4 / baud - 1) / 2;
  *_ucsra = 1 << U2X0;

  // hardcoded exception for 57600 for compatibility with the bootloader
  // shipped with the Duemilanove and previous boards and the firmware
  // on the 8U2 on the Uno and Mega 2560. Also, The baud_setting cannot
  // be > 4095, so switch back to non-u2x mode if the baud rate is too
  // low.
  if (((F_CPU == 16000000UL) && (baud == 57600)) || (baud_setting >4095))
  {
    *_ucsra = 0;
    baud_setting = (F_CPU / 8 / baud - 1) / 2;
  }

  // assign the baud_setting, a.k.a. ubbr (USART Baud Rate Register)
  *_ubrrh = baud_setting >> 8;
  *_ubrrl = baud_setting;

  _written = false;

  //set the data bits, parity, and stop bits
#if defined(__AVR_ATmega8__)
  config |= 0x80; // select UCSRC register (shared with UBRRH)
#endif
  *_ucsrc = config;
  
  sbi(*_ucsrb, RXEN0);
  sbi(*_ucsrb, TXEN0);
  sbi(*_ucsrb, RXCIE0);
  cbi(*_ucsrb, UDRIE0);
}

Se non è impostata la configurazione, viene messo di default 8N1

void begin(unsigned long baud) { begin(baud, SERIAL_8N1); }

Poichè non è previsto il back port nella 1.0.5+ ce lo teniamo così. :grin:

CHIUSO!!

@Nid:
no, mi era sfuggito. Ma come ha detto Paolo, parliamo del sesso degli angeli visto che hanno decretato come morto il ramo 1.0 :sweat_smile: