Bitte um Review: Neuer Battery Code, Ausschalten bei Akku leer und MAX17055

Genau, ich hab das Problem gekonnt ignoriert und einfach den Widerstand irgendwie verbunden. :grin:

Ich habe den PR gestern mal lokal in mein Repository integriert und auf einer Complete mit FEPO-Akku getestet (ohne MAX17055).

Mindestens zwei Bugs habe ich da:
a) Obwohl die Spannung deutlich zu hoch gemessen wird (glaub es waren 4,5 V rum), leuchtet die LED-Anzeige rot (critical).
b) Nach dem Einschalten kommt aus dem Lautsprecher kein Ton. Ich muss erst einen Kopfhörer einstecken (der geht dann) und wieder rausziehen. Klingt für mich nach einer falschen Reihenfolge von was auch immer.

War dann zu spät gestern, um das weiter zu analysieren.

a) Also du meinst die Spannungsmessung ist bei mir falsch? Der Code ist ja eigentlich komplett aus der alten Version übernommen, nur anders verpackt. Kannst du nochmal kontrollieren ob auch wirklich die settings richtig sind? Die dummy-implementierung gibt nämlich eigentlich 4.2V als Spannung zurück, aber auch isCritical → False, das passt also irgendwie nicht zusammen.
Also in der settings musste ich ja ein paar Sachen ändern, vielleicht ist da im merge auch was falsch gelaufen. Wird BATTERY_MEASURE_ENABLE richtig definiert?

b) Oh ja, mh. Ich habe etwas an der Reihenfolge gedreht. Zum Beispiel aktiviere ich erst die Audioausgabe, damit die einen definierten Wert hat und dann schalte ich den Verstärker erst an. Hat bei mir ein Knacken beseitigt. Dürfte zwar nicht daran liegen, aber tausch doch mal Power_PeripheralOn() mit AudioPlayer_Init(). Sonst ist es vielleicht ein Problem mit der invertierten Logik, wobei die Power.h eigentlich denkbar einfach ist.

Weiteres:

  • Passt die Änderung mit I2C so? Habe kein anderes I2C Device am Bus. Also Port-Expander wird weiterhin erkannt?
  • Könntest du mit einem „normalen“ Akku testen? Nur um auszuschließen dass es nicht ein bestehendes Problem mit LiFePO4 ist, an der Spannungsmessung hab ich nämlich nicht wirklich was geändert.

Wenn die Probleme nicht gleich ersichtlich sind, mache ich am besten nochmal kleinere Pull-requests, die einzeln getestet werden können, um nicht zu viele Änderungen auf einmal zu haben.

a) Das muss ich generell nochmal überprüfen. Hier hatte ich vorher immer mit deutlich zu niedriger Spannung zu kämpfen und habe deswegen ein Offset draufgehauen: ESPuino/settings-complete.h at 0476fb05f85c67feeaf7e4d9e320b7233866b766 · biologist79/ESPuino · GitHub.
Das ist halt der Prototyp von den Tonuino-Jungs, da habe ich keinen Schaltungsplan. Ich habe die Platine aber auch nicht mit LiPo getestet, das will ich jetzt endlich mal machen.

b) Also es ist so, dass es zwei Ansätze gibt. Bei einem hängt der SD-Pin des MAX98357a quasi offen und wird, wenn man eine Kopfhörerplatine hat, auf LOW gezogen, wenn ein Kopfhörer eingesteckt ist. Den ESP32 muss dann vom Einstecken eigentlich nix mitkriegen, aber wünschenswert ist es doch, weil man dann die max. Lautstärke auf dem Kopfhörer begrenzen kann.
Die zweite Alternative, und von dieser rede ich, ist die Enable-Pins der DACs für Lautsprecher und Kopfhörer nicht direkt miteinander zu verbinden, sondern sie stattdessen auf einen Port-Expander aufzulegen. D.h. es wird immer vom ESP32 gesteuert, welcher DAC gerade aktiv ist. Schön an dieser Lösung ist auch, dass man problemlos eine Einschaltverzögerung realisieren kann. ESPuino schaut dann also, ob welchen logischen Pegel HP_DETECT (ESPuino/settings-complete.h at 0476fb05f85c67feeaf7e4d9e320b7233866b766 · biologist79/ESPuino · GitHub) hat und steuert damit GPIO_HP_EN und GPIO_PA_EN wechselseitig (ESPuino/settings-complete.h at 0476fb05f85c67feeaf7e4d9e320b7233866b766 · biologist79/ESPuino · GitHub und ESPuino/settings-complete.h at 0476fb05f85c67feeaf7e4d9e320b7233866b766 · biologist79/ESPuino · GitHub).

Ja, also i2c beim Port-Expander funktioniert, nur wie gesagt passt offenbar mit GPIO_PA_EN was nicht, da der Lautsprecher nicht angeht. Das tut er erst, nachdem ich einen Kopfhörer (mindestens mal kurz) eingesteckt hatte.

Kannst du mal probieren die zwei Zeilen zu vertauschen? Kann mir gut vorstellen, dass HP_DETECT einfach undefined ist wenn alles aus ist und dadurch etwas falsch läuft.

Welche? PA und HP _EN?

Power_PeripheralOn() mit AudioPlayer_Init() in main().

Und bitte nochmal pullen, gab einen kleinen Fehler wenn PN5180 ohne LPCD aktiviert ist, dann wurde init() nicht aufgerufen.

AudioPlayer.cpp:
pinMode(HP_DETECT, INPUT_PULLUP);

settings-complete.h
#define DETECT_HP_ON_HIGH

Passt meiner Ansicht nach nicht zusammen. Klar, wenn jetzt der Pin undefiniert ist wird durch den Pullup der Kopfhörer „erkannt“. Der port expander kann nicht beliebig pullup/pulldown setzen, oder? Eigentlich müsste hier je nach DETECT_HP_ON_HIGH PULLUP oder PULLDOWN verwendet werden.
Ich würde ja fast sagen eigentlich müsste ein externer Widerstand an den pin und es ist ein Hardware-Fehler :grin:.

Der Port-Expander hat immer einen „weak“ PullUp-Widerstand, wenn er auf Input konfiguriert ist. Und per Default ist ein Port-Expander immer auf Input konfiguriert; wenn man Output auf einem Pin will, dann muss man ihm das explizit „sagen“.

Eigentlich muss man die Zeile (zum Verständnis) rausnehmen, wenn man GPIOs hat, die nicht in der Range von 0 bis 39 liegen. Wenn man da einen ungültigen GPIO reinsteckt, was bei 10x ja zB der Fall wäre, dann passiert da einfach gar nix.

Aber nicht auszuschließen, dass hier irgendwas noch doppelt ist. Was in meinem Fall auf jeden Fall greift ist das hier:

Also das alleine hat nix geändert.
PN5180 habe ich an diesem Board nicht. Tja, muss ich wohl debuggen. Sau stark, dass ich die Commits ohne neuen Branch eingebunden habe, jetzt kann ich nicht einfach hin und herspringen (außer ich mache es rückgäng und nachträglich alles nochmal).

Wie auch immer: Das kann ich leider eh nicht fertigstellen, bevor keine P-Mosfets da sind, weil das muss auch noch für meine Platine mit Port-Expander getestet werden. Keine Ahnung was gerade mit der Briefpost abgeht, aber kommt alles erheblich verspätet.

Ah ok, ursprünglich hat AudioPlayer_Init wesentlich weiter unten gestanden. Unter anderem nach Port_Init(). Das könnte natürlich sehr gut der Fehler sein.
Es ist etwas schwierig zu sehen, welches Modul von einem anderen Modul abhängt und nach diesem ausgeführt werden muss. Das Problem gab es ja auch schon bei LPCD, wo die Reihenfolge nicht gepasst hat. Wenn es da eine Möglichkeit gäbe die Reihenfolge zu definieren, wäre für die Zukunft sicher auch gut.

Generell fände ich es gut, wenn der Verstärker möglichst zeitnah zum Anschalten auch definierte Eingänge bekommt. Wenn man nicht in Software stumm schaltet kommt es sonst manchmal zu einem unschönen Knacken beim Einschalten.

Kurzgesagt: Vermutlich muss Audio_Init() wieder später initialisiert werden.

Ja, ok. Ich hatte es mir so genau noch nicht angeschaut aber das klingt sehr danach, dass das der Fehler sein könnte. Eija, ich schaue es mir mal an. Aber vermutlich heute nicht mehr, da ich noch Puppenbett bauen muss :joy:.

Nachtrag: Das kam tatsächlich heute noch an. Dachte ja schon, dass es verschollen ist. War jetzt vom 25.9 bis heute unterwegs :joy:. Flaschenpost vielleicht? :slight_smile:.

Aber schön: Damit kann ich weiterhin sagen, dass noch nie was „verschütt“ gegangen ist, was ich bei Ali bestellt habe.

Zum Thema LiFePO4 noch ein interessantes Video von Andreas Spiess.

@SZenglein Ich habe das Review nicht vergessen. Ist nur gerade so ein bisschen die Luft raus. Ich kümmere mich darum.

Ja ist von meiner Seite kein Problem. Kann auch total verstehen wenn dir der pull request etwas zu umfangreich ist. Die Änderungen haben sich einfach ein wenig ineinander verknotet :smiley:

Wenn du den pull lieber in kleinere Häppchen unterteilt haben willst sag einfach Bescheid.

Ich habe das Thema nicht vergessen und habe es lokal inzwischen nochmal gebrancht. Beim Drücken des Buttons des Drehencoders wird auf meinem Port-Expander-Lolin D32pro-PCB „rot“ angezeigt. Das passt so nicht; muss ich nochmal schauen, woran das liegt. Dann hast du ein paar neue NVS-Variablen eingeführt; da ist ein Copy’n’Paste-Fehler drin (das habe ich bei mir schon gefixt).

Port_Init() habe ich mal nach oben geschoben - das behebt das Problem, dass ich weiter oben hier schon beschrieben habe, dass man auf dem Lautsprecher nix hört nach dem Einschalten. Nur habe ich mit dem Lolin D32 pro in o.g. PCB so ein elendiges Problem, in das ich anderweitig auch schon gelaufen bin. Und zwar wird der Power-GPIO ja zuerst deklariert und im Anschluss (in deinem Code später) dann HIGH drauf geschickt. Ohne Kopfhörerplatine startet der D32 pro verzögert und mit Kopfhörerplatine gar nicht. Ich habe zwei D32 pro hier, wobei der Erste irgendwie einen „Schlag“ hat. Jedenfalls mit dem hatte ich im Code, der aktuell online ist, auch so ein Problem. Das habe ich in den Griff bekommen, wenn ich zwischen der Deklaration und dem Draufschalten von High kein delay() hatte:

Mir ist leider völlig unklar an was das liegt. Äußern tut sich das Ganze so, dass man die geschalteten 3.3 V messen kann, sie jedoch immer wieder einbrechen. Man hört das auch ganz ganz leise. Also irgendwie versucht er zu starten aber es klappt nicht.

Vielleicht hat irgendwer ja nen Tipp dazu. Ich hatte das so noch bei keinem anderen ESP32.

@SZenglein Was meinst du mit folgendem Kommentar?

Was spricht denn gegen System_RequestSleep() an dieser Stelle?. Das setzt System_GoToSleep = true und damit wird System_DeepSleepManager() durchlaufen und der ganze Shutdown-Prozess wird ausgeführt.

Und noch eine Frage: Für was steht eigentlich „SOC“?

Der gleiche Grund aus dem im Rfid code auch manuell esp_deep_sleep_start() aufgerufen wird. Die RequestSleep Funktion greift erst nachdem die ganze Initialisierung durch ist, also Wifi, Neopixel, etc. Es ist etwas problematisch wenn bei sehr geringer Batterie dafür Energie verschwendet wird, aber viel unschöner ist dass der ESPuino nur angeht um dann sofort wieder auszugehen.

Kurzgesagt: Weil ich an der Stelle sofort wieder runterfahren will.

SOC steht für State of Charge (der Batterie). Ist eben die Lingo von dem chip.

Ich fühle mich ein wenig schlecht weil du so viel Arbeit in diesen Pull steckst, aber das ist ja letztendlich deine Entscheidung.
Ich kann die nächsten Wochen dazu leider gar nichts beitragen, aber danach kann ich mir das ganze nochmal in Ruhe anschauen…

Nee, keine Angst. Ist jetzt nicht so, dass ich da seit Wochen stundenlang pro Tag dran hänge. Die Hardware-Probleme, die ich zuvor beschrieben habe mit diesem PCB, habe ich gestern mit einem zusätzlichen Kondensator gefixt bekommen. @tueddy Du hattest mir das ja auch mal in einer PM geschrieben, dass man das vielleicht machen sollte. Also mit 100 uF geht’s auf jeden Fall.

Da läuft halt auch die generische Spannungsmessung drüber. Daher wollte ich das einfach gerne verstehen.

Das mit der Spannungsmessung klappt doch, weiß nicht, was ich da beim letzten Mal falsch gemacht habe. Was mir nicht so gefällt, das ist die Umwidmung der Farben bei der Spannungsanzeige auf dem Neopixel. Orange ist jetzt critical und greift erst bei einer vergleichsweisen tiefen Spannung. Ich wollte das aber eher so als „halb voll“. Da werde ich nochmal ne Runde drüber nachdenken, wie wir da vorgehen.

Ansonsten muss statt „…Impl“ für mich da was Anderes hin. Ich bin immer sehr für sprechende Funktions/Methodennamen zu haben.

Wie auch immer: Wenn ich mit der Arbeit durch bin, dann werde ich zum Testen einen eigenen Branch aufmachen und die vielen Commits (von mir sind auch welche dabei) mittels eines Squash-Merge in einen einzigen packen.

@SZenglein Du hattest ja kürzlich die Power.cpp eingeführt. Geändert hat sich dabei, dass die Initialisierung des Power-Pins und das anschließende Beschalten nicht mehr zwei Anweisungen hintereinander sind. @compactflash verwendet ja mit dem LTC2954 einen Powerswitch und hat seit dieser Aufspaltung offenbar ein Problem mit seiner Complete.
@compactflash Vielleicht willst du nochmal kurz was dazu schreiben, was das Problem ist.