AVR Freaks

Suggest one singleton instantiation of your class per UART / ISR vector.

(You couldn't  have two+ instances of the class for one UART anyway? Two init()s? Two potentially different baud rates? etc. etc.)

IS it worth it?

With an AVR, you're not going to have that many UARTs - so it's probably just as easy to copy & paste the code as to mess around complicating it with parameterisation ...

EDIT

Or you could have a general handler with a parameter, and the hardware-specific ISR fills-in that parameter to identify which UART it came from - something like:

void general_handler( u8 uart_id );

void ISR_UART0( void )
{
    general_handler( 0 );
}

void ISR_UART1( void )
{
    general_handler( 1 );
}

void ISR_UART2( void )
{
    general_handler( 2 );
}

Which is, IIRC, what ST do for the STM32 ...

Unfortunately the provided code often lacks a layered architecture that would allow the code to be easily reused.

I'd say, big deal...if it is 4K of AVR code you can just redo it (rewrite) or copy/paste & get 80% of the way there.  Making the code a lot more complex just to allow multiple instances & extreme re usability seems a poor trade-off, at least for smaller apps.  Of course, once you get that method working, maybe then you say great wonderful things about it...as long as it doesn't take up half of your avail code space! 

Please don't block-quote the entire post - just pick out enough for context.

Otherwise the thread gets very cluttered & unweildy & hard to follow.

If you're just replying to one post in general, use the 'Reply' button in that post - then the forum will automatically say which one you're referring to.

PS forgot to say that this dichotomy between the "floating" nature of C++ class instances and the fixed nature of the AVR interrupt vectors is a common hurdle in trying to make a modular C++ solution for peripherals that use interrupts in AVR.

Arduino does it like this:

D:\arduino-1.8.8\hardware\arduino\avr\cores\arduino>type HardwareSerial0.cpp
      // snip comment
#include "Arduino.h"
#include "HardwareSerial.h"
#include "HardwareSerial_private.h"

// Each HardwareSerial is defined in its own file, sine the linker pulls
// in the entire file when any element inside is used. --gc-sections can
// additionally cause unused symbols to be dropped, but ISRs have the
// "used" attribute so are never dropped and they keep the
// HardwareSerial instance in as well. Putting each instance in its own
// file prevents the linker from pulling in any unused instances in the
// first place.

#if defined(HAVE_HWSERIAL0)

#if defined(USART_RX_vect)
  ISR(USART_RX_vect)
#elif defined(USART0_RX_vect)
  ISR(USART0_RX_vect)
#elif defined(USART_RXC_vect)
  ISR(USART_RXC_vect) // ATmega8
#else
  #error "Don't know what the Data Received vector is called for Serial"
#endif
  {
    Serial._rx_complete_irq();
  }

#if defined(UART0_UDRE_vect)
ISR(UART0_UDRE_vect)
#elif defined(UART_UDRE_vect)
ISR(UART_UDRE_vect)
#elif defined(USART0_UDRE_vect)
ISR(USART0_UDRE_vect)
#elif defined(USART_UDRE_vect)
ISR(USART_UDRE_vect)
#else
  #error "Don't know what the Data Register Empty vector is called for Serial"
#endif
{
  Serial._tx_udr_empty_irq();
}

#if defined(UBRRH) && defined(UBRRL)
  HardwareSerial Serial(&UBRRH, &UBRRL, &UCSRA, &UCSRB, &UCSRC, &UDR);
#else
  HardwareSerial Serial(&UBRR0H, &UBRR0L, &UCSR0A, &UCSR0B, &UCSR0C, &UDR0);
#endif

// Function that can be weakly referenced by serialEventRun to prevent
// pulling in this file if it's not otherwise used.
bool Serial0_available() {
  return Serial.available();
}

#endif // HAVE_HWSERIAL0

So this instantiates one (global) copy of the HardwareSerial class per UART (the above file is for a single UART or UART0 if more than one). There are also similar HardwareSerial1.cpp, HardwareSerial2.cpp, etc

Now this does kind of break the suggestion above as it calls(EDIT: *) a class member function from the ISR (that is Serial._rx_complete_irq() or Serial._tx_udr_empty_irq()) but this is Arduino and they've never been too fussed about efficiency! ;-)

It does, however, show one possible approach.

*: sorry I just realised that the code "called" from the ISRs may well be something like:

// Actual interrupt handlers //////////////////////////////////////////////////////////////

void HardwareSerial::_rx_complete_irq(void)
{
  if (bit_is_clear(*_ucsra, UPE0)) {
    // No Parity error, read byte and store it in the buffer if there is
    // room
    unsigned char c = *_udr;
    rx_buffer_index_t i = (unsigned int)(_rx_buffer_head + 1) % SERIAL_RX_BUFFER_SIZE;

    // if we should be storing the received character into the location
    // just before the tail (meaning that the head would advance to the
    // current location of the tail), we're about to overflow the buffer
    // and so we don't write the character or advance the head.
    if (i != _rx_buffer_tail) {
      _rx_buffer[_rx_buffer_head] = c;
      _rx_buffer_head = i;
    }
  } else {
    // Parity error, read byte but discard it
    *_udr;
  };
}

However the class header has:

  public:
    inline HardwareSerial(
      volatile uint8_t *ubrrh, volatile uint8_t *ubrrl,
      volatile uint8_t *ucsra, volatile uint8_t *ucsrb,
      volatile uint8_t *ucsrc, volatile uint8_t *udr);
    void begin(unsigned long baud) { begin(baud, SERIAL_8N1); }
    void begin(unsigned long, uint8_t);
    void end();
    virtual int available(void);
    virtual int peek(void);
    virtual int read(void);
    virtual int availableForWrite(void);
    virtual void flush(void);
    virtual size_t write(uint8_t);
    inline size_t write(unsigned long n) { return write((uint8_t)n); }
    inline size_t write(long n) { return write((uint8_t)n); }
    inline size_t write(unsigned int n) { return write((uint8_t)n); }
    inline size_t write(int n) { return write((uint8_t)n); }
    using Print::write; // pull in write(str) and write(buf, size) from Print
    operator bool() { return true; }

    // Interrupt handlers - Not intended to be called externally
    inline void _rx_complete_irq(void);
    void _tx_udr_empty_irq(void);

So the code of it should be inlined into the ISR handler.

I think that's a chicken & egg thing: the chip makers stick with 'C' because that's what the majority of customers want; the customers don't want to jump to C++ because all the chip stuff is in 'C' ...

Chipmakers are in business to sell chips, not software or programming languages.

mbed uses C++ ...

Traditionally yes.

But I think that is being somewhat eroded by the prevalence of ARM Cortex, where you just decide whether you M0, M3, M4 or whatever performance level - and then pretty much any manufacturer's chip will do ?

Whether that's ultimately a Good Thing is another question ... ?

Making code "universal"  so that it can run on different chips having different timer sizes, different peripherals, etc, seems like falling into a bottomless hole.  Maybe we'll eventually have an app where you just list some English specifications and it will take care of all the rest for the chip at hand (or will tell you it can't be done), sold with the famous tagline  "no programming needed".

Not hugely keen on stuff like:

            m_base_vect(n*3)
            ,m_base((vu8ptr)(n == USART3 ? 0x130 : 0xc0 + n*8))

That's very reliant on a specific layout of interrupt vectors and register sets.

Is the reader/maintainer even going to understand the significance of the "* 3", etc here ?

Actually are you really supposed to do arithmetic on enums ?

My point was that while I'm sure you could do something like:

enum {
    RED,
    AMBER,
    GREEN
} color_e;

enum {
    STOPPED,
    FORWARD,
    REVERSE
} motion_e;

int n = AMBER * REVERSE;

it may well be valid syntax but it does not make a lot of sense!