ISR with function arguments (in a modular driver approach)

Go To Last Post
47 posts / 0 new
Author
Message
#1
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Hi,

 

I am writing a driver in the style of Beningo, following this White Paper.

The structure is this:

 

I have a "serial" library, with a structure ser_dev_st containing a buffer and all the needed stuff, and can be instantiated as array to handle more serial consoles.

The "serial" rely on a HW related "uart", where here I am using a struct uart_hal_cgf_t initialized according to a Configuration Table. I wrote the driver and the hierarchy succesfully, but I crashed in the moment I inserted the ISRs in the "serial".

 

Here is the working structure:

 

------------------------------------------------------------------
in serial.c
------------------------------------------------------------------
typedef struct serial_device
{
	rb_t serial_tx_buff;
	rb_t serial_rx_buff;
	uint8_t tx_buff[SER_RB_SIZE_TX];
	uint8_t rx_buff[SER_RB_SIZE_RX];
	uart_hal_cgf_t hal;
} ser_dev_st;

void Ser_init(ser_dev_st* console, uart_hal_cgf_t* config)
{
    console->hal = config;
    /// a lot of code initializing the parameters of "console" according to "config"
    UartHal_init(console->hal);
}

------------------------------------------------------------------
in uart_hal.c
------------------------------------------------------------------

typedef struct
{
	uart_hal_ch_t channel;
	uart_hal_mode_t mode;
	long	baudrate;
} uart_hal_cgf_t;

address_t volatile * const ucsra[NUM_UART_CHANNELS] =
{
	(address_t*)&UCSR0A,
	(address_t*)&UCSR1A
};

void UartHal_init(uart_hal_cgf_t* handle)
{
	uint16_t baud_reg = 0;

	ucsra[handle->channel] = 0;

    // ecc ecc
}

Now the situation is that the application can call the Ser_* functions with the relative ser_dev_st console, accessing to that particular ring buffers and interacting with them. The Serial is now modular and reusable and works, as long as the HAL, which is modular with respect to the hardware (2x UART, means ucsra is an array of 2 SFR register elements and so on). With the proper tables, I make the initializations.

 

The problem happens when I need to handle the driver uaing the ISR, I get stuck in the following situation, becase I have no way to tell which console use in the ISR:

 

---------------------------------------------
in serial.c
---------------------------------------------

static void ser_rx_char_ISR(/* NO PARAMETERS ALLOWED */)
{
	if (!Rb_is_full(console->serial_rx_buff)) ////////////////////////////// I WANT TO REFER TO &console SOMEHOW...
	{
		Rb_put_char(console->serial_rx_buff, Uart_get_char(console->hal));
	}
	else
	{
		// ecc
	}
	//ecc
}

How can I use a console associated to a given hal, or to a given interrupt? This is just because I cannot pass any argument in the ISR and I have to find a way to know which console to use, when an ISR trig.

Also, I normally use an "interrupt hal" to setup the ISR, like IntHal_vector_register(ser_rx_char_ISR, HAL_VECT_UART_RX_ID); where I just link to a callback present in the ISR(vect) space. This approach makes it portable nicely between AVR and ARM, but only now I switched to this more modular approach, and hence facing this ISR issue.

 

Thanks!

Last Edited: Mon. Oct 7, 2019 - 06:05 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

The ISR is connected by hardware to a specific UART.

 

it knows nothing about "consoles" - that's a higher-level abstraction that your driver makes.

 

The ISR just interacts with buffers specific to its UART. It's you driver which associates those buffers with a particular "console".

 

 

Top Tips:

  1. How to properly post source code - see: https://www.avrfreaks.net/comment... - also how to properly include images/pictures
  2. "Garbage" characters on a serial terminal are (almost?) invariably due to wrong baud rate - see: https://learn.sparkfun.com/tutorials/serial-communication
  3. Wrong baud rate is usually due to not running at the speed you thought; check by blinking a LED to see if you get the speed you expected
  4. Difference between a crystal, and a crystal oscillatorhttps://www.avrfreaks.net/comment...
  5. When your question is resolved, mark the solution: https://www.avrfreaks.net/comment...
  6. Beginner's "Getting Started" tips: https://www.avrfreaks.net/comment...
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

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.)

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

awneil wrote:

The ISR is connected by hardware to a specific UART.

 

it knows nothing about "consoles" - that's a higher-level abstraction that your driver makes.

 

The ISR just interacts with buffers specific to its UART. It's you driver which associates those buffers with a particular "console".

 

 

 

I know that, but I need to convey the char to a particular buffer, which is part of the serial driver, not the uart - the uart just take care of the hw. So when using the IntHal_* interrupts HALs, before I was linking the unique HW interrupt to the unique serial driver. Now the problem changed, as I might want to use the same ISR code with different HW UARTs, hence I need to parameterize the variables, with an additional form of information used to chose the right serial element of the serial array. The code at the serial.c level should run with no changes on more complex architectures as well with many more UARTs, ideally. As you can imagine, this is more about architecting a firmware more than achieving something practical: this scheme (if possible) could then be applied on other contexts as well.

 

clawson wrote:

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

 

 

 

I thought the same, but there is a thing: to do this, I need to save into the serial.c context the whole structure of the UART_0, UART_1 and so on, so the singleton can access to that freely. But now, really is the application on top of the serial.c that should hold such data according to the serial structure. I can make a workaround and buffer all the serial content (from the context of the application on top of the serial) inside the serial.c context. But with buffers of many bytes, is going to take exactly the double of the RAM memory effectively needed.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 1

thexeno wrote:
hence I need to parameterize the variables

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 ...

 

Top Tips:

  1. How to properly post source code - see: https://www.avrfreaks.net/comment... - also how to properly include images/pictures
  2. "Garbage" characters on a serial terminal are (almost?) invariably due to wrong baud rate - see: https://learn.sparkfun.com/tutorials/serial-communication
  3. Wrong baud rate is usually due to not running at the speed you thought; check by blinking a LED to see if you get the speed you expected
  4. Difference between a crystal, and a crystal oscillatorhttps://www.avrfreaks.net/comment...
  5. When your question is resolved, mark the solution: https://www.avrfreaks.net/comment...
  6. Beginner's "Getting Started" tips: https://www.avrfreaks.net/comment...
Last Edited: Mon. Oct 7, 2019 - 09:51 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 2

awneil wrote:
IS it worth it?
+1

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 2

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! 

When in the dark remember-the future looks brighter than ever.   I look forward to being able to predict the future!

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 2

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 ISR_UART0( void ) {
    general_handler( 0 );
}

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

Note that on AVRs, there is significant overhead to using an additional layer of function calls from an ISR.

Significant enough to be comparable to the total code size of a typical USART ISR, in fact.

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

avrcandies wrote:

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! 

 

I totally agree. Is to learn, and make it as much standard as possible and not avoid certain paths just because of my ignorance. If I want to make something open source later on that, with a mess of code also on more complex devices, everyone will not follow the standard, because there might no be any. Because such things are done in complex MCUs with C++, and I want to learn how much I can do in this direction without using C++ also in small AVR. So this work is a learning exercise which hopefully could give me perspective on these firmware architectures.

 

awneil wrote:

thexeno wrote:
hence I need to parameterize the variables

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 ...

 

 

I am actually already doing this, like I had discussed once here: but this does not solve the problem when I have a parameterized data in the ISR. hence this post. Seems the solution is the one proposed by @Clawson here (singleton approach), but again:

 

thexeno wrote:

 

clawson wrote:

 

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

 

 

 

I thought the same, but there is a thing: to do this, I need to save into the serial.c context the whole structure of the UART_0, UART_1 and so on, so the singleton can access to that freely. But now, really is the application on top of the serial.c that should hold such data according to the serial structure. I can make a workaround and buffer all the serial content (from the context of the application on top of the serial) inside the serial.c context. But with buffers of many bytes, is going to take exactly the double of the RAM memory effectively needed.

 

 

 

The thing is that I don't want to give up because "is too difficult": is for learning, not for delivering to a customer.

I try to explain more...
This "requirement" is given from this thought: let's say The UART is used in 2 different drivers, one for a "monitor application" and the other for a "teletype application" both running on the same MCU. The teletype needs 32byte of TX buffer, and needs to be visible only on the teletype application module, the monitor application (with another 32bytes but on the RX buffer) does not have to have any means to access such data, so if by mistake I make an access to the teletype data through the monitor, I must get a compilation error. In this way, the only solution is to parse the public type of the UART (accessed through the UART haders) from the teletype to the UART driver. On the other side, from the monitor, is the same. So when calling the (char)UART_Get(*uart public type), is effectively using different UART HW modules from the same library, one from the monitor, the other from the teletype.

Because for me in this communication libraries the normal approach is to use ISR (while for some reason on Beningo is using wait loops on the SPI example*). The best approach seems to handle the ring buffers directly in the ISR, but if the ISR is trig from the HW, the HW has no way to access the teletype buffer, because the HAL does not have information about the application.

 

 

* = because otherwise was impossible with such approach? Are we really sure about this?

Last Edited: Tue. Oct 8, 2019 - 08:33 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 1

westfw wrote:
Note that on AVRs, there is significant overhead to using an additional layer of function calls from an ISR.

Agreed - hence the "is it worth it?" question.

 

Is that overhead mitigated by making the called function 'inline' ... ?

 

EDIT: yes, it does - see #16.

Top Tips:

  1. How to properly post source code - see: https://www.avrfreaks.net/comment... - also how to properly include images/pictures
  2. "Garbage" characters on a serial terminal are (almost?) invariably due to wrong baud rate - see: https://learn.sparkfun.com/tutorials/serial-communication
  3. Wrong baud rate is usually due to not running at the speed you thought; check by blinking a LED to see if you get the speed you expected
  4. Difference between a crystal, and a crystal oscillatorhttps://www.avrfreaks.net/comment...
  5. When your question is resolved, mark the solution: https://www.avrfreaks.net/comment...
  6. Beginner's "Getting Started" tips: https://www.avrfreaks.net/comment...
Last Edited: Tue. Oct 8, 2019 - 09:20 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

westfw wrote:

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 ISR_UART0( void ) {
    general_handler( 0 );
}

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

Note that on AVRs, there is significant overhead to using an additional layer of function calls from an ISR.

Significant enough to be comparable to the total code size of a typical USART ISR, in fact.

 

 

ah interesting, what do you mean? My current ISR is this (with the "singleton" approach, where the buffers are public and directly used in the ISR):
 

static void ser_rx_char_ISR(void)
{
	if (!Rb_is_full(&serial_rx_buff))
	{
		Rb_put_char(&serial_rx_buff, Uart_get_char(), (rb_sz_t)SER_RB_SIZE);		
	}
	else
	{
		/* Discard the byte */
		(void)Uart_get_char();
	}
}

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

awneil wrote:

westfw wrote:
Note that on AVRs, there is significant overhead to using an additional layer of function calls from an ISR.

Agreed - hence the "is it worth it?" question.

 

 

 

sure, I see. I don't think is worth if I don't plan to use this on Tiva beasts and the like.

Same for the RTOS approach, I learn a lot by hitting the head if using on AVR, exactly because is not designed for that - after couple of trials and realizing immediately how much was a fail, it took me 10 times less time to make a port to a proper ARM because I developed a feeling of that. So for me this learn approach works, I hope you understand what I mean. I learn only if I struggle on something :D :D

Last Edited: Tue. Oct 8, 2019 - 08:42 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

 

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.

 

 

 

Top Tips:

  1. How to properly post source code - see: https://www.avrfreaks.net/comment... - also how to properly include images/pictures
  2. "Garbage" characters on a serial terminal are (almost?) invariably due to wrong baud rate - see: https://learn.sparkfun.com/tutorials/serial-communication
  3. Wrong baud rate is usually due to not running at the speed you thought; check by blinking a LED to see if you get the speed you expected
  4. Difference between a crystal, and a crystal oscillatorhttps://www.avrfreaks.net/comment...
  5. When your question is resolved, mark the solution: https://www.avrfreaks.net/comment...
  6. Beginner's "Getting Started" tips: https://www.avrfreaks.net/comment...
Last Edited: Tue. Oct 8, 2019 - 08:44 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

thexeno wrote:
ah interesting, what do you mean?
If you build this:

#include <avr/io.h>
#include <avr/interrupt.h>

uint8_t uart_data;

int main(void)
{
	while (1)
	{
	}
}

ISR(USART_RX_vect) {
	uart_data = UDR0;
}

the ISR code yields:

00000092 <__vector_18>:

ISR(USART_RX_vect) {
  92:	1f 92       	push	r1
  94:	0f 92       	push	r0
  96:	0f b6       	in	r0, 0x3f	; 63
  98:	0f 92       	push	r0
  9a:	11 24       	eor	r1, r1
  9c:	8f 93       	push	r24
	uart_data = UDR0;
  9e:	80 91 c6 00 	lds	r24, 0x00C6	; 0x8000c6 <__TEXT_REGION_LENGTH__+0x7e00c6>
  a2:	80 93 00 01 	sts	0x0100, r24	; 0x800100 <_edata>
  a6:	8f 91       	pop	r24
  a8:	0f 90       	pop	r0
  aa:	0f be       	out	0x3f, r0	; 63
  ac:	0f 90       	pop	r0
  ae:	1f 90       	pop	r1
  b0:	18 95       	reti

If, however I move the reading into a called function:

void read_UDR() {
	uart_data = UDR0;
}

int main(void)
{
	while (1)
	{
	}
}

ISR(USART_RX_vect) {
	read_UDR();
}

Then what that ends up with is:

00000090 <read_UDR>:

void read_UDR() {
	uart_data = UDR0;
  90:	80 91 c6 00 	lds	r24, 0x00C6	; 0x8000c6 <__TEXT_REGION_LENGTH__+0x7e00c6>
  94:	80 93 00 01 	sts	0x0100, r24	; 0x800100 <_edata>
  98:	08 95       	ret
0000009c <__vector_18>:

ISR(USART_RX_vect) {
  9c:	1f 92       	push	r1
  9e:	0f 92       	push	r0
  a0:	0f b6       	in	r0, 0x3f	; 63
  a2:	0f 92       	push	r0
  a4:	11 24       	eor	r1, r1
  a6:	2f 93       	push	r18
  a8:	3f 93       	push	r19
  aa:	4f 93       	push	r20
  ac:	5f 93       	push	r21
  ae:	6f 93       	push	r22
  b0:	7f 93       	push	r23
  b2:	8f 93       	push	r24
  b4:	9f 93       	push	r25
  b6:	af 93       	push	r26
  b8:	bf 93       	push	r27
  ba:	ef 93       	push	r30
  bc:	ff 93       	push	r31
	read_UDR();
  be:	0e 94 48 00 	call	0x90	; 0x90 <read_UDR>
  c2:	ff 91       	pop	r31
  c4:	ef 91       	pop	r30
  c6:	bf 91       	pop	r27
  c8:	af 91       	pop	r26
  ca:	9f 91       	pop	r25
  cc:	8f 91       	pop	r24
  ce:	7f 91       	pop	r23
  d0:	6f 91       	pop	r22
  d2:	5f 91       	pop	r21
  d4:	4f 91       	pop	r20
  d6:	3f 91       	pop	r19
  d8:	2f 91       	pop	r18
  da:	0f 90       	pop	r0
  dc:	0f be       	out	0x3f, r0	; 63
  de:	0f 90       	pop	r0
  e0:	1f 90       	pop	r1
  e2:	18 95       	reti

The fact is that as soon as you call a function from an ISR the compiler has to assume that the called function could be corrupting any of the important registers so it PUSHes then POPs all of them to make sure that the potential damage goes by unnoticed.

 

That is quite an overhead !

 

If the C++ class (singleton) can make a pointer to its buffer structures available in a global then you could just have (inline) the insert_into_buffer code within the ISR() and avoid the CALL overhead.

 

Because it is a singleton then once instantiated the buffer structure instance is at a fixed address and there only will be one so it's quite safe to put a pointer to it into a globally accessible pointer so the ISR code can "find" it.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

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.

Last Edited: Tue. Oct 8, 2019 - 09:11 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

clawson wrote:
The fact is that as soon as you call a function from an ISR the compiler has to assume that the called function could be corrupting any of the important registers so it PUSHes then POPs all of them to make sure that the potential damage goes by unnoticed.

 

making the called function 'static' helps a lot:

int main(void)
{
  ae:	ff cf       	rjmp	.-2      	; 0xae <main>

000000b0 <USART_RX_vect>:
	while (1)
	{
	}
}

ISR(USART_RX_vect) {
  b0:	1f 92       	push	r1
  b2:	0f 92       	push	r0
  b4:	0f b6       	in	r0, 0x3f	; 63
  b6:	0f 92       	push	r0
  b8:	11 24       	eor	r1, r1
  ba:	8f 93       	push	r24


volatile uint8_t uart_data;

static inline void read_UDR() {
	uart_data = UDR0;
  bc:	8c b1       	in	r24, 0x0c	; 12
  be:	80 93 00 01 	sts	0x0100, r24	; 0x800100 <_edata>
	}
}

ISR(USART_RX_vect) {
	read_UDR();
}
  c2:	8f 91       	pop	r24
  c4:	0f 90       	pop	r0
  c6:	0f be       	out	0x3f, r0	; 63
  c8:	0f 90       	pop	r0
  ca:	1f 90       	pop	r1
  cc:	18 95       	reti

(the above also has 'inline', but just 'static' gave the same result - I guess the compiler decides to inline it itself)

 

Top Tips:

  1. How to properly post source code - see: https://www.avrfreaks.net/comment... - also how to properly include images/pictures
  2. "Garbage" characters on a serial terminal are (almost?) invariably due to wrong baud rate - see: https://learn.sparkfun.com/tutorials/serial-communication
  3. Wrong baud rate is usually due to not running at the speed you thought; check by blinking a LED to see if you get the speed you expected
  4. Difference between a crystal, and a crystal oscillatorhttps://www.avrfreaks.net/comment...
  5. When your question is resolved, mark the solution: https://www.avrfreaks.net/comment...
  6. Beginner's "Getting Started" tips: https://www.avrfreaks.net/comment...
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

>Because such things are done in complex MCUs with C++, and I want to learn how much I can do in this direction without using C++ also in small AVR.

 

There is no reason that c++ cannot be used on a small avr. I use it on a tiny416 without problems. If you want c++ like code, it is much better to use c++ than try to turn c into c++.  You don't need to treat the c++ compiler like its running on a pc, and there are plenty of features available that are useful on a micro that do not require bringing in a bunch of extra code.

 

This is not something that would work on an 'old' avr, but I use only __vector_default as my one entry into every isr, and use a function pointer array to get to the needed isr code-

https://github.com/cv007/ATTiny416XplainedNano/blob/master/Cpuint.cpp

since the 0/1 series shows which interrupt vector you are in (when in round-robin), it is used as an index to the array. Its not a time efficient way to do things, or ram friendly (but doesn't seem to cause any problems for me), but you end up with only one isr entry/exit and can set/change the interrupt function pointers as needed. I do the same thing on a pic32mm, and it works basically the same. Never have to deal with setting up isr's, just set a function for an irq to use (lamba's work good also).

 

There is a lot to like with c++, and I wish I would have started learning it sooner, but I'm usually behind the times anyway.

 

Here is a simple example of things I keep discovering in c++ that are useful, even in micros-

Wouldn't it be nice to treat eeprom like a normal var? I think it would, so came up with a way to do it with operator overloading, and the code produced is just as small as anything else.

 

//headers here

EEMEM EEvar<uint8_t> ee_var{0xEE}; //init data will be in hex file
URMEM URvar<uint8_t> ur_var{0x77};

 

int main(){

    ee_var = ur_var; //write userrow var to eeprom var

    for(;;){}

}

00000086 <main>:
  86:    80 91 00 13     lds    r24, 0x1300    ; 0x801300 <ur_var>
  8a:    90 91 02 10     lds    r25, 0x1002    ; 0x801002 <__RODATA_PM_OFFSET__+0x7f9002>
  8e:    91 fd           sbrc    r25, 1
  90:    fc cf           rjmp    .-8          ; 0x8a <main+0x4>
  92:    80 93 00 14     sts    0x1400, r24    ; 0x801400 <ee_var>
  96:    8d e9           ldi    r24, 0x9D    ; 157
  98:    84 bf           out    0x34, r24    ; 52
  9a:    83 e0           ldi    r24, 0x03    ; 3
  9c:    80 93 00 10     sts    0x1000, r24    ; 0x801000 <__RODATA_PM_OFFSET__+0x7f9000>
  a0:    ff cf           rjmp    .-2          ; 0xa0 <main+0x1a>

 

 

I don't mean to steer this thread in some other direction, but I think it would be somewhat of a mistake to invest any significant amount of time trying to be 'c++ like' with c. Several years from now you will have wished that time was spent learning c++. That's only my opinion of course. When you move up to a 32bit micro, things get even better. Currently, the biggest downside is that it seems micro manufacturers have little interest in c++. I'm sure all it would take is one of them to make a big push, and they all would want to get in on the action. I'm probably dreaming, though.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

curtvm wrote:
This is not something that would work on an 'old' avr,
Sadly it would appear from OP's other threads that it is 328P he's most likely working with. So I don't see how you could have a generic handler as there's no obvious way to identify what the source of the "current interrupt" is?

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

>Sadly it would appear from OP's other threads that it is 328P he's most likely working with

He showed 2 uarts with old style names

	(address_t*)&UCSR0A,
	(address_t*)&UCSR1A

I think a 328 has only 1 (?). In any case its not a 0/1 series, so no it would not work.

 

You can start tearing into the whole vectors, making them all call's instead of jump's, then handcraft your own vector to figure out what is on the stack to get the vector number, remove it, etc. By the time you get to that point you will wonder why its being done in the first place. If its too much work to setup such a contraption, its usually not worth the trouble because it will follow you everywhere you go and you will have to keep feeding it, giving it attention, etc. (it becomes a pet).

 

edit-

here is an example of trying to do the same for a mega640-

https://godbolt.org/z/gyxkw8

https://godbolt.org/z/IByv_b

(just pieced together, isr stuff in middle/bottom somewhere- probably would work, don't look too closely at the other code, it was just trying some things)

 

There is such a good selection of micros now, that unless I had some good reason to remain with the 'old' avr's, I would move over to the 0/1 series or pick a 32bit micro of some sort. 

Last Edited: Tue. Oct 8, 2019 - 10:12 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I indeed use the 328p and I am trying to make some standard basic HAL for the same application running on a completely different architecture, like the TI CC3200, an STM CM0 (to compare 2 ARM types how much differs to one another) and a PIC18. I smoothed out a firmware architecture, starting from the 328, hitting my head on different architectures and shaping to find common ground: seeing what is required to make the application isolated from the hardware, or somehow adapting the interface with few configuration headers.

 

The idea was to test the modularity on a mega2560 which contains 4 UARTs. I will check now with using the singleton approach and maybe layering through static or inline statements. I might say an atrocity because I am bit ignorant on C++ (let's say more than C), but seems that unless I switch to C++, the modularity can be achieved but not in the ISRs, but it seems that C++ will allow me to achieve what I want...

Last Edited: Tue. Oct 8, 2019 - 01:17 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

curtvm wrote:
it seems micro manufacturers have little interest in c++.

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++ ...

Top Tips:

  1. How to properly post source code - see: https://www.avrfreaks.net/comment... - also how to properly include images/pictures
  2. "Garbage" characters on a serial terminal are (almost?) invariably due to wrong baud rate - see: https://learn.sparkfun.com/tutorials/serial-communication
  3. Wrong baud rate is usually due to not running at the speed you thought; check by blinking a LED to see if you get the speed you expected
  4. Difference between a crystal, and a crystal oscillatorhttps://www.avrfreaks.net/comment...
  5. When your question is resolved, mark the solution: https://www.avrfreaks.net/comment...
  6. Beginner's "Getting Started" tips: https://www.avrfreaks.net/comment...
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I can understand how you can use a HAL to make a "software only" process like JPEG decode or fixed point maths or even an OS build for multiple architectures but I've never really understood the HAL thing for micros for a couple of reasons:

 

1) adding layers of abstraction to code makes it slow and bloaty. You just need to look at Arduino for an example of this (or Atmel ASF / Start etc)

 

2) surely the whole point of micros is that they have "unique selling points" that makes one type attractive over another. If you make a HAL that has to work on multi platforms don't you limit yourself to the lowest common deonmiator of generic operation. Take a micro that can not only do the "standard" baud rates of 300, 1200, 2400, 4800, 9600, etc but it has a fractional baude rate generator and a high speed clock so you can achieve 1234567 baud. Only thing is you can't use your generic HAL/UART code to achieve this as there are 3 platforms the HAL works on that can't achieve this. Or how about a micro with 32 bit timers - only your HAL has to operate on multiple platforms, some of which only have 8 or 16 bit timers so you can't offer 32 bit counter as a generic device. Everything is drawn back to the lowest common denominators.

 

So what is the point of HAL - does the code re-use over 3 or 7 architectures really justify the size, speed and feature penalties of a generic interface?

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

clawson wrote:
2) surely the whole point of micros is that they have "unique selling points" that makes one type attractive over another.

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 ... ?

 

Top Tips:

  1. How to properly post source code - see: https://www.avrfreaks.net/comment... - also how to properly include images/pictures
  2. "Garbage" characters on a serial terminal are (almost?) invariably due to wrong baud rate - see: https://learn.sparkfun.com/tutorials/serial-communication
  3. Wrong baud rate is usually due to not running at the speed you thought; check by blinking a LED to see if you get the speed you expected
  4. Difference between a crystal, and a crystal oscillatorhttps://www.avrfreaks.net/comment...
  5. When your question is resolved, mark the solution: https://www.avrfreaks.net/comment...
  6. Beginner's "Getting Started" tips: https://www.avrfreaks.net/comment...
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I'd suggest the concepts Beningo presents work well on the likes of ARMs rather than AVRs. If you have a 10cent micro with 32 bytes of ram and 1k flash, you're probably not going to use C. If you have an AVR with 1k ram, you're probably not going to use a pre-emptive RTOS.

 

Apply the right technique to the right application.

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

only your HAL has to operate on multiple platforms, some of which only have 8 or 16 bit timers so you can't offer 32 bit counter as a generic device. Everything is drawn back to the lowest common denominators.

 

Only addressing this for a moment: this topic is for a personal project, but I am actually working on a big project with a lot of C++ abstraction. I had long discussions, about exactly what you said. But you know, a company like have things standard, so the answer to how to use the specialized functions was "just add a specialized function" because are normally seldom used, while the common denominator ones are always used (that is true, from TI a UART is likely to be similar to other TI or even ARM of same family of different manufacturers). Probably what I am doing is an unconscious attempt to apply what I am learning on the job to my personal projects, just without the overthinking of making an absolute standard across every possible Turing machine at expenses of ridiculous performance :D (sometimes a slow fast firmware which does not requires so much customization on a new product is cheaper, despite I strongly dislike that if is slower for compatibility - like using Linux to blink a LED)
Is philosophical material....

In my personal pet project, it may very well turn out that is not doable or ridiculously worthless, but I already have everything written on my personal firmware architecture project, except the ISR part, so just for the learning curve is worth knowing - ok maybe because I am not a guru, I am still learning a lot with every mistake or success. Best case scenario is a working firmware, worst case I can write a note on how not to write firmware.

 

Last Edited: Tue. Oct 8, 2019 - 02:53 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

awneil wrote:
and then pretty much any manufacturer's chip will do ?
yes but the reason there are 70+ different MO/3/4 vendors is that each tries to give some USPs like a new kind of peripherals (perhaps 5 of those 70 have a DMA engine?) or some common peripherals (SPI, timers, ADC, I2C, etc) have added extras like an SPI that can vary the bit length from 5 to 32 bits or an ADC that can decode 4 channels in parallel or something. If you make a HAL then it can't offer 27 bit SPI or parallel channel ADC because only one of the supported devices actually has these functions. Equally the DMA thing - you can't put it in your HAL because 65 manufacturers devices don't have it.

 

Sure I get VxWorks/Linux/Nucleus BSPs - but actually their low level h/w support only probably demands a handful of common peripherals like one 32 bit timer or something. 

 

But micrcontrollers and microncontrollers - they present special hardware peripherals for specialist hardware jobs and it's difficult to make a common API that covers all they have. And if you do you possibly limit yourself to their common functionality (cf Arduino!).

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

clawson wrote:

awneil wrote:
and then pretty much any manufacturer's chip will do ?
yes but the reason there are 70+ different MO/3/4 vendors is that each tries to give some USPs like a new kind of peripherals (perhaps 5 of those 70 have a DMA engine?) or some common peripherals (SPI, timers, ADC, I2C, etc) have added extras like an SPI that can vary the bit length from 5 to 32 bits or an ADC that can decode 4 channels in parallel or something. If you make a HAL then it can't offer 27 bit SPI or parallel channel ADC because only one of the supported devices actually has these functions. Equally the DMA thing - you can't put it in your HAL because 65 manufacturers devices don't have it.

 

Sure I get VxWorks/Linux/Nucleus BSPs - but actually their low level h/w support only probably demands a handful of common peripherals like one 32 bit timer or something. 

 

But micrcontrollers and microncontrollers - they present special hardware peripherals for specialist hardware jobs and it's difficult to make a common API that covers all they have. And if you do you possibly limit yourself to their common functionality (cf Arduino!).

 

I also think that a graphic driver sitting on the SPI, if uses a faster SPI with some weird functionalities, those can be clustered within the API, so the LCD function will still see the same interface, but under the hood the SPI will be used differently: the result will be a hopefully faster graphic library.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I also think that a graphic driver sitting on the SPI, if uses a faster SPI with some weird functionalities, those can be clustered within the API, so the LCD function will still see the same interface, but under the hood the SPI will be used differently: the result will be a hopefully faster graphic library.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

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".

When in the dark remember-the future looks brighter than ever.   I look forward to being able to predict the future!

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

avrcandies wrote:
"no programming needed".

laugh

 

 

Top Tips:

  1. How to properly post source code - see: https://www.avrfreaks.net/comment... - also how to properly include images/pictures
  2. "Garbage" characters on a serial terminal are (almost?) invariably due to wrong baud rate - see: https://learn.sparkfun.com/tutorials/serial-communication
  3. Wrong baud rate is usually due to not running at the speed you thought; check by blinking a LED to see if you get the speed you expected
  4. Difference between a crystal, and a crystal oscillatorhttps://www.avrfreaks.net/comment...
  5. When your question is resolved, mark the solution: https://www.avrfreaks.net/comment...
  6. Beginner's "Getting Started" tips: https://www.avrfreaks.net/comment...
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

In that scenario using C++, what I can say is that is solved by havin all the API using the same interfaces, but changing the object, let the usage of different HALs without using arrays of peripherals: but this is done in C++ apparently fairly simply. What I don't like is the lack of debug easiness and a feel that I don't really know which object really instantiates the statement

 

myclass_teletype::myclass_teletype(HAL::UART* UART)
{
      this->UART = UART;
}

 

So when you use the

 

UART->anyUART_API()  // same as an UART->myISR() inside the Int Vector

one  must always refer to the constructor to understand which UART of which MCU/HAL, also assuming no one some how override the API function (wrong word for sure, but is the one that redefines a function, and uses a base type if such redefinition does not exist - I heard some people do that), so to navigate in the code you need to step in debug and is impossible to check it through an editor like Sublime or the like. This is self documenting on the high level, but not on the lower details. Is nasty to read in my opinion, but the C++ guru love that: I am (still) ignorant in C++, so my opinion does not really matter on C++.

 

So a 8 bit timer can be used to do the same with an 16bit one. But if more precision is needed, specific subset functions for the 16 bit timer are used.

 

Either case, I think I reached a point in which I need to shape the structure of the firmware, maybe simplifying a bit the layering, at least for the ISR.

Last Edited: Tue. Oct 8, 2019 - 09:11 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I need to shape the structure of the firmware, maybe simplifying a bit the layering

By now you could have probably already written 10 UART routines.   Where is the savings?  How big is the headache?  Also, you have to verify your code will work under all future situations to prevent more debugging headaches later on.

 

With that said, once you get it working & fully tested, please post a copy. angel

When in the dark remember-the future looks brighter than ever.   I look forward to being able to predict the future!

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

You can certainly do a thing where you "register" interrupts and arguments for them at runtime, and your ISRs have fixed code that looks a bit like:

 

ISR(USART0_RX_vect) {
    myuarts[0].rxCallBack(myuarts[0]);
}
ISR(USART1_RX_vect) {
    myuarts[1].rxCallBack(myuarts[1]);
}

registerUartISR(int8_t phycicalUartNum, myUartStruc *logicalUart,
                void (*rxisr)(), void (*txisr)());
{
        // blah, blah, blah.
}

int main() {
    registerUartISR(0, Console, charrxfifo, chartxfifo);
    registerUartISR(1, PPPintf, PPPrxSM, PPPtxSM);
}

 

It's just ... particularly expensive on the slower chips where you most need it not to be expensive :-(

On an ARM, where you can put the ISR vectors in RAM, and more of the ISR overhead is "fixed", and there's better support for pointers, it would probably be "not too bad."

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 1

My last post to any code (if anyone dislikes me posting these, just say so)-

https://godbolt.org/z/OuZeRx

 

The 4 usart isr's (x3) are grouped into a function pointer array, and the usart class handles any need to enable/set the function pointer to its isr code that gets/puts from its own buffer. Its all transparent to the user as the usart class handles everything. This code was just meant to test ideas, so is not complete or even necessarily correct or good. The compiler explorer web page is a nice place to test ideas (I think with a little work, you can also set it up to run on your own pc with its own set of compilers).

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Interesting.  I was thinking of mentioning that a bit of inline asm and some C++ templates might do a reasonable job.

 

However:

ldi r2, 2

Is not a legal instruction :-(

 

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

>Is not a legal instruction :-(

 

Its been a while, but I guess its like dealing with an M0, and not a mips. I should stick to read-only asm mode.

 

Change the r2 to r16. It does not appear the isr prologue will touch it as its a 'call saved' register, but I don't know for a fact.

https://godbolt.org/z/Iq1-j2

slowly changes

https://godbolt.org/z/k1OWqw

 

Last Edited: Wed. Oct 9, 2019 - 06:36 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Looks similar to the concept I wanted, but would make possible to declare such callbacks as static? Inline is not possible here, I believe.
But if static, here the other question: I should write all the ISR() etc in the uart.c context, not in a generic ISR HAL, ain't I?

 

Regarding the ARM, you say so because they are "powerful" or really the situation gets optimized with this approach?

Last Edited: Wed. Oct 9, 2019 - 06:52 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

curtvm wrote:

 

https://godbolt.org/z/OuZeRx

 

 

Not for the code (I just took a quick look), but for the on-line editor and the asm with the highlight feature: it is awesome!

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

If you are referring to the personal project: I already said before why I am doing this :)
(and I have a fully working and of course nothing tested "traditional" code with none of this overhead... not even with the ISR callback - I start questioning and changing a bit my programming approach when I started to play with ARMs and widening a bit the horizon)

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

 I already said before why I am doing this :)

Fair enough...it's certainly a learning experience...maybe will lead to the ultimate breakthrough!!   Keep climbing the mountain of knowledge.

When in the dark remember-the future looks brighter than ever.   I look forward to being able to predict the future!

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

avrcandies wrote:

maybe will lead to the ultimate breakthrough!! 

i.e. the re-invention of C++

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Since no objections yet, here is a straight-forward non-template/non-static Uart class version/modification-

https://godbolt.org/z/iar_Y5

 

Its takes some extra work to deal with member pointers, and the code size may end up being about the same as the static version. There are trade offs whichever direction you take- if you use a non-static class and member function pointers, any use of templates starts to get complicated as the template starts to 'infect' everything you touch. If you use templates, then everything becomes a different type (and maybe may as well use static to make some things easier) and you leave it up to the compiler to figure out what code is common and let it optimize whatever it can.

 

I'm not sure if I have the member function pointers correct, but I started simple and could figure out what the assembly was doing and it made sense. But as you start adding more it soon gets to a point that one cannot easily see what is going on and 8bits makes it worse as any significant pointer usage clutters everything. That is where 32bits starts to shine- it has an easy time dealing with pointers.

 

FYI- the Compiler Explorer is quite simple to get running on a pc. I had a spare Linux pc to test it out (spare pc's are good to do experimental things). Git cloned the github repository, downloaded/extracted nodejs to /usr/local, then in the cloned dir, just run make (you run make every time to use), it will find your compilers (that are in common places) and start a local server. You can edit or add a file to override/change the compilers. I had xc8 and xc32 installed, so added them to a config file where they can now be chosen.

 

I think there are YT videos about Compiler Explorer, just search Matt Godbolt or Compiler Explorer.

 

I can imagine at some point ide's will start getting in on the act, because it is a pretty useful tool.

 

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

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 ?

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

>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 ?

 

You are looking at the wrong tree in the forest. Or something. Its an incomplete example sitting on some server in the cloud, done by a non-professional, and will simply vanish into wherever 1's and 0's disappear to (thin air, I guess).

 

The minor problems can easily be fixed in various ways-

static constexpr volatile uint8_t* USARTN_BASE[4]{ &UCSR0A, &UCSR1A, &UCSR2A, &UCSR3A };
...
//explain here that there are 3 vectors per uart, and we
//need a base index into a Usart member pointer array
//Usart0 uses 0-2, Usart1 3-5, etc., so n*3 becomes the base index
//base index = RX vector, base index+1 = UDRE, +2 = TX
 m_base_vect(n*3)
,m_base(USARTN_BASE[n])

The idea behind grouping the usart irq's (group of 12 in this case, 3 for each usart), was to prevent creating a function pointer array for 57 interrupts, many of which would never be used. The Usart.cpp file would deal with usart isr's as it sees fit. The next peripheral class can do what it wants with its isr's- the same type of thing, or the old fashioned way, doesn't matter to the usart code.

 

 

>Actually are you really supposed to do arithmetic on enums ?

 

I'm sure you already know, but for others-

https://en.cppreference.com/w/cpp/language/enum

https://en.cppreference.com/w/c/language/enum

https://en.cppreference.com/w/c/language/operator_arithmetic

 

they are basically constants of its underlying type so yes, you can do arithmetic with them. In c++, they also make a great way to validate an argument at compile time. For example, the USARTN in the example contains the available usart numbers 0-3, and trying to instantiate the Usart class with anything other than what is in USARTN will fail at compile time. So no need to validate/assert that a valid usart number was passed in (unless you are intentionally looking for trouble and start casting). I use enum's quite a bit, and they work quite well.

 

An enum, like a constant, cannot be assigned a value after its initialization-

enum MYENUMS { A, B, C, D };

int a = A; //ok

int b = A * 100 ^ B / C % D >> 2; //ok

int c = A++; //no, there is no ++ for a constant/enum

for( MYENUM m = A; m <= D; m++ ); //no ++

for( MYENUM m = A; m <= D; m = MYENUM(m+1) ); //ok, conversion from int m + 1, initialized with MYENUM(), then assigned to m (c++11)

//but now you better know what is in the enum if using it for more than just its raw value

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

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!

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

It does a lot of sense, it's n = BLUE.

:D

Last Edited: Thu. Oct 10, 2019 - 05:29 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

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

 

This doesn't make sense, either-

  int const AMBER = 1;

  int const REVERSE = 2;

  int n = AMBER * REVERSE;

which I assume doesn't stop you from using a const.

 

 

I'm not sure what is hard to understand. Its a constant value with extra features.

The USARTN values becomes an index for several uses, in addition to being a specific type that the compiler enforces.

 

With just the usart number as an index, we can get the base address, and calculate the vector array offset needed for isr function pointers. Use a non-enum value type as the Usart constructor argument (like a uint8_t) and you now have to validate its value, and still come up with the other needed values based on which usart you wanted. If you want to store the usart number (n) instead of computing the base vector offset (n*3), that could also be done but makes little difference and you just moved its computation to a later time [n*3+RX_VECT], where RX_VECT is also an enum (0). 

 

enum USARTN : uint8_t { USART0, USART1, USART2, USART3 };
static constexpr volatile uint8_t* USARTN_BASE[4]{ 
    &UCSR0A, &UCSR1A, &UCSR2A, &UCSR3A 
};

 

struct Usart {

    Usart(USARTN n)
    : m_base_vect(n*3), m_base(USART_BASE[n])
    {}

    //...

    void somefunc(){ 
        isr_funcs[m_base_vect+RX_VECT] = &Usart::rxISR; 
    }

    //...

    const uint8_t m_base_vect;
    volatile uint8_t* const m_base;

};

 

Usart u0(USART0); //ok

Usart u1(1); //not ok

 

 

 

In case anyone still reading this, I made this change-

https://godbolt.org/z/jDigEP

 

The Usart pointer array (of 4) is enough info for the isr to figure things out, and there is no need for the member function array. That eliminates the 12*4 byte member pointer array, and also gets rid of the need to keep the vector offset value in the class. The only job left for the Usart class is to initialize the global Usart pointer array, which it does in its constructor. 

 

The isr stubs now encode its value (via r16) with both the Usart pointer array index (0-3) and also the vector type (0-2 <<2). The actual working usart isr function now only needs to lookup the Usart pointer from the array and determine which function to call by the vector type passed in.

 

None of this is probably very useful, except as a learning exercise where ideas can be worked out that hopefully apply in some way in others areas (or at least figure out what not to do, or when not to do). The bigger point in my opinion, is that c++ is the right tool for doing c++. The code I linked to is one page of code (although missing some details in using the uart), can handle any of the 4 usarts, has a generic buffer class that anyone can use, has a Printf class anyone can use (just need your own putch function), and there is no need for the 'user' to deal with the isr's. Just create an instance of the usart class, and start using it.

Last Edited: Sat. Oct 12, 2019 - 01:02 AM