Go Down

Topic: Unbeabsichtigter Aufruf einer virtuellen Methode. (Read 1 time) previous topic - next topic

combie

Schönen guten Tag.

Endlich habe ich auch mal wieder ein Problem, dessen Lösung/Ursache sich mir entzieht.


Erstmal, wozu es dienen soll:
Es ist die Xte Variante meiner Multitasking Makros.
Dieses mal ganz im OOP Gewand.


Seit Monaten läuft es stabil.
In mehren Anwendungen.

Für das nächste Projekt musste die Erzeugung von Tasks dynamisch erfolgen. Auf Grund des hohen Speicher/RAM Bedarfs einzelner Aufgaben.

Und da klemmt es jetzt.


Meine Task Klasse hat 3 virtuelle Methoden:
1. begin()
2. loop()
3. destructor

Mit Constructor, Destructor, loop() ist alles ok.
Bisher immer.

Das Kind, welches Husten hat, ist begin()


Grundsätzlich ist der Aufbau jetzt:
Jede konstruierte Task, trägt sich mit Scheduler::add(Task*) in die Liste ein.
> Scheduler::instance().begin();
Ruft die begin() Methoden aller von Task abgeleiteten Objekte  auf.
Perfekt!

Scheduler::begin(); setzt auch eine interne Variable beginDone.
Klappt perfekt.

Scheduler::add(Task*) prüft ob beginDone schon gesetzt ist, und wenn ja ruft es begin() der jeweiligen Task auf.
Und das geht eben schief.
Es wird dummer Weise die Methode von Task aufgerufen, und nicht die Methode begin() der abgeleiteten Klasse.
In dem konkreten Beispiel äußert es sich dann so, dass pinMode() nicht aufgerufen wird.



Code: [Select]
void setup() // funktioniert
{
  new BlinkTask(13,500);
  Scheduler::instance().begin();
}


Code: [Select]
void setup() // versagt
{
  Scheduler::instance().begin();
  new BlinkTask(13,500);
}


Es ist also so, dass alle Tasks, welche vor dem Scheduler::begin() Lauf erzeugt werden, perfekt funktionieren. Nur die später Erzeugten, da versagt der Task::begin() Aufruf.
Task::loop() Aufrufe haben kein Problem.


Es wäre schön, wenn mir einer sagt, was ich da falsch mache.

Auch eine alternative Variante, wie man solche Tasks dynamisch verketten kann, wäre genehm.
Wobei Singleton, List und ListNode auch schon mehrfach im Einsatz sind und andernorts wie erwartet  arbeiten.




EDIT:
Die korrigierte/aktuelle Lib findet sich in diesem Beitrag
Alle sagen: Das geht nicht!
Einer wusste das nicht und probierte es aus.
Und: Es ging nicht.

Tommy56

Hm, so wie ich das jetzt verstehe, ruft Scheduler::instance().begin(); die vorhandenen begin()-Methoden auf. Etwas anderes hat er ja nicht.
Ich denke, wenn Du später neue Tasks hinzufügst, müsste add diese Aufgabe mit erledigen, denn add erhält ja eine gültige Instance und kennt deren begin()-Methode.

Gruß Tommy
"Wer den schnellen Erfolg sucht, sollte nicht programmieren, sondern Holz hacken." (Quelle unbekannt)

combie

#2
Feb 07, 2019, 05:08 pm Last Edit: Feb 07, 2019, 05:19 pm by combie
Ja, so ist mein Vorhaben!
Genau das war mein Plan, an dem ich versage.

Auszug:
Code: [Select]
    void add(Task *task)
    {
      list.add(task);
      if(beginDone) task->begin(); // hier wird das falsche begin() aufgerufen
    }

Das ist dem Scheduler seine add() Methode.

Das addieren zur Liste klappt, der begin() Aufruf versagt.



Code: [Select]
  void begin()
  {
     Task *temp = (Task *)list.getFirst();
     if(beginDone) return;
     while(temp)
     {
       temp->begin();
       temp = (Task *)list.getNext(temp);
     }
     beginDone = true;
  }

Das ist dem Scheduler seine begin() Methode.
Alle sagen: Das geht nicht!
Einer wusste das nicht und probierte es aus.
Und: Es ging nicht.

Tommy56

Wenn Du einen neuen Task hinzufügst, kannst Du doch davon ausgehen, dass dessen begin() noch nicht aufgerufen worden ist und das if(beginDone) weg lassen.

Wenn das nicht garantiert ist, dann sollte beginDone eine Instanzvariable von task sein, die von dessen begin() auf true gesetzt wird. (als "begin() ist schon erledigt")
Also dann etwa so:
Code: [Select]

    void add(Task *task)
    {
      list.add(task);
      if(!task->isBeginDone()) task->begin();
    }



Alles aber nur ins Unreine gedacht, nicht getestet.

Gruß Tommy
"Wer den schnellen Erfolg sucht, sollte nicht programmieren, sondern Holz hacken." (Quelle unbekannt)

Whandall

Vielleicht ist Task zu niedrig in der Virtualisierungskette,
hast du mal einen cast auf Runable versucht?
Ah, this is obviously some strange usage of the word 'safe' that I wasn't previously aware of. (D.Adams)

combie

Quote
hast du mal einen cast auf Runable versucht?
Ich habe Runable erst  später da raus destilliert.
Als der Fehler schon persistierte.

Und ja, gerade mal schnell getestet:
Code: [Select]
   void add(Task *task)
    {
      list.add(task);
      if(beginDone) ((Runable*)task)->begin();
    }
 

Gleiches Versagen.
Es wird nicht BlinkTask::begin() aufgerufen, sondern Task::begin()
Alle sagen: Das geht nicht!
Einer wusste das nicht und probierte es aus.
Und: Es ging nicht.

Whandall

Ah, this is obviously some strange usage of the word 'safe' that I wasn't previously aware of. (D.Adams)

combie

Quote
Wenn das nicht garantiert ist, dann sollte beginDone eine Instanzvariable von task sein, die von dessen begin() auf true gesetzt wird. (als "begin() ist schon erledigt")
beginDone wird korrekt ausgewertet und begin wird auch aufgerufen, nur das falsche.

Ich habe die beginDone  Abfrage auch schon in den Konstruktor von Task verfrachtet.
Und darin dann Task::begin() aufgerufen.
Ebenso das gleiche Ergebnis/versagen.
Alle sagen: Das geht nicht!
Einer wusste das nicht und probierte es aus.
Und: Es ging nicht.

combie

Was ist überhaupt die abgeleitete Klasse?
Die findet sich in den Beispielen.
Da nicht zur Lib gehörig, sondern zur Applikation

Hier in vollständig:
Code: [Select]

#pragma once

#include <CooperativeTask.h>


class BlinkTask: public Task
{
  protected:
    const byte pin;
    const unsigned long interval;
 
  public:
    BlinkTask(const byte pin,const unsigned long interval):
       pin(pin),
       interval(interval)
    {
     
    }
   
    virtual void begin()
    {
      Serial.print("BlinkTask begin, pin: "); Serial.println(pin);
      pinMode(pin,OUTPUT);
    }
   
    virtual void loop()
    {
      TaskBlock
      {
        digitalWrite(pin,HIGH);
        taskPause(interval);
        digitalWrite(pin,LOW);
        taskPause(interval);
      }
    }
   

    virtual ~BlinkTask()
    {
      digitalWrite(pin,LOW);
      pinMode(pin,INPUT);
    }

};
Alle sagen: Das geht nicht!
Einer wusste das nicht und probierte es aus.
Und: Es ging nicht.

Whandall

#9
Feb 07, 2019, 05:54 pm Last Edit: Feb 07, 2019, 06:06 pm by Whandall
Ich denke dein Problem ist, dass du im Konstruktor von Task das add machst.
Zu dem Zeitpunkt gibt es den Blinker noch nicht.
Ah, this is obviously some strange usage of the word 'safe' that I wasn't previously aware of. (D.Adams)

combie

#10
Feb 07, 2019, 06:11 pm Last Edit: Feb 07, 2019, 06:26 pm by combie
Durchaus möglich!
Hört sich plausibel an.
Die Konstruktorkette hat Task() schon abgearbeitet und BlinkTask() noch nicht, darum ist die virtuelle Methodentabelle noch nicht auf dem letzten Stand.

Aber ich habe mir schon das Hirn zermartert: Wie anders machen?

Code: [Select]
(new BlinkTask(13,500))->begin();
Wäre eine Möglichkeit, die funktioniert.
Aber schön, ist das nicht.
Ich würde das lieber einer Automatik überlassen.

Wenn es nicht anders geht, dann allerdings so!
Alle sagen: Das geht nicht!
Einer wusste das nicht und probierte es aus.
Und: Es ging nicht.

combie

#11
Feb 07, 2019, 07:14 pm Last Edit: Feb 07, 2019, 09:44 pm by combie
So:
Wenn es nicht anders geht:

Hauptdatei:
Code: [Select]


#include <CooperativeTask.h>
#include "BlinkTask.h"
#include "MasterTask.h"


 

BlinkTask Blinker[]{// {pin,interval}
                       {12,100},
                       {11,333},
                       {10,777},
                   };     

MasterTask master {13,500,10000};

void setup()
{
  Scheduler::instance().begin();
}

void loop()
{
  Scheduler::instance().loop();
}



MasterTask, welche regelmäßig eine Slave Task erzeugt und wieder löscht:
Code: [Select]
#pragma once

#include <CooperativeTask.h>


class MasterTask: public Task
{
  protected:
    const byte pin;
    const unsigned long blinkInterval;
    const unsigned long slaveInterval;
    Task * slave;
 
  public:
   MasterTask(const byte pin,const unsigned long blinkInterval,const unsigned long slaveInterval):
       pin(pin),
       blinkInterval(blinkInterval),
       slaveInterval(slaveInterval),
       slave(nullptr)
    {
     
    }
   
   
    virtual void loop()
    {
      TaskBlock
      {
        slave = new BlinkTask(pin,blinkInterval);
        slave->begin();
        taskPause(slaveInterval);
       
        delete slave;
        taskPause(slaveInterval);
      }
    }
   

    virtual ~MasterTask()
    {
      delete slave;
    }

};



Die Lib im jetzigen Zustand, im Anhang

Naja, so geht es.
Aber wirklich glücklich macht mich das noch nicht.
Eine Automatik steht weiterhin ganz oben auf der Wunschliste.

Und damit wären wir beim nächsten Punkt:
Ist sonst noch was verbesserungswürdig?
Alle sagen: Das geht nicht!
Einer wusste das nicht und probierte es aus.
Und: Es ging nicht.

Tommy56

Nur für mich zum Mitdenken: Was spricht gegen meinen Vorschlag den Aufruf des task->begin() in der add()-Methode zu erledigen?

Gruß Tommy
"Wer den schnellen Erfolg sucht, sollte nicht programmieren, sondern Holz hacken." (Quelle unbekannt)

combie

#13
Feb 07, 2019, 10:24 pm Last Edit: Feb 07, 2019, 10:32 pm by combie
Quote
Was spricht gegen meinen Vorschlag den Aufruf des task->begin() in der add()-Methode zu erledigen?
Dagegen spricht: Dass es genau das ist, was ich getan habe!
Siehe #2, da siehst du die add Methode.

Und dass es nicht funktioniert.
In dem Augenblick scheint dieses zu gelten:
Quote
Die Konstruktorkette hat Task() schon abgearbeitet und BlinkTask() noch nicht, darum ist die virtuelle Methodentabelle noch nicht auf dem letzten Stand.
Die Konstruktoren einer Vererbungskette werden in der Vererbungsreihenfolge aufgerufen.
  • Die Basisklasse "Runable" zuerst.(es wird der default Konstruktor verwendet)
  • Dann der Konstruktor von "Task" (hier wurde fälschlicher weise add() und damit begin() ausgeführt)
  • Danach erst der Konstruktor von BlinkTask (erst hier zeigt die VMT auf die richtige begin() Methode)
Wie du siehst, befindet sich der add() Aufruf in der Mitte, dann ist BlinkTask noch nicht fertig konstruiert.
Darum ist die virtuelle Methodentabelle noch nicht vollständig.







Alle sagen: Das geht nicht!
Einer wusste das nicht und probierte es aus.
Und: Es ging nicht.

combie

#14
Feb 08, 2019, 03:09 pm Last Edit: Feb 08, 2019, 04:58 pm by combie
So...

Für die Testrunde, obs noch so läuft, wie gedacht, habe ich mal einen kleinen Benchmark gebastelt.

Ergebnisse, nur mit dieser (weiter unten gezeigten) Zähltask, ohne Beiwerk.
Alle Ergebnisse leicht schwankend.
Hier so ca die Mittelwerte
Es wird gezählt, wie oft der Scheduler seine Task Liste abklappert.
Wie oft jede Task (hier nur die eine) an die Reihe kommt.

Code: [Select]

Arduino UNO:       124100 per s
Arduino Mega:      102540 per s
Arduino DUE:       280100 per s

Blue Pill (STM32): 614000 per s


ESP8266 (80MHz):   102000 per s
ESP8266 (160MHz):  200000 per s

ESP32S :           471000 per s


Die UNO Nachbauten von Inhaos mit MD-328D Prozessor waren nicht testbar, da die Core Dateien nicht alle nötigen Features zur Verfügung stellen.

Ja, ich weiß, nicht sonderlich Aussagekräftig.
Aber dennoch ist zu sehen, dass es mit einigen µC Typen läuft.
Mich verblüfft der ESP8266 etwas, da hätte ich mehr erwartet.

Abschätzung:
Wenn bei 1 Task 100000 Durchläufe pro Sekunde zu schaffen sind, dann sind bei 4 Tasks immer noch 25000 Durchläufe pro Sekunde drin.

Ohne getestet zu haben, erwarte ich, dass ein 8MHz AVR Arduino die halbe Leistung eines UNO zeigt.
Und dass ein Mega2560 genau so schnell, wie ein UNO ist.


TestProgramm.ino:
Code: [Select]
/**
 * Ein Scheduler loop counter
 *
 *
*/

#include <Streaming.h>
#include <CooperativeTask.h>
#include "SchedulerLoopCountTask.h"

Scheduler &scheduler  = Scheduler::instance();



void setup()
{
 Serial.begin(9600);
 Serial.println("Start");
 
 scheduler.begin();
}

void loop()
{
 scheduler.loop();
}




SchedulerLoopCountTask.h
Code: [Select]

#pragma once

#include <CooperativeTask.h>
#include <Streaming.h>


class SchedulerLoopCountTask: public Task
{
  protected:
    const unsigned long interval;
    unsigned long count;
  
  public:
    SchedulerLoopCountTask(const unsigned long interval):
       interval(interval),
       count(0){}
    
    virtual void loop()
    {
      count++;
      TaskBlock
      {
        taskPause(interval);
        Serial << F("Scheduler loops: ") << count << F(" per ") << interval << F("ms") << endl;
        count = 0;
      }
    }

} schedulerLoopCountTask(1000);




Alle sagen: Das geht nicht!
Einer wusste das nicht und probierte es aus.
Und: Es ging nicht.

Go Up