Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix an issue if not all configured MPPTs delivers valid Data #1118

Merged
merged 1 commit into from
Jul 21, 2024

Conversation

SW-Niko
Copy link

@SW-Niko SW-Niko commented Jul 21, 2024

Hallo @schlimmchen ,
in #975 wurde noch ein Problem entdeckt wenn mehrere Victron MPPTs konfiguriert wurden, aber mindestens einer keine Daten über die VE.Direct Schnittstelle sendet.

Das Problem ergibt sich aus der Kombination der Funktion VictronMpptClass::isDataValid() und einer weiteren wie VictronMpptClass::getOutputVoltage()

Die VictronMpptClass::getOutputVoltage()
liefert den kleinsten Wert aus der Menge an MPPTs die gültige Daten senden.

Die VictronMpptClass::isDataValid()
liefert nur true wenn alle konfigurierten MPPTs gültige Daten liefern.

Und wenn man das wie in der Funktion PowerLimiterClass::getBatteryVoltage() kombiniert
float chargeControllerVoltage = -1;
if (VictronMppt.isDataValid()) {
res = chargeControllerVoltage = static_cast(VictronMppt.getOutputVoltage());
}
Dann ist das Ergebnis immer -1 wenn ein MPPT ausfällt.

Dieser PR sollte das Problem beheben. Ich bin mir nicht sicher ob die aktuelle Logik
nicht aus Gründen die ich nicht kenne absichtlich so geplant war.
Ich finde 3 Stellen im Code wo VictronMpptClass::isDataValid() verwendet wird und an keiner der 3 Stellen sehe ich ein Problem wenn wir die Logik umdrehen.

@schlimmchen
Copy link
Member

Meine Idee war, möglichst konservativ zu sein: "Wenn ein MPPT ausfällt, dann machen wir mal lieber gar nichts." Aber das ist in der Tat nicht nötig. Wenn einer gültige Daten liefert, dann kann man mit denen schon handtieren, sowohl mit der Spannung, als auch mit der gemeldeten Leistung. Ggf. wird ja trotzdem die korrekte Netzwerkgesamtleistung gemeldet, sodass Solar-Passthrough sogar korrekt weiterfunktioniert.

Daher finde ich diesen PR in Ordnung. Danke dafür.

@schlimmchen schlimmchen merged commit 415c767 into hoylabs:development Jul 21, 2024
8 checks passed
Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new discussion or issue for related concerns.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 21, 2024
@SW-Niko SW-Niko deleted the VeDirect-4 branch November 5, 2024 20:16
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants