C++ gpio class inheritance problems

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

I'm trying out c++ and I can't figure this one out.

 

I'm trying to create a driver for the MAX7219 with classes. The MAX7219 inherits a SPI class and a  GPIO class, but the GPIO class is causing a lot of problems by using indirect instructions rather the SBI/CBI instructions, and even worse is that it sometimes calls the gpio functions rather inline them.

 

Without the MAX7219 being created the GPIO class works as intended, but once it's created everything goes out the window. Somehow the first gpio function is able to work with SBI/CBI after the GPIO object is created though.

 

I'm really not sure what I'm doing to cause this issue.

 

Some sample dissassembly code I'm getting:

		 GPIO(volatile uint8_t *port, const uint8_t pins) : port(port), pins(pins)
00000059  LDI R24,0x25        Load immediate 
0000005A  LDI R25,0x00        Load immediate 
0000005B  STD Y+24,R25        Store indirect with displacement 
0000005C  STD Y+23,R24        Store indirect with displacement 
0000005D  LDI R24,0x24        Load immediate 
0000005E  LDI R25,0x00        Load immediate 
0000005F  STD Y+26,R25        Store indirect with displacement 
00000060  STD Y+25,R24        Store indirect with displacement 
00000061  LDI R24,0x23        Load immediate 
00000062  LDI R25,0x00        Load immediate 
00000063  STD Y+28,R25        Store indirect with displacement 
00000064  STD Y+27,R24        Store indirect with displacement 
00000065  LDI R24,0x02        Load immediate 
00000066  STD Y+29,R24        Store indirect with displacement 
        *(pin) |= pins;
00000067  SBI 0x03,1        Set bit in I/O register 
        *(port-1) |= pins;
00000068  LDD R30,Y+23        Load indirect with displacement 
00000069  LDD R31,Y+24        Load indirect with displacement 
0000006A  LD R25,-Z        Load indirect and predecrement 
0000006B  LDD R24,Y+29        Load indirect with displacement 
0000006C  OR R24,R25        Logical OR 
0000006D  STD Z+0,R24        Store indirect with displacement 
        *(port) &=~ pins;
0000006E  LDD R30,Y+23        Load indirect with displacement 
0000006F  LDD R31,Y+24        Load indirect with displacement 
00000070  LDD R25,Z+0        Load indirect with displacement 
00000071  LDD R24,Y+29        Load indirect with displacement 
00000072  COM R24        One's complement 
00000073  AND R24,R25        Logical AND 
00000074  STD Z+0,R24        Store indirect with displacement 
        *(pin) |= pins;
00000075  LDD R30,Y+27        Load indirect with displacement 
00000076  LDD R31,Y+28        Load indirect with displacement 
00000077  LDD R25,Z+0        Load indirect with displacement 
00000078  LDD R24,Y+29        Load indirect with displacement 
00000079  OR R24,R25        Logical OR 
0000007A  STD Z+0,R24        Store indirect with displacement 

 

Attachment(s): 

This topic has a solution.
Last Edited: Sun. Mar 1, 2020 - 11:26 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

How can something that is only given the port and bit number at run time possibly encode an SBI/CBI. If you want SBI/CBI efficiency you need to know the port and bits at compile time. I've seen some great C++ template based solutions for this that always seem to generate the SBI/CBI you are looking for. (trying to find them again is another question of course!!)

 

EDIT: it's just vaguely possible this may have been the thread I was thinking of:  https://www.avrfreaks.net/forum/c-template-specialization-sfr

Last Edited: Fri. Feb 21, 2020 - 10:17 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Its quite a minefield to navigate to get the cbi/sbi to always show up. It can be done but it takes some work.

 

Here is your code stripped down, and I can make the compiler easily go from sbi/cbi to ugly. In the first example, simply having the class function created outside its header is enough to generate the extra code- if the code is in the header (it is implicitly 'inline') OR the function outside the header is specified as inline then you get the cbi/sbi in this example. The keyword 'inline' is a bit of a multi-purpose name that may have more meaning for the linker than anything else, and may not have the meaning it appears to have even if the results gives the appearance its what we think it is (read that sentence a few times- its as confusing as trying to figure out what inline really means). 

https://godbolt.org/z/MmT9oj

 

In the second example changing the gpio class instance from local to global does the damage. Could be related to the above example in some way, but I'm sure there are another half dozen ways to transition from optimized cbi/sbi to actual class code.

https://godbolt.org/z/DZjsbC

 

 

I use templates for avr0/1, and can get cbi/sbi every time. For 32bit, its most likely not worth the trouble to enter template world as 32bit does pointers with ease and may end up looking better without (see below).

 

https://godbolt.org/z/eXp-yT

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

As mentioned in #2, it all comes down to the compiler knowing the arguments at compile time with absolute certainty. Probably giving it the -flto optimization switch will allow it to know more, by fusing different compile units together and allowing more inlining.

 

One way to be sure is by passing template arguments instead of function arguments. But, as we know, SFR addresses such as &PORTB are invalid template arguments according to the standard, though older versions of gcc will just ignore this and compile anyway.

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

Thanks for all the help.

 

I was avoiding templates because I find them too confusing to understand. But after reading the article below and curtvm's template code for ARM I understand it a bit better now. As a sidenote, the article below uses the AVR0/1 series. 

 

https://www.tablix.org/~avian/blog/archives/2019/10/specializing_c_templates_on_avr_pins/

 

Essentially templates can't accept pointers and thus you have to do some casting to trick the template.

 

I've updated my code for the gpio.

#pragma once

#include <stddef.h>
#include <stdint.h>
#include <avr/io.h>

static const int PORTB_ADDR = (int) &PORTB; 
static const int PORTC_ADDR = (int) &PORTC; 
static const int PORTD_ADDR = (int) &PORTD;
 
template <uint16_t port_addr, uint8_t  pins> 
class GPIO {
	
	public:
		
	void setOutput(void){ *(volatile uint8_t*)(port_addr-1) |= pins; }		
	void setPullUp(void){ *(volatile uint8_t*)(port_addr-1) &=~ pins; *(volatile uint8_t*)(port_addr) |= pins; }		
	void on(void){ *(volatile uint8_t*)port_addr |= pins; }
	void off(void){	*(volatile uint8_t*)port_addr &=~ pins; }		
	void toggle(void){ *(volatile uint8_t*)(port_addr-2) |= pins; }	
};

This works as expected, but now my problem is that I don't know/understand the syntax for allowing the MAX class to inherit the template class. I've read a few different sources and tutorials, but I can't make heads or tails of it. What I've done so far is:


//class declaration
class MAX7219 : public SPI, public GPIO<uint16_t,uint8_t>

//class constructors
MAX7219(SPI& spi, GPIO<uint16_t,uint8_t>);

int main(void)
{
	
	SPI spi;
	spi.init();
	
	GPIO<PORTB_ADDR,2> select;
	select.setOutput();
	select.on();
		
	MAX7219 display0(spi,select);
	 while (1) 
	{};
}

I keep getting errors related to mismatching. 

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

Consider making the derived class a template too that passes it into the template below.

 

When you instantiate the MAX class you probably want to specify ports/pins anyway to say where it's located.

Last Edited: Sat. Feb 22, 2020 - 09:59 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Ultimately, is having the most 'optimal' code going to make a difference to what is ostensibly a slow operation anyways?  When I was transitioning from asm to C, I used to worry about such things. Nowadays a few microseconds here or there is probably not an issue.

 

Nevertheless, it is an interesting technical issue ensuring the compiler does what you want.

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

Cyclonus101 wrote:

https://www.tablix.org/~avian/blog/archives/2019/10/specializing_c_templates_on_avr_pins/

 

Essentially templates can't accept pointers and thus you have to do some casting to trick the template.

 

Well, that trick works, but you are forced to create a new intermediate variable for each SFR address you need to pass to a template.

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

>Essentially templates can't accept pointers and thus you have to do some casting to trick the template.

 

There is no law that states you have to use existing defines. Use alternate things like enums, which take a minute to create. Don't let a few defines you had no part in writing prevent you from doing something better.

 

Here is another minimal example-

https://godbolt.org/z/UZ2gM5

 

Once you start down the template road, templates start propagating everywhere because you cannot 'get out' once inside.

 

I would consider what Kartman said- it may not be worth the trouble, and not necessary to get everything down to minimal instructions. If you don't like the asm code, quit looking at it. If the code is reliable, and fits in the flash, its probably ok. Arduino is an example- I haven't looked, but I'm sure it looks horrible to anyone who knows how to read avr asm, but it works and most could care less what goes on under the hood. If it works = happy.

 

Here is my avr0/1 port code-

https://github.com/cv007/Avr0PlusPlus/blob/master/Port.hpp

and the 'normal' code for a pic32mm (each peripheral one hpp, one cpp, no templates)-

https://github.com/cv007/PIC32MM_Curiosity_CPP/blob/master/Pins.hpp

 

With a 32bit mcu, templates don't help much for code size, and can actually make things worse (the nature of 32bit instruction set). I have a stm32g01 nucleo board I was playing around with, created a port/gpio class similar to my other template code, and setup to display a single 7seg led. Then I started over and made the code 'normal' c++ without templates. The non-template code was about 100 bytes less and was quite a bit easier to create and deal with.

 

When I wrote the avr0/1 c++ code, I saw the asm code and got scared so then went to templates. It all works fine, but can be a little bit of a pain to get to where you want to be. I'm slowly starting over for the avr0/1 without using templates and will try to see how it compares to the template version. You have to get a ways into it before you can compare, though.

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

curtvm wrote:
. Don't let a few defines you had no part in writing prevent you from doing something better.
One option is to use ATDF and a smattering of Python to generate your own "device headers" in whatever form suits your own implementation. 

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

I appreciate everyone's comments, this template stuff is just killing me though. curt your example would is great, except I still need the MAX class to inherit spi and I don't the know syntax for making that work. More specifically I need to know the syntax for the constructor and initialize in main.  I've looked at many different sources to understand what I need to do, but every example fails to provide any insight. I think I'm just misunderstanding how all of this template inheritance stuff is suppose to work.

 

There are a few workarounds, but I was trying to stick to the coding principle that all the peripheral dependencies would be hidden by the class functions. Obviously the easy way is just to forget about inheriting a GPIO class and toggling the SS pin for each device in the main loop. Or the brute force way of creating a gpio class with each value for the port and pin hardcoded and then just let the spi device inherit the specific gpio class. 

 

 

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

https://godbolt.org/z/bPQWcC

(not quite correct, as putting the load pin into the spi class)

 

another, using ss pin (free to use as output in master-only mode) instead of creating our own for the LOAD pin-

https://godbolt.org/z/LmN3pH

and spi class remains more generic

 

and a variation which can specify the LOAD pin (which could also be the ss pin, would just get init twice- no harm)-

https://godbolt.org/z/H8Sghb

 

 

You do not have to use inheritance, and the first and third example shows versions for the MAX class (spi). I would rather see a spi_.write than a this->write (you need the 'this' in this case), or in the second case make sure your local function names do not clash with inherited names.  Sometimes inheritance is the right thing to do, sometimes its better to just put the 'inherited' class in as a private member with a name. 

 

The above example would let you deal with the MAX class only (specifying single pin), and the underlying spi would be taken care of the rest which is what we are after in c++. Or the seconds just uses the free ss pin and some of the problems go away.

 

 

Last Edited: Mon. Feb 24, 2020 - 06:57 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

You can also create GPIO interface (with virtual methods - dynamic dispatching is needed) and inherit from it, so you can pass child object into reference to parent class (=interface).

 

Or you can pass simple function pointer that toggles correct pin (= callback). It should be even possible to pass lambda function into simple function pointer, as far as you don't use captures.

 

However, I don't see any point in trying to get SBI/CBI, as far as the call overhead is much more.

At least you can avoid Read-Modify-Write by toggling pins by using PINx = _BV(pin) port instead of PORTx |= _BV(pin) and PORTx &= ~_BV(pin).

Computers don't make errors - What they do they do on purpose.

Last Edited: Mon. Feb 24, 2020 - 10:12 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

clawson wrote:
curtvm wrote:
. Don't let a few defines you had no part in writing prevent you from doing something better.
One option is to use ATDF and a smattering of Python to generate your own "device headers" in whatever form suits your own implementation.
IIRC you did just that: wrote a python script to turn the SFRs into a giant struct.

Templates do accept offsets.

 

Another possibility is to use named constants as the template parameters and to test them with a custom version of static_assert.

// Needs to be invoked in a function that will not be optimized away by the compiler.
// The linker may remove it.
// Optimization needs to be high enough to turn the argument into a compile-time constant.
// Providing same is left as an exercise for the reader.
#define static_assert_custom(ok) \
                      if(!(ok)) asm(" .error \"" #ok "\" is zero.")

Not my design.  'Twas easier to remember than to search for.

I think I got the qquotes right.

The argument need not be any kind of syntactic constant expression to pass the test.

Iluvatar is the better part of Valar.

Last Edited: Mon. Feb 24, 2020 - 09:15 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Many thanks to curtvm, your examples are great. I have one more piece of code I want you guys to look at because it gets pretty close to what I wanted. Based off this stackoverflower answer I came up with:

 

class GPIO
{
	constexpr static volatile uint8_t *const PORTb() { return &PORTB; }
	constexpr static volatile uint8_t *const DDRb() { return &DDRB; }
	constexpr static volatile uint8_t *const PINb() { return &PINB; }
	constexpr static volatile uint8_t *const PORTc() { return &PORTC; }
	constexpr static volatile uint8_t *const DDRc() { return &DDRC; }
	constexpr static volatile uint8_t *const PINc() { return &PINC; }
	constexpr static volatile uint8_t *const PORTd() { return &PORTD; }
	constexpr static volatile uint8_t *const DDRd() { return &DDRD; }
	constexpr static volatile uint8_t *const PINd() { return &PIND; }	
	
	public:	
	enum PORTS_ : uint8_t {B,C,D};
	enum PINS_ : uint8_t  {P0=1,P1=2,P2=4,P3=8,P4=16,P5=32,P6=64,P7=128};
		
	GPIO(PORTS_ port, PINS_ pin) : port_(port), pin_(pin)
	{};
			
	const enum PORTS_ port_;
	const enum PINS_  pin_;
	
	void setOutput() 
	{ 		
		switch (port_)
		{
			case B: *DDRb() |= pin_; break;
			case C: *DDRc() |= pin_; break;
			case D: *DDRd() |= pin_; break;
		}		
				
	}

	void on()
	{	
		switch (port_)
		{
		
		case B: *PORTb() |= pin_; break;
		case C: *PORTc() |= pin_; break;
		case D: *PORTd() |= pin_; break;
		}
	}
	
	void off()
	{
		switch (port_)
		{
			case B: *PORTb() &= ~pin_; break;
			case C: *PORTc() &= ~pin_; break;
			case D: *PORTd() &= ~pin_; break;
		}
	}
	
};

 

The assembly code gets pretty close, except with inheritance it will actually create the lookup code. 

 

    void off()
MOVW R30,R24        
        switch (port_)
LDD R24,Z+0        
CPI R24,0x01       
BREQ PC+0x0B      
BRCS PC+0x04        
CPI R24,0x02     
BREQ PC+0x0E       
RET        
            case B: *PORTb() &= ~pin_; break;
IN R25,0x05        
LDD R24,Z+1      
COM R24        
AND R24,R25   
OUT 0x05,R24       
RET         
            case C: *PORTc() &= ~pin_; break;
IN R25,0x08        
LDD R24,Z+1     
COM R24         
AND R24,R25        
OUT 0x08,R24        
RET         
            case D: *PORTd() &= ~pin_; break;
IN R25,0x0B        
LDD R24,Z+1       
COM R24        
AND R24,R25      
OUT 0x0B,R24        
RET         

 

This reply has been marked as the solution. 
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

> it gets pretty close to what I wanted

 

You will not get to where you want (or what I think you want) without templates. You have storage for port_ and pin_, and if the compiler cannot deal with crossing a scope or whatever it is thinking at a particular moment, then the vars need storage and you get all the extra code that is then needed. Making a global pin for example is all that is required to step into a state where the vars need to be stored.

 

You will end up with some pins as you wish and others producing the 'more code' versions and you will have a hard time knowing what the result may be. With templates, all the information is carried in the resulting type and is always known. So, you live with one or the other depending on how much you care, or if you want to avoid templates.

 

A little rearranging of your code (well, I gutted it), with a template-

https://godbolt.org/z/ZwwKsH

You have to create the pin enum, so just go the little extra to put them all in the one enum where you can deduce the port later. Passing a single pin template parameter is better than having to specify both port and pin, plus you the eliminate using a non-existing pin (like PC7).

 

edit- another variation (there are many ways to do the same thing, so pick something you like and make it yours)

https://godbolt.org/z/MSazZ6

(not sure why the online compiler is allotting one bss byte for each gpio pin used, maybe it normally gets cleaned up in a later linker stage as I never see these bytes otherwise)

 

>I'm slowly starting over for the avr0/1 without using templates and will try to see how it compares to the template version. You have to get a ways into it before you can compare, though.

 

I went a little ways in, and found out all the things I was giving up by not using templates so gave up and will continue to use templates for the avr. One simple example was setting pins in an interrupt to set an led display (7 or 14 segment), with the template version you get the cbi/sbi and without you get quite a mess in comparison. In the big scheme of things, it probably still would not matter much but once you see the difference its hard to give it up. With a smaller avr0/1, it also helps with code size although there is a solution to that- get a bigger avr.

 

 

 

Last Edited: Sun. Mar 1, 2020 - 09:17 PM