AVR Freaks

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.

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:

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?

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();
	}
}

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.

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.

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.

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?

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)

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

And I definitely need to understand better the assembly. It is not what happens only in the ISR which worries me (because this might be optimized on ARMs), but the overhead of instructions I get when abstracting the registers. I am not sure this can change if changing MCU architecture.

For example, when writing to TCNT0 (line 00000B2D), via the array of registers gives a lot more indexing instructions rather than the following direct assignment (at line 00000B37 )

    *tcnt[handle->channel] = val & 0xff;
00000B2D  LD R30,X        Load indirect
00000B2E  LDI R31,0x00        Load immediate
00000B2F  LSL R30        Logical Shift Left
00000B30  ROL R31        Rotate Left Through Carry
00000B31  SUBI R30,0x76        Subtract immediate
00000B32  SBCI R31,0xFE        Subtract immediate with carry
00000B33  LD R0,Z+        Load indirect and postincrement
00000B34  LDD R31,Z+0        Load indirect with displacement
00000B35  MOV R30,R0        Copy register
00000B36  STD Z+0,R22        Store indirect with displacement
    TCNT0 = val;
00000B37  OUT 0x26,R22        Out to I/O location 

I wonder if there is a way to let the compiler natively optimize or writing some inline code to reduce the instructions while keeping the abstraction.
Or probably I am asking for something impossible...
 

Regarding the fact register preservation gets lost, I am not sure I understood what you meant.

To try understand, I made the experiment with the task tick increment, which is a variable in the scope of the "scheduler" application.
The ISR with no callback is:
 

ISR(TIMER1_COMPA_vect)
{
  task_systick_increment_ISR();
}

And brings this disassembly:
 

{
00000256  PUSH R1		Push register on stack
00000257  PUSH R0		Push register on stack
00000258  IN R0,0x3F		In from I/O location
00000259  PUSH R0		Push register on stack
0000025A  CLR R1		Clear Register
0000025B  PUSH R18		Push register on stack
0000025C  PUSH R19		Push register on stack
0000025D  PUSH R20		Push register on stack
0000025E  PUSH R21		Push register on stack
0000025F  PUSH R22		Push register on stack
00000260  PUSH R23		Push register on stack
00000261  PUSH R24		Push register on stack
00000262  PUSH R25		Push register on stack
00000263  PUSH R26		Push register on stack
00000264  PUSH R27		Push register on stack
00000265  PUSH R30		Push register on stack
00000266  PUSH R31		Push register on stack
task_systick_increment_ISR();
00000267  CALL 0x00000CD2		Call subroutine
}
00000269  POP R31		Pop register from stack
0000026A  POP R30		Pop register from stack
--- hal/atmega328/interrupt_hal.c
0000026B  POP R27		Pop register from stack
0000026C  POP R26		Pop register from stack
0000026D  POP R25		Pop register from stack
0000026E  POP R24		Pop register from stack
0000026F  POP R23		Pop register from stack
00000270  POP R22		Pop register from stack
00000271  POP R21		Pop register from stack
00000272  POP R20		Pop register from stack
00000273  POP R19		Pop register from stack
00000274  POP R18		Pop register from stack
00000275  POP R0		Pop register from stack
00000276  OUT 0x3F,R0		Out to I/O location
00000277  POP R0		Pop register from stack
00000278  POP R1		Pop register from stack
00000279  RETI 		Interrupt return
{

But with the callback:
 

ISR(TIMER1_COMPA_vect)
{
	isr_pt_array[TIMER1_COMPA_vect_num].isr_pt();
}

Generates:

{
00000256  PUSH R1		Push register on stack
00000257  PUSH R0		Push register on stack
00000258  IN R0,0x3F		In from I/O location
00000259  PUSH R0		Push register on stack
0000025A  CLR R1		Clear Register
0000025B  PUSH R18		Push register on stack
0000025C  PUSH R19		Push register on stack
0000025D  PUSH R20		Push register on stack
0000025E  PUSH R21		Push register on stack
0000025F  PUSH R22		Push register on stack
00000260  PUSH R23		Push register on stack
00000261  PUSH R24		Push register on stack
00000262  PUSH R25		Push register on stack
00000263  PUSH R26		Push register on stack
00000264  PUSH R27		Push register on stack
00000265  PUSH R30		Push register on stack
00000266  PUSH R31		Push register on stack
	isr_pt_array[TIMER1_COMPA_vect_num].isr_pt();
00000267  LDS R30,0x030F		Load direct from data space
00000269  LDS R31,0x0310		Load direct from data space
0000026B  ICALL 		Indirect call to (Z)
--- user/hal/atmega328/interrupt_hal.c
}
0000026C  POP R31		Pop register from stack
0000026D  POP R30		Pop register from stack
0000026E  POP R27		Pop register from stack
0000026F  POP R26		Pop register from stack
00000270  POP R25		Pop register from stack
00000271  POP R24		Pop register from stack
00000272  POP R23		Pop register from stack
00000273  POP R22		Pop register from stack
00000274  POP R21		Pop register from stack
00000275  POP R20		Pop register from stack
00000276  POP R19		Pop register from stack
00000277  POP R18		Pop register from stack
00000278  POP R0		Pop register from stack
00000279  OUT 0x3F,R0		Out to I/O location
0000027A  POP R0		Pop register from stack
0000027B  POP R1		Pop register from stack
0000027C  RETI 		Interrupt return 

In the first case, the direct ISR is called with

CALL 0x00000CD2	

While with the callback, is
 

00000267  LDS R30,0x030F		Load direct from data space
00000269  LDS R31,0x0310		Load direct from data space
0000026B  ICALL 		Indirect call to (Z) 

With the expected overhead discussed before because is a function call, it seems to save the stack exactly in both cases. It is even documented in FreeRTOS documentation this behaviour.

So what is the data which is not preserved you are referring to?

Just for reference, putting the increment directly in the ISR (which normally is not done in a RTOS):

00000256  PUSH R1		Push register on stack
00000257  PUSH R0		Push register on stack
00000258  IN R0,0x3F		In from I/O location
00000259  PUSH R0		Push register on stack
0000025A  CLR R1		Clear Register
0000025B  PUSH R24		Push register on stack
0000025C  PUSH R25		Push register on stack
0000025D  PUSH R26		Push register on stack
0000025E  PUSH R27		Push register on stack
task_system_tick+=1000;
0000025F  LDS R24,0x01BF		Load direct from data space
00000261  LDS R25,0x01C0		Load direct from data space
00000263  LDS R26,0x01C1		Load direct from data space
00000265  LDS R27,0x01C2		Load direct from data space
00000267  SUBI R24,0x18		Subtract immediate
00000268  SBCI R25,0xFC		Subtract immediate with carry
00000269  SBCI R26,0xFF		Subtract immediate with carry
0000026A  SBCI R27,0xFF		Subtract immediate with carry
0000026B  STS 0x01BF,R24		Store direct to data space
0000026D  STS 0x01C0,R25		Store direct to data space
0000026F  STS 0x01C1,R26		Store direct to data space
--- hal/atmega328/interrupt_hal.c
00000271  STS 0x01C2,R27		Store direct to data space
}
00000273  POP R27		Pop register from stack
00000274  POP R26		Pop register from stack
00000275  POP R25		Pop register from stack
00000276  POP R24		Pop register from stack
00000277  POP R0		Pop register from stack
00000278  OUT 0x3F,R0		Out to I/O location
00000279  POP R0		Pop register from stack
0000027A  POP R1		Pop register from stack
0000027B  RETI 		Interrupt return 

Essentially save fewer registers because there is no need to address any pointer or call.
In all the cases, with the proper declaration, task_system_tick does not lose its value on the next call.

Or did you refer to something else?

I read that as the register preservation gets lost (out of the roof, out of the window, in the trash bin). Reading again now I guess you meant that gets very big, ain't you?

LOL - my mistake then. In this case, indeed makes sense, when comparing the ISRs.