avr-c++ virtual functions don't get called

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

Hi All

 

I'm coding my first C++ project on Microchip(AVR) Studion 7.0.2542. I'm using ATmega644

 

I have the following problem. The project builds without errors and warnings. But when I upload it to the device nothing happens. An LED must blink but it doesn't. I know the problem is in the virtual functions because when I make the virtual function, where the blinking code is, a non-virtual and move the code to the base class function - it works. After I discovered this I changed the code so that other virtual functions are no longer virtual and moved their functionalities in their respective base classes those functionalities started working.

 

So this is how the issue looks like:

 

//+++ BaseClass.h +++
class CProcess
{
           public:
		CProcess();
		virtual ~CProcess();
		virtual void InterruptTimer1CompATriggers(){};
};
//+++ DerivedClass.h +++
#include "BaseClass.h"

class CIndicator : public CProcess
{
	public:
		CIndicator();
		void InterruptTimer1CompATriggers() override;
};
//+++ DerivedClass.cpp +++
// Correct initialization of the Tmer/Counter1 here ...
void CIndicator::InterruptTimer1CompATriggers()
{
	gpioPinsRegC.bPin7 = true; // Blink a LED on port C, pin 7
}
//+++ main.cpp +++
#include <avr/interrupt.h>
#include "DerivedClass.h"

static CIndicator* g_ptr = NULL;
ISR(TIMER1_COMPA_vect) // This way id DOES NOT work!
{
//	gpioPinsRegC.bPin7 = true;
	g_ptr->InterruptTimer1CompATriggers();
}

int main(void)
{
    CIndicator var; // Timer setup in the constructor
    g_ptr = &var; // The interrupts are disabled here so the interrupt vector cannot be called before the g_ptr points to the actual object.
    sei(); // Enable interrupts.
    while(1)
    {
    }
}
//+++++++++++++ If I do this it DOES work +++++++++

ISR(TIMER1_COMPA_vect)
{
	gpioPinsRegC.bPin7 = true; // Blinking directly here. It does work.
//	g_ptr->InterruptTimer1CompATriggers();
}
// ************** BUT MORE IMPORTANTLY IF I DO THIS IT ALSO WORKS! ********************
class CProcess
{
	public:
		CProcess();
		virtual ~CProcess();
		/*virtual*/ void InterruptTimer1CompATriggers(){ gpioPinsRegC.bPin7 = true; }; // Blinking now in the base class
};
//+++ DerivedClass.h +++
#include "BaseClass.h"
class CIndicator : public CProcess
{
	public:
		CIndicator();
//		void InterruptTimer1CompATriggers() override;
};
+++ DerivedClass.cpp +++
// Correct initialization of the Tmer/Counter1 ...

//+++ main.cpp +++
#include <avr/interrupt.h>
#include "DerivedClass.h"
static CIndicator* g_ptr = NULL;

ISR(TIMER1_COMPA_vect)
{
	g_ptr->InterruptTimer1CompATriggers(); // The base class function is called and it does work.
}
int main(void)
{
    CIndicator var; // Timer setup in the constructor
    g_ptr = &var; // The interrupts are disabled here so the interrupt vector cannot be called before the g_ptr points to the actual object.
	sei(); // Enable interrupts.
    while(1)
    {
    }
}

As you can see if there are no virtual functions in the classes it works.

 

Also I tried to access a protected member of a base class in a virtual function of a derived class and it threw and error "This variable is not defined in this scope"

 

 

Any information you might have about this problem is greatly appreciated.

 

Thanks in advance :)

 

Attachment(s): 

This topic has a solution.
Last Edited: Tue. Jul 5, 2022 - 05:23 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Please try to post code with a reasonable amount of white space.

 

I lose the plot when I can't see stuff on the Browser screen.

Either attach a ZIP of the whole AS7.0 project or make allowances for reader's dementia.

 

David.

Last Edited: Mon. Jun 27, 2022 - 08:12 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I did what I could.

I'm going to put the code into a single cpp file and format it properly.

Thanks

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

Is CIndicator::InterruptTimer1CompATriggers() public here ?

 

//+++ DerivedClass.cpp +++
// Correct initialization of the Tmer/Counter1 here ...
void CIndicator::InterruptTimer1CompATriggers()

 

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

Yes it is.

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

Did you forget the = 0; in BaseClass.h ?

//+++ BaseClass.h +++
class CProcess
{
    public:
    CProcess();
    virtual ~CProcess();
    virtual void InterruptTimer1CompATriggers(){} = 0;
};
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I'm not sure that's a valid syntax. Or the avr c++ has it's own peculiarity. In normal C++ you cannot have {} And = 0; one must go. If you mean if I tried to have the BaseClass virtual functions be pure virtual "= 0" Yes I did. It's the same.

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

StarTraveler wrote:
I'm not sure that's a valid syntax

Oh bugger - that was a copy & paste error on my part. I meant to write:

    virtual void InterruptTimer1CompATriggers() = 0;
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Yeah it happens no worries. Well I did try it with no success. My project can deal without virtual functions for now. I changed the architecture so no virtual functions are used but I'm encountering other strange problems. I debug using a simulated MCU and my code runs with no problems. The logic has no problems but when I upload the program to the actual device - nothing, it doesn't work.

Right now I'm taking steps to make it possible for me to debug in emulation mode. There the code runs on the actual device and can be stepped through.

Last Edited: Mon. Jun 27, 2022 - 02:34 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

StarTraveler wrote:
I debug using a simulated MCU and my code runs with no problems. The logic has no problems but when I upload the program to the actual device - nothing, it doesn't work.
This suggests that the simulator and the MCU are getting different code.

'Tis possible that your hex file generation is incorrect for C++.

Moderation in all things. -- ancient proverb

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

Yes it's possible. That possibility didn't cross my mind. I'm going to try and see what code the simulator gets and what the MCU gets.

Thanks.

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

I think you are running into an optimization problem. The compiler cannot see the local class instance doing anything so seems to only create stack space for the class but no vtable is created. It also seems to not see g_ptr being used in an isr so also optimizes accordingly. 

 

simple example-

https://godbolt.org/z/zMT1vYons

 

Play with the code a little (commented out code) and watch the asm output. It can be seen when the vtable shows up.

 

I'm not sure if there is a gcc attribute or something else that will prevent the compiler from effectively optimizing away the local class instance. The simple answer is to make the class global or static so you get the vtable generated.

 

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

Well that's a bit nasty. I tried adding a non-trivial constructor to B and it got inlined but still no v-table.

 

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

I'd suggest:

 

In CProcess:
virtual void InterruptTimer1CompATriggers()=0;

In CIndicator:
virtual void InterruptTimer1CompATriggers();

C: i = "told you so";

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

You can make the (empty) B constructor noinline, and you will get the vtable-

https://godbolt.org/z/fzxver7Wx

 

Not sure what rules apply, but some optimization along the way is removing the vtable. One would think an isr function would be seen by the toolchain as using the class pointer (the non-static afunc() shows the same thing, and you can replace it with an isr to see there is also no vtable generated).

 

 

 

 

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

I’ll note that Arduino makes use of virtual functions, and doesn’t seem to have problems.

 

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

I’ll note that Arduino makes use of virtual functions, and doesn’t seem to have problems.

This happens to be an unusual combo, and outside of the code presented in the original post which is a test of virtual functions, I'm not sure how you would end up using this combo in real code, but could happen I guess. There must be a whole world of vtable optimizations that goes unseen, and this happens to hit a certain combo and reveals what seems to be a flaw. The flaw seems to be no connection is made between the class instance and the global class pointer used in an isr, so the vtable optimizes away. At least several ways to make the vtable show up, but it seems to require either the class instance creating the vtable regardless of what the global pointer is doing (like making the constructor noinline), or having the compiler see the global pointer being used while the instance is still on the stack (which it doesn't in the case of the isr use).

 

 

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

OP says it works in simulation, but not on real device.

That implies either the simulator not work so good

or that it is running different code.

The latter is a loading issue, not a language lawyer issue.

Moderation in all things. -- ancient proverb

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

I haven't analyzed the code.  Actually it confuses me.  Just a reminder that to call a virtual function you must use a pointer to the base class.  I think the value of the base class pointer is the same as the value of the derived class pointer but the types are different.

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

steve17 wrote:
Just a reminder that to call a virtual function you must use a pointer to the base class.
One can, but one need not.

That said,he ability to do so is the reason for and pretty much the definition of a virtual function.

Moderation in all things. -- ancient proverb

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

I haven't analyzed the code.  Actually it confuses me.

An even simpler example, no need for a virtual function to see a problem-

https://godbolt.org/z/f7Ts6xq9K

The instance on the stack is not initialized as the toolchain is optimizing it away since it cannot see the isr using the instance indirectly via pointer.

 

You probably never get to point where you are doing something like this, and it only shows up when doing some test as in the original post (which happens to use main, which does not return so the instance would always remain valid). Probably nice to know this can happen, but also will probably never see something like this. Its possible there is a compiler warning option to cover this, but -Wall + -Wextra does not, so probably not.

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

If optimization is the issue,

why does it work in simulation?

 

That said, I believe the compiler is allowed to do the complained of optimization.

It just didn't.

 

Are the device code and the simulator code derived from the same ELF?

Moderation in all things. -- ancient proverb

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

Sorry for the late post. I had some unrelated issues, and also I was busy making a little board with 16 LED that take all of port A and C for a running light test.

After I wrote a very simple running light program and saw that it didn't work and/or has some really strange issues I think I owe you all an apology. My entire setup is a custom one designed by me, which means there can be many problems with it. I make use of the MCP2210 USB-to-SPI adapter chip as an integrated programmer. I use a programming software I wrote myself using the API provided for the MCP2210. That all means that my hardware could be faulty, my software could be buggy, but most likely, as I found out yesterday, the entire MCP2210 platform is crap! Actually I didn't find out that yesterday, but through out my development I encountered some weird behavior from that chip, but yesterday when I was researching a fix for another problem with it, I came across an article blasting the MCP2210 concluding with the phrase: "Stay away form that chip. It's very poorly designed." Now I would add: "And it's sparsely documented if the documentation is up to date at all."

 

Now, despite the problems I'm having, I managed to bring my system to the state of: being able to get the ATmega644 into programming mode; Read and write the fuse bits; Read/write/erase the flash memory, which lets me upload and run small programs. Of course that's when it decides to work. Because often times it doesn't. Gives me this weird error: "The SPI is held by an external master." That issue is squarely MCP2210's fault. When it happens I have to wait for days to resolve itself. Yes! No restarting works, no detaching and reattaching of the device works, no restarting of the PC, no detaching restarting the PC while the device is detached and reattaching it after the PC has booted. Nothing works... I also often get the "Unrecognized USB device" warning upon plugging the device.

 

I have many extremely weird issues such as: (Meta code)

main()

{

       while(1)

       {

              PORTXn = !PORTXn. //Yes I know I can use the PIN input register to toggle while the pin is configured as output, but I wanted to use the data register since that's how you implement a running light...

              delay(...)

       }

}

The above code works. The pin toggles like this: 0, 1, 0, 1... What if I want the pin to start form 1 like so: 1, 0, 1, 0... I have to set the pin to 1 before the loop right? Like so:

main()

{

       PORTXn = 1;

 

       while(1)

       {

              PORTXn = !PORTXn.

              delay(...)

       }

}

Yeah, but it stops working. In general when ever I try to operate with a data port register it only lets me write 1s into it. If I try to write 0 after a 1 it doesn't work. And I specifically disabled all optimizations! I build my code with ether -Og or -O0 with no difference in behavior.

 

I'm fed up with this all. I'm going to move away form Microchip studio and MCP2210 and Try to use the Arduino ecosystem. It is possible even though ATmega644 isn't supported right off the bat. I'll replace the MCP2210 with an Arduino nano and write and build my code in the Arduino IDE. I think that'll work.

 

Thanks a lot for all your advice, and sorry again for not disclosing all the details, I really didn't think they were relevant before I built the LED board which let me see how bad things actually are.

I'll write here about my Arduino experience when I have some results.

Last Edited: Sun. Jul 3, 2022 - 07:50 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Answering to Skeeve: " Are the device code and the simulator code derived from the same ELF?"

Microchip studio (former AVR studio) generates a bunch of output files. ELF, HEX, LSS, EEP and SREC. I use the HEX file since it's the easiest format to read by my program. I thought that the ELF and HEX contain the same program but now after a couple suggestions that the reason why the code works in the simulator and not in the actual device might be because both run different code I'm not so sure anymore that the ELF and HEX files contain the same program. I opened the ELF in a HEX editor and tried find the same sequences of bytes but couldn't. The ELF file is much bigger and contains a lot more meta data besides the actual binary data of the program.

 

 

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

StarTraveler wrote:

I have many extremely weird issues such as: (Meta code)

main()

{

       while(1)

       {

              PORTXn = !PORTXn. //Yes I know I can use the PIN input register to toggle while the pin is configured as output, but I wanted to use the data register since that's how you implement a running light...

              delay(...)

       }

}

The above code works. The pin toggles like this: 0, 1, 0, 1... What if I want the pin to start form 1 like so: 1, 0, 1, 0... I have to set the pin to 1 before the loop right? Like so:

main()

{

       PORTXn = 1;

 

       while(1)

       {

              PORTXn = !PORTXn.

              delay(...)

       }

}

Yeah, but it stops working. In general when ever I try to operate with a data port register it only lets me write 1s into it. If I try to write 0 after a 1 it doesn't work. And I specifically disabled all optimizations! I build my code with ether -Og or -O0 with no difference in behavior.

 

I realize that you said this is "meta code" and thus it may not actually be representative of your actual code, but are you configuring the pins you want to toggle as outputs by setting the relevant bits in the DDRx registers? If you haven't done this, you are enabling/disabling the internal pull-up resistor instead of driving the pin high/low.

build-avr-gcc: avr-gcc build script

toolchain-avr-gcc: CMake toolchain for cross compiling for the AVR family of microcontrollers

avr-libcpp: C++ standard library partial implementation (C++17 only) for use with avr-gcc/avr-libc

picolibrary: C++ microcontroller driver/utility library targeted for use with resource constrained microcontrollers

picolibrary-microchip-megaavr: picolibrary HIL for megaAVR microcontrollers

picolibrary-microchip-megaavr0: picolibrary HIL for megaAVR 0-series microcontrollers

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

Yes I did configure them as outputs...

The latest update form today is that I switched to Arduino environment. I added MightyCore through Board manager in order to add support for ATmega644 and, even though still using my faulty integrated programmer, I was able to make a simple 16-bit (using PORTA and C) running light program that runs correctly.

Another thing that I failed to mention is that I'm quite new to embedded programming in C/C++ (although that's probably quite obvious at this point XD).

This is my experience with the Arduino environment:

 

static int index;

void setup()
{
    DDRA = 0xff;
    DDRC = 0xff;
    PORTA = static_cast<uint8_t>(0);
    PORTC = static_cast<uint8_t>(1);
    index = 0;
}

void loop()
{
    if(index == 16)
    {
//        PORTA = static_cast<uint8_t>(0); <- if this is uncommented the program stops running.
        index = 0;
    }
    if(index <  8) PORTC = 1 << static_cast<uint8_t>(index);
    if(index == 8) PORTC = static_cast<uint8_t>(0);
    if(index >= 8) PORTA = 1 << static_cast<uint8_t>(15 - index);

    ++index;
    delay(100);
}

So in order for the running light to work correctly there must be something to turn off the last LED before the first lights up. Well that's the code in the if(index == 16) block. However if I uncomment the actual code that does the job the hole program stops working only the first LED lights up and that's it. Without this line of code the running light does work but obviously the last LED improperly remains on until if(index >= 8) becomes true. You might Notice the (15 - index) that's because the indexing of bits in port A is inverse to that of port C.

 

How did I fix this? I used the digitalWrite function provided by the Arduino environment.

static int index;

void setup()
{
    DDRA = 0xff;
    DDRC = 0xff;
    PORTA = static_cast<uint8_t>(0);
    PORTC = static_cast<uint8_t>(1);
    index = 15;
}

void loop()
{
    if(index > 31)
    {
        digitalWrite(31, LOW);
        index = 15;
    }
    if(index > 15) digitalWrite(index - 1, LOW);
    if(index < 24) digitalWrite(index, HIGH);
    else           digitalWrite(index, HIGH);
    index++;
    
    delay(100);
}

 

Here the inverse indexing of port A is not resolved yet so even though the runnig light works correctly in that all LEDs turn on or off with no one remaining on, currently I have two banks of lights running towards the middle of the LED stripe. In alternating faction.

 

I have no idea why the first example doesn't work and the second does. Must be some scope issue. there are probably not enough registers to handle all operations while digital write isolates everything properly.

I have a lot to learn. I have to refresh the AVR assembly knowledge since my previous experience from 10 years ago is in programming in assembly. This is why I was so confident I could start programming in embedded C++ right off the bat, but I was mistaken :)

 

Thanks again. I'm impressed how involved some of you got with solving my problem, I greatly appreciate that! :)

Last Edited: Sun. Jul 3, 2022 - 08:09 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Here we go again.

I thought using the digiralWrite was the solution but it isn't.

So In my last example in comment #26 the running light works but because the inverted indexing of port A what happens is the light runs right to left to the middle and then jumps to the left most LED and starts running left to right to the middle.

In order to fix that I have do some simple math an logic. However the code doesn't work and I have no Idea why. Here's the complete code:

//                                                                                                      Port A                                         Port C                      <---- Direction of the running light.

// Those are the pin indices as per the Arduino nomenclature: 24, 25, 26, 27, 28, 29, 30, 31 - 23, 22, 21, 20, 19, 18, 17, 16, 15

 

static int index;

void setup()
{
    DDRA = 0xff;
    DDRC = 0xff;

    PORTA = 0;
    PORTC = 0;

 

    index = 15;
}

void loop()
{
    if(index > 31)
    {
        digitalWrite(24, LOW); // <- If I comment this line out the program starts working obviously with the fault of the last LED not turning off when it should.
        index = 15;
    }
    if(index < 24)
    {
        if(index > 15)
        {
            digitalWrite(index - 1, LOW);
        }

//     if(index >= 15) // <- If I add this useless 'if' here the program starts working in full?!!!! No idea why. The 'if' has no logical significance but without it only the first LED lights up and that's it the light doesn't run.
              digitalWrite(index, HIGH); // <- If I comment this line out, (if the useless 'if' isn't there) the program starts to with the obvious flaw of the first 8 LEDs not lighting up.
    }
    else
    {
        if(index == 24) digitalWrite(index - 1, LOW);
        if(index >  24) digitalWrite(55 - index + 1, LOW);

        digitalWrite(55 - index, HIGH); // <- Also if I comment this line out (without the useless if) the program starts working with the obvious flaw that the the second 8 LEDs don't light up.
    }
    index++;
    delay(100);
}

 

From what I see it's as if there are too many calls to digitalWrite?! :O It makes no sense to me. Apparently when I enclose that call digitalWrite into the useless 'if' I create a new nested scope for it lifting the apparent limitation. That's my explanation anyway, I know it doesn't make much sense. I did try to enclose the digitalWrite into a simple scope {} Like: { digitalWrite(...) } with no effect.

 

 

Last Edited: Sun. Jul 3, 2022 - 08:47 PM
This reply has been marked as the solution. 
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 1

I found where the bug was! My programming software wasn't uploading the whole program. (facepalm) It was skipping the last few bytes of every program. this is why I had such inconsistent behavior. Huge apologies to everyone! Your advices were great given the info I was providing. You couldn't know you were dealing with a dummy who blames the compiler rather than checking all the failure points in his own system. XD In my defense the reason why I didn't suspect the fault is in my program was because my program worked exactly as per the article I learned how to read a HEX file from, which was complete with code and detailed explanations. The dude who wrote the article said "You can ignore the last two rows of the file, they are some Intel nonsense". It turns out, however, that the row before the last row, even though shorter that the others, is a valid part of the program. Very much in contrast with what he was saying. So my fault was that, I trusted a guy on the internet knew what he was going. XD Instead of finding the official HEX file format specification and writing my code based on that. After "banging" my head all day today it was a miracle I found what the problem was. Thanks to the Microchip debugger which shows the raw binary I noticed that the binary shown does not end with the bytes my uploaded program ends with. I could identify the last bytes of my uploaded program and thus exactly how many more bytes are missing. I manually added them to the HEX file, uploaded it, and boom! All of a sudden the program worked as expected. Then I realized that the byes I added were already in the file, in one of the last two rows I was ignoring...

 

This solves the mystery.

Thanks everyone again!

:)

Last Edited: Tue. Jul 5, 2022 - 05:21 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

StarTraveler wrote:

The dude who wrote the article said "You can ignore the last two rows of the file, they are some Intel nonsense". It turns out, however, that the row before the last row, even though shorter that the others, is a valid part of the program.

 

The Internet, as far a technical articles are concerned, is generally somewhere around 80% wrong.

#1 Hardware Problem? https://www.avrfreaks.net/forum/...

#2 Hardware Problem? Read AVR042.

#3 All grounds are not created equal

#4 Have you proved your chip is running at xxMHz?

#5 "If you think you need floating point to solve the problem then you don't understand the problem. If you really do need floating point then you have a problem you do not understand."

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

Mr Sturgeon will not be denied.

 

On an Intel 8-bit hex file, every line is significant - the last line contains zero data bytes but is of a different record type[1] (0x01 instead of 0x00) to indicate that there is no more data. See the Wiki article https://en.wikipedia.org/wiki/In... for details.

 

Neil

 

[1] Apparently some files are generated with the end of file indicated as a normal record of zero length. I don't recall seeing one, but it's certainly possible.

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

I've written an Intel hex parser and know that every line MUST be parsed.

I'm calling that "Internet Dude" out as a complete idiot.

 

 

PS: As a result of your post; curtvm did discover that edge case bug in the compiler.

Last Edited: Wed. Jul 6, 2022 - 08:02 AM