In Konstruktor dynamisch instanz einer Klasse generieren, vererbung, C++

[Matze (Gast)]

Hallo zusammen,

Ich möchte einen LED-String aus WS2812B ansteuern können.

Dazu habe ich einen Treiber der die Bits auf der HW Rausschreibt.
-->Funktioniert.

Eine Klasse RGB_LED, welche die eigenschaften(Farbe) der LED enthält.
-->Funktioniert.

Eine Klasse RGB_STRING welche eine Anzahl an RGB_LEDs enthalten soll.
Sie soll über einen Aufruf von write_to_string() die eigenschaften aller 
LEDs an den String übertragen.

Weiterhin soll sie Farben, der LED nach Index einstellbar machen.
set_RGB_LED(uint8_t index, uint8_t red, uint8_t green, uint8_t blue)

Durch den Construktor soll ein Array mit zeigern auf die Elemente vom 
Typ LED_String angelegt werden, durch welches die oben genannten 
Funktionen von RGB_STRING durchitterieren können.

Hier leigt das Problem mit der Meldung:
Error : expected type-specifier before ';' token

In dieser Zeile:
RGB_STRING::string[i]=*new(RGB_LED());

Ich bin überfragt welchen Typ ich hier angeben soll.

Wäre schön wenn jemand eine idee hätte.
Weitere Fehler/Warungen gibt es nicht.

Hier der gesamte Code:

class RGB_LED
{
  public:
  RGB_LED();
  void set_LED(void);
  void set_colors(uint8_t r, uint8_t g, uint8_t b);
  private:
  uint8_t red;
  uint8_t green;
  uint8_t blue;
};

RGB_LED::RGB_LED()
{
  RGB_LED::red=0x00;
  RGB_LED::green=0xff;
  RGB_LED::blue=0x00;
}

void RGB_LED::set_colors(uint8_t r, uint8_t g, uint8_t b)
{
  red = r;
  blue = b;
  green = g;
};

void RGB_LED::set_LED()
{
  RGB_LED_write(RGB_LED::green,RGB_LED::red,RGB_LED::blue);
};

class RGB_STRING : public RGB_LED
{
  public:
    RGB_STRING(uint8_t led_cnt);
    void set_RGB_LED(uint8_t index, uint8_t red, uint8_t green, uint8_t blue);
    void write_RGB_STRING();
  private:
    uint8_t number_of_LEDs;
    RGB_LED string[];
};

RGB_STRING::RGB_STRING(uint8_t led_cnt)
{
    RGB_STRING::number_of_LEDs = led_cnt;
    for (int i=0; i < number_of_LEDs;i++)
    {
      RGB_STRING::string[i]=*new(RGB_LED()); //Problemzeile!!
    }
}

void RGB_STRING::set_RGB_LED(uint8_t index, uint8_t red, uint8_t green, uint8_t blue)
{
  for (int i=0; i < RGB_STRING::number_of_LEDs;i++)
  {
    RGB_STRING::string[i].set_colors(red,green,blue);
  }
}

void RGB_STRING::write_RGB_STRING()
{
  for (int i=0; i < RGB_STRING::number_of_LEDs;i++)
  {
    RGB_STRING::string[i].set_LED();
  }
}



int main(void)
{
  RGB_STRING LED_Chain(3);
    while(1)
    {
    for (int i=0;i<3;i++)
    {
      LED_Chain.set_RGB_LED(i,0xff,0xff,0xff);
    }
    LED_Chain.write_RGB_STRING();
    _delay_ms(10);
    }
}

Schöne Grüße,
Matze

[Carl D. (jcw2)]

Du erzeugst eine RGB_LED Instanz auf dem Heap und übergibst deren 
Adresse an ein "placement new", d.h. new soll an der übergebenen Adresse 
eine Instanz des nach new(addr) folgende Typs erzeugen, also dessen 
Konstruktor ausführen. Genau dieser Typ wird vermisst.
Daß du dieses Objekt nur kurz leben läßt, nämlich bis es vom Assignment 
Operator in einen Slot deines Arrays kopiert wurde, ist zwar syntaktisch 
korrekt, aber ungeschickt.
Generell: in der Implementierung einer Methode einer Klasse hat man 
Zugriff auf die andern Member der Klasse, ohne den "Klassen-Namespace" 
davorzuschreiben.

[Matze (Gast)]

Hallo und vielen Dank,

so war es nicht gedacht, new soll nicht an der übergebenen Addresse ein 
element des fehlenden Typs erzeugen.

Ich stelle es mit so vor, dass LED_STRING die zeiger auf die einzelnen 
LEDs in string[] enthält.

Da ich die größe von String nicht fix festlegen möchte, soll sie 
variabel sein. Nach Programmstart in String ein zeiger auf die 
Basisaddresse eines  Arrays von RGB_LED stehen.

Oh, ich habe nun eine Lösung gefunden :)

Hier meine Änderungen in RGB_STRING:

class RGB_STRING : public RGB_LED
{
  public:
    RGB_STRING(uint8_t led_cnt);
    void set_RGB_LED(uint8_t index, uint8_t red, uint8_t green, uint8_t blue);
    void write_RGB_STRING();
  private:
    uint8_t number_of_LEDs;
    RGB_LED* string[];
};

RGB_STRING::RGB_STRING(uint8_t led_cnt)
{
    RGB_STRING::number_of_LEDs = led_cnt;
    for (int i=0; i < number_of_LEDs;i++)
    {
      RGB_STRING::string[i*sizeof(RGB_LED)]=(RGB_LED*)malloc(sizeof(RGB_LED));
    }
}

void RGB_STRING::set_RGB_LED(uint8_t index, uint8_t red, uint8_t green, uint8_t blue)
{
  for (int i=0; i < RGB_STRING::number_of_LEDs;i++)
  {
    RGB_STRING::string[i*sizeof(RGB_LED)]->set_colors(red,green,blue);
  }
}

void RGB_STRING::write_RGB_STRING()
{
  for (int i=0; i < RGB_STRING::number_of_LEDs;i++)
  {
    RGB_STRING::string[i*sizeof(RGB_LED)]->set_LED();
  }
}

Es funktioniert bis auf einen kleinen BUG, wenn in main() keine farbe 
gesetzt wird, bleiben die LEDs aus.
--> Folglich wird der Konstruktor nicht aufgerufen.
Wenn ich in den Konstruktor von RGB_String nach dem Anlegen der neuen 
LED:

RGB_STRING::string[i*sizeof(RGB_LED)]->RGB_LED();

kommt die Meldung:
Error invalid use of 'RGB_LED::RGB_LED'  Ver3_1_Atmega8

Bei

RGB_STRING::string[i*sizeof(RGB_LED)]->RGB_LED::RGB_LED();

bekomme ich ein:
Error cannot call constructor 'RGB_LED::RGB_LED' directly

Wäre super wenn noch jemand wüsste warim die Klasse ohne 
Konstruktoraufrif angelegt werden kann?

Denke ich korrekt, wenn ich davon ausgehe, dass die RGB_LED-Objekte nun 
ab der basisaddresse des strings im Speicher liegen?

Grüße,
Matze

[Rolf M. (rmagnus)]

Matze schrieb:
> so war es nicht gedacht, new soll nicht an der übergebenen Addresse ein
> element des fehlenden Typs erzeugen.

Du nutzt hier aber placement new:

> RGB_STRING::string[i]=*new(RGB_LED());

Placement new sieht etwa so aus:

new(wert) Typ(Parameter);

Bei dir fehlt der Typ. Wenn du ein normales new willst, musst du die 
Klammern weglassen.
Aber auch dann ergibt das alles so keinen Sinn. Du hast ein Element vom 
Typ RGB_STRING, und nun erzeugst du ein neues dynamisches Objekt vom 
gleichen Typ und weist das diesem Element zu. Den Zeiger auf das 
dynamische Objekt verwirfst du dann und hast damit ein Memory-Leak.
Was mir noch auffällt:
RGB_STRING ist von RGB_LED abgeleitet. Warum?
Dem Array RGB_LED string[]; fehlt eine Größe.
Auch das ergibt in mehrererlei Hinsicht keinen Sinn:

> RGB_STRING::string[i*sizeof(RGB_LED)]=(RGB_LED*)malloc(sizeof(RGB_LED));

> Bei
> RGB_STRING::string[i*sizeof(RGB_LED)]->RGB_LED::RGB_LED();
> bekomme ich ein:
> Error cannot call constructor 'RGB_LED::RGB_LED' directly

Da hat der Compiler recht. Konstruktoren kann man nicht direkt aufrufen.

> Wäre super wenn noch jemand wüsste warim die Klasse ohne
> Konstruktoraufrif angelegt werden kann?

Das weiß ich auch nicht. Da dem Array die Größe fehlt, hätte ich 
erwartet, dass schon dort mit Fehler abgebrochen wird. Aber gcc lässt 
das wohl als Erweiterung selbst im Standard-Modus durchgehen. Wie das 
funktionieren soll, ist mir aber nicht klar.
Wenn das sauber funktionieren soll, wirst du nicht drum herum kommen, 
ein paar Grundlagen von C++ zu lernen.

[Erik S. (erik_s)]

Da sind einige Ungereimtheiten, Undurchgängigkeiten und Lücken im 
Grundlagenwissen zu finden. Anbei mein Vorschlag basierend auf dem, was 
ich als gewünschte Funktionalität vermute. Nicht übermäßig schön, aber 
funktionsfähig und der Heap wird dank Destruktor auf wieder freigegeben.

#include <memory>

class RGB_LED
{
  public:
    RGB_LED();
    void set_LED(void);
    void set_colors(uint8_t r, uint8_t g, uint8_t b);
  private:
    uint8_t red;
    uint8_t green;
    uint8_t blue;
};



RGB_LED::RGB_LED()
{
  set_colors(0x00, 0xff, 0x00);
// sofern die LED jetzt bereits grün leuchten sollte: 
  set_LED();
}



void RGB_LED::set_colors(uint8_t r, uint8_t g, uint8_t b)
{
  red = r;
  blue = b;
  green = g;
};


void RGB_LED::set_LED()
{
  RGB_LED_write(RGB_LED::green,RGB_LED::red,RGB_LED::blue);
};


class RGB_STRING
{
  public:
    RGB_STRING(uint8_t led_cnt);
    void set_RGB_LED(uint8_t index, uint8_t red, uint8_t green, uint8_t blue);
    void set_RGB_STRING();
    uint8_t get_led_cnt(void) const { return m_led_cnt; }
    ~RGB_STRING();
    
  private:
    uint8_t m_led_cnt;
    RGB_LED* (*m_p_leds)[]; // Pointer auf ein Array von Pointern auf RGB_LED
};



RGB_STRING::RGB_STRING(uint8_t led_cnt)
{
  m_p_leds =(RGB_LED*(*)[])malloc(led_cnt * sizeof(RGB_LED*)); // Speicher für Pointer-Array(RGB_LEDs) allokieren
  m_led_cnt = led_cnt;
  
  for(uint8_t i=0; i<m_led_cnt; i++){
    (*m_p_leds)[i] = new RGB_LED; // RGB-LED auf Heap instanziieren und Adresse in Pointer-Array ablegen. new sorgt dafür, dass der Konstruktor aufgerufen wird.
  }
}



void RGB_STRING::set_RGB_LED(uint8_t index, uint8_t red, uint8_t green, uint8_t blue)
{
  (*m_p_leds)[index]->set_colors(red,green,blue);
}



void RGB_STRING::set_RGB_STRING()
{
  for (int i=0; i < m_led_cnt; i++)
  {
    (*m_p_leds)[i]->set_LED();
  }
}


RGB_STRING::~RGB_STRING(){
  for(uint8_t i=0; i<m_led_cnt; i++){
    delete (*m_p_leds)[i]; // allokierten Speicher der RGB_LEDs freigeben
  }
  free(m_p_leds);  // allokierten Speicher des Pointer-Array(RGB_LEDs) freigeben
}


void main()
{
  {    
    RGB_STRING LED_Chain(3);
   
    for (int i=0;i<LED_Chain.get_led_cnt();i++)
    {
      LED_Chain.set_RGB_LED(i,0xff,0xff,0xff);
    }
    LED_Chain.set_RGB_STRING();
  }
}

[Dirk K. (merciless)]

Warum verwendest du nicht std::vector<RGB_LED>
(oder std::vector<RGB_LED *>) anstelle des C-Arrays?

merciless