mal wieder: Arduino Klasse in Klasse - bitte Hilfe!

[Frank E. (Firma: Q3) (qualidat)]

Wie ich bereits in einem anderen Thread schrieb, möchte ich die LEDs in 
Tastern mit Hilfe des I2C-Servocontrollers PCA9685 ansteuern. Während 
ein estes Testprogramm ohne eigene Klasse problemlos läuft, hat die 
Version mit meiner Klasse ein merkwürdiges Problem: Es gibt beim 
Compilieren keinen Syntaxerror, aber es passiert in der Ausführung 
einfach NICHTS. Wo ist der Fehler? Danke für Tips!

einfacher Bais-Code:

#include <Wire.h>
#include <Adafruit_PWMServoDriver.h>

int addr = 0x40;
Adafruit_PWMServoDriver led = Adafruit_PWMServoDriver(addr);

void setup()
{
  led.begin();
  led.setPWMFreq(1000); 
  Wire.setClock(400000);
  pinOn(0);
  pinOn(1);
  pinOff(2);
  pinOn(3);  
}

void loop(){}
void pinOff(int p){led.setPWM(p, 0, 4096);}
void pinOn(int p){led.setPWM(p, 4096, 0);}

meine Klasse:

#include <Wire.h>
#include <Adafruit_PWMServoDriver.h>

class LEDKey
{
  private:
    Adafruit_PWMServoDriver *led;   
  public:

  LEDKey(int addr) //0x40 oder 0x41
  { 
    led = new Adafruit_PWMServoDriver(addr);
    led -> begin();
    led -> setPWMFreq(1000);
    Wire.setClock(400000);
  } 

  void pinOff(int p)  {led->setPWM(p , 0 , 4096);}
  void pinOn(int p)   {led->setPWM(p , 4096 , 0);}
};

und die Nutzung der Klasse:

#include "ledkey.h"
int addr = 0x40;
LEDKey led = LEDKey(0x40);

void setup()
{
  Serial.begin(9600);
  led.pinOn(0);
  led.pinOn(1);
  led.pinOn(2);
  led.pinOff(3);
}
void loop(){}

Zur Ergänzung: Pin 0 ist die gemeinsame Anode, die Pins 1, 2 u. 3 sind 
jeweils die Kathoden der LEDs. Beispiel 1 schaltet die grüne LED ein, 
Beispiel 2 sollte die blaue LED einschalten.

[ESP4096 S. (esp4096)]

Vermeide 'new' wo es nur geht. Immer wenn es einen Weg gibt ohne 'new' 
ist dieser Weg zu bevorzugen.

Was mir auch noch auffällt. Hör am besten auf Dinge die kaputt gehen 
können in Konstruktoren zu packen. Mach für deine LEDKey Klasse eine 
eigene begin Methode. Was passiert mit dem Objekt, wenn der Konstruktor 
mittendrin kaputt geht?

[J. S. (jojos)]

sieht man denn die Unterschiede wenn pinOn() schnell nacheinander 
aufgerufen wird? Beim Test mit Klassen ist als letztes pinOff(3) drin, 
vielleicht siehst du nur nicht das die LED an war.

Das pauschale 'kein new' ist Quatsch. Wenn es so wie hier ein pseudo 
statisches new ist, dann ist das kein Problem.
Dann schreibt man beim -> Operator keine Leerzeichen davor und dahinter, 
das sieht übel aus.
Und Adafruit_PWMServoDriver led = Adafruit_PWMServoDriver(addr); ist ein 
Copy Konstruktor. Ich weiß nicht warum das in vielem Arduino Code drin 
ist, es kostet unnötig Speicher und Laufzeit und kann Probleme machen 
wenn HW kopiert werden soll.
Adafruit_PWMServoDriver led(addr); reicht.

[EAF (Gast)]

Frank E. schrieb:
> Adafruit_PWMServoDriver led = Adafruit_PWMServoDriver(addr);

Das ist doch irgendwie unnötiger Unfug, möchte ich meinen...

Sollte auch funktionieren:

Adafruit_PWMServoDriver led {addr};

Gleiches gilt auch hier!
Frank E. schrieb:
> LEDKey led = LEDKey(0x40);

// dieses mal mit ein paar Alternativen Schreibweisen.
LEDKey led(0x40);
// oder
LEDKey led {0x40};
// oder
LEDKey led = 0x40;

Dann:
Wenn es einen Konstuktor mit new gibt, sollte es auch einen Destruktor 
mit delete geben.
Siehe: RAII
Darum auch: Auf Zeiger verzichten, wenn möglich.
Hier ist es möglich.

Dann, der Aufruf im Konstruktor, die Ursache der Probleme (ohne Gewähr)

led -> begin();

Das muss nicht klappen, da zu dem Zeitpunkt keine Garantie besteht, dass 
die Peripherie schon vollständig initialisiert wurde.

[ESP4096 S. (esp4096)]

J. S. schrieb:
> Das pauschale 'kein new' ist Quatsch.

Na dann ist es ja gut, dass Keiner davon geredet hat.

[EAF (Gast)]

ESP4096 S. schrieb:
> Na dann ist es ja gut, dass Keiner davon geredet hat.

Merksätze:
1. Jedes vermeidbare new ist böse.
2. Jedes vermiedene new ist gut.
Bemerke: Zwischen den beiden gibt es eine (winzig kleine?) Hysterese, 
der unvermeidbaren new.

[ESP4096 S. (esp4096)]

Schön das wir da einer Meinung sind! :)

[Frank E. (Firma: Q3) (qualidat)]

ICh habe jetzt mal den Code entsprechend einigen der Empfehlungen 
angepasst. Er wird fehlerfrei compiliert, tut aber nach wie vor NIX 
(sollte rote LED einschalten).

#include <Wire.h>
#include <Adafruit_PWMServoDriver.h>

class LEDKey
{
  private:
    Adafruit_PWMServoDriver * led; 
      
  public:
  void begin(int addr)
  { 
    Adafruit_PWMServoDriver led(addr);
    delay(100);
    led.begin(addr);
    led.setPWMFreq(1000);
  }
  void pinOff(int p)  {led->setPWM(p , 0 , 4096);}
  void pinOn(int p)   {led->setPWM(p , 4096 , 0);}
};

LEDKey myled;

void setup()
{
  Wire.begin();
  Wire.setClock(400000);
  myled.begin(0x40);
  myled.pinOn(0);
  myled.pinOff(1);
  myled.pinOn(2);
  myled.pinOn(3);
}

void loop(){}

Ich habe auch die Variante mit "private: Adafruit_PWMServoDriver led; " 
und dann mit Punkt statt -> ausprobiert. Ebenfalls kein Compile-Fehler, 
aber auch keine Aktion.

[J. S. (jojos)]

*led und led(addr) sind jetzt zwei Variablen die nix miteinander zu tun 
haben. led-> greift ins Nirwana.

[EAF (Gast)]

Zufall!?

[EAF (Gast)]

Frank E. schrieb:
> Ich habe auch die Variante mit "private: Adafruit_PWMServoDriver led; "
> und dann mit Punkt statt -> ausprobiert. Ebenfalls kein Compile-Fehler,
> aber auch keine Aktion.

Mein Rat:
Du solltest dringenst ein C++ Buch durcharbeiten.
Und auch das logische Denken aktivieren.

Denn: Ausprobieren ist eher selten Zielführend.
Ist mehr ein ahnungsloses stochern im Nebel.

[EAF (Gast)]

EAF schrieb:
> Zufall!?

Ach da waren andere schneller....
Soll natürlich heißen:

ESP4096 S. schrieb:
> Schön das wir da einer Meinung sind! :)
Zufall!?

[Frank E. (Firma: Q3) (qualidat)]

Wird compiliert, tut aber auch nix. M.E. maximal an den funktionierenden 
Basiscode aus dem Threadstart angelehnt. Wieso bitte?

#include <Wire.h>
#include <Adafruit_PWMServoDriver.h>

class LEDKey{
  private:
    Adafruit_PWMServoDriver led; 
  public:
  void begin(int addr){ 
    Adafruit_PWMServoDriver led = Adafruit_PWMServoDriver(addr); 
    delay(100);
    led.begin();
    led.setPWMFreq(1000);
  }
  void pinOff(int p)  {led.setPWM(p , 0 , 4096);}
  void pinOn(int p)   {led.setPWM(p , 4096 , 0);}
};

LEDKey myled;

void setup(){
  Wire.begin();
  Wire.setClock(400000);
  myled.begin(0x40);
  myled.pinOn(0);
  myled.pinOff(1);
  myled.pinOn(2);
  myled.pinOn(3);
}

void loop(){}

[J. S. (jojos)]

Frank E. schrieb:
> Wieso bitte?

jetzt hast du immer noch zwei led Instanzen die nichts miteinander zu 
tun haben.
Warum nimmst du nicht den Code vom ersten Arduino Fan?

[Εrnst B. (ernst)]

Damit verlierst du die Flexibilität, die die PMWservo Klasse im 
Konstruktor hat, u.A. da ein eigenes "TwoWire" Objekt durchzureichen, 
z.B. falls der µC mehrere I²C-Busse hat, oder auch für Soft-I2C usw.

Besser:

   ...
class LEDKey {
  private:
    Adafruit_PWMServoDriver * led;    
  public:
    LEDKey(Adafruit_PWMServoDriver * driver): led(driver) {}
    void begin() {
      led->begin();
    }
   ...
};

   ... main.cpp
   Adafruit_PWMServoDriver driver(0x42);
   LEDKey myled(&driver);

  setup() {
    ...
    myled.begin();

[J. S. (jojos)]

Εrnst B. schrieb:
> private:
>     Adafruit_PWMServoDriver * led;
>   public:
>     LEDKey(Adafruit_PWMServoDriver * driver): led(driver) {}

und noch schöner mit Referenz wie es auch die Adafruit_PWMServoDriver 
Klasse macht. Adafruit_PWMServoDriver funktioniert nur wenn es auch 
einen Bus gibt, wenn driver = nullptr ist dann knallt es wieder.

[Εrnst B. (ernst)]

J. S. schrieb:
> und noch schöner mit Referenz

stimmt.

Adafruit_PWMServoDriver::Adafruit_PWMServoDriver(const uint8_t addr,
                                                 TwoWire &i2c)
    : _i2caddr(addr), _i2c(&i2c) {}

[Frank E. (Firma: Q3) (qualidat)]

Ich habe selber bemerkt (noch bevor ich den Folge-Post gelesen habe), 
dass die erneute Deklaration von led innerhalb von begin() natürlich 
falsch ist und das geändert ...

[EAF (Gast)]

Das mit dem "driver" ist schon wahr/gut/besser/schöner/universeller....

#include <Wire.h>
#include "ledkey.h"

constexpr byte addr = 0x40;


Adafruit_PWMServoDriver driver{addr};
LEDKey led {driver};

void setup()
{
  Serial.begin(9600);
  led.begin();
  Wire.setClock(400000);
  led.pinOn(0);
  led.pinOn(1);
  led.pinOn(2);
  led.pinOff(3);
}
void loop(){}

-----------------

#pragma once

#include <Wire.h>
#include <Adafruit_PWMServoDriver.h>
class LEDKey
{
  private:
    Adafruit_PWMServoDriver &driver;   
  public:
  LEDKey(Adafruit_PWMServoDriver &driver): driver{driver} {} //0x40 oder 0x41

  void begin()
  {
    driver.begin();
    driver.setPWMFreq(1000);
  }

  void pinOff(int p)  {driver.setPWM(p , 0 , 4096);}
  void pinOn(int p)   {driver.setPWM(p , 4096 , 0);}
};

[J. S. (jojos)]

wobei ich den Adafruit_PWMServoDriver schon als Instanz in der Klasse 
lassen würde und im Konstruktor den Wire als Referenz übergeben würde.
Mehrere Led Objekte mit dem gleichen Adafruit_PWMServoDriver machen 
vermutlich keinen Sinn, aber mehrere Led mit verschiedenen I2C Adressen 
(und damit jeweils eigenem Adafruit_PWMServoDriver) schon.

[Frank E. (Firma: Q3) (qualidat)]

EAF schrieb:
> ESP4096 S. schrieb:
>> Na dann ist es ja gut, dass Keiner davon geredet hat.
>
> Merksätze:
> 1. Jedes vermeidbare new ist böse.
> 2. Jedes vermiedene new ist gut.
> Bemerke: Zwischen den beiden gibt es eine (winzig kleine?) Hysterese,
> der unvermeidbaren new.

Ich habe den Code benutzt (den von 12:20 und den von 12:59), vielen 
herzlichen Dank für die Mühe.

Aber leider gleiches Resultat: Fehlerfreie Compilation, keine Wirkung.

Nochmal der Hinweis: Der "Basiscode" im ersten Post funzt auf 
identischer Hardware einwandfrei! Kann es sein, dass der Adafruit-Code 
"irgendwie" nicht geeignet ist, in einer Klasse verarbeitet zu werden? 
Gibts sowas?

Dann muss ich das Projekt eben "klassisch" mit ungebundenen Funktionen 
fertigstellen, das wird mit Sicherheit funktionieren. Wäre nur anders 
herum schöner gewesen ... aber ich abe leider nicht endlos Zeit.

[EAF (Gast)]

J. S. schrieb:
> Mehrere Led Objekte mit dem gleichen Adafruit_PWMServoDriver machen
> vermutlich keinen Sinn, aber mehrere Led mit verschiedenen I2C Adressen
> (und damit jeweils eigenem Adafruit_PWMServoDriver) schon.

Hmmm...
Aus dem Blickwinkel schon.

Mir dachte eigentlich, dass man "class LEDKey" zu einer Fassade oder 
Adapter ähnlichen Konstruktion ausbauen kann, welcher verschiedenste LED 
Treiber bedienen kann.
Schließlich gibt es ja einige viele Chpis/Möglichkeiten PWM auszugeben, 
nicht nur das Adafruit Servo Board.

[J. S. (jojos)]

schade, den I2C PWM Baustein kennt die Simu nicht, oder übersehe ich 
etwas?
https://wokwi.com/projects/346671167697846866
oder gibt es bessere?

Heute Abend könnte ich die reale HW ausprobieren, so ein Teil müsste ich 
haben.

ATMega ist etwas besser, man kann einen LA an I2C anschliessen. Man 
sieht auch das auf dem Bus etwas zyklisch rauskommt, aber der PCA ist 
nicht da und entsprechend gibt es NAK auf dem Bus.
https://wokwi.com/projects/346672212853391955

Ist es also das simple Problem das die Abschlusswiderstände am Bus 
fehlen?

[EAF (Gast)]

J. S. schrieb:
> Heute Abend könnte ich die reale HW ausprobieren, so ein Teil müsste ich
> haben.

Habe ich gerade getestet!
Der Code von 12:59 funktioniert auf dem UNO mir dem Adafruit (Klon) 
Dingen.

Frank E. schrieb:
> Ich habe den Code benutzt (den von 12:20 und den von 12:59), vielen
> herzlichen Dank für die Mühe.
>
> Aber leider gleiches Resultat: Fehlerfreie Compilation, keine Wirkung.
Da machst du was falsch!
Oder hältst was geheim

[Frank E. (Firma: Q3) (qualidat)]

EAF schrieb:
> J. S. schrieb:
>> Heute Abend könnte ich die reale HW ausprobieren, so ein Teil müsste ich
>> haben.
>
> Habe ich gerade getestet!
> Der Code von 12:59 funktioniert auf dem UNO mir dem Adafruit (Klon)
> Dingen.
>
> Frank E. schrieb:
>> Ich habe den Code benutzt (den von 12:20 und den von 12:59), vielen
>> herzlichen Dank für die Mühe.
>>
>> Aber leider gleiches Resultat: Fehlerfreie Compilation, keine Wirkung.
> Da machst du was falsch!
> Oder hältst was geheim

Mea culpa!

Irgendwie hängt sich der PCA9685 auf und ist daraus nur durch kompletten 
Spannungs-Entzug für mind. 5s zurückzuholen. Oder per speziellem 
Reset-Befehl, den ich schließlich in der Adafruit-Lib gefunden habe.

Also: Es funktioniert! Danke und Sorry für de Stress.

[Helmut (Gast)]

J. S. (jojos)
27.10.2022 13:34

>ATMega ist etwas besser, man kann einen LA an I2C anschliessen. Man
>sieht auch das auf dem Bus etwas zyklisch rauskommt, aber der PCA ist
>nicht da und entsprechend gibt es NAK auf dem Bus.
>https://wokwi.com/projects/346672212853391955

Cool .. ich wusste gar nicht, dass man mit Wokwi Logik-Analysator Files 
erzeugen kann.
Es ist zwar etwas unpraktisch, dass man PulseView extra auf dem PC 
installieren muss, es funktioniert aber schnell und zuverlässig.