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

Properly handle VE.Direct fragmentation #814

Merged
merged 6 commits into from
Apr 2, 2024

Conversation

schlimmchen
Copy link
Member

  • add unique prefix to VE.Direct messages. this makes it easier to map debug messages to one-another.
  • properly handle fragmented VE.Direct messages. queue every text event until the frame was checked by it checksum. then process the data directly into the buffer struct. do not clear the buffer struct, so it will always include the most recent value of a particular data point.
  • use float rather than double. double precision floating point numbers are not needed to handle VE.Direct values. handling double is implemented in software and hence much more resource intensive.
  • make state machine timeout robust against overflow.

Closes #774 and #689.

@schlimmchen
Copy link
Member Author

@bikerbiker12 @SW-Niko Könnt ihr bitte die Firmware aus dem PR Build Run testen? Einfach die passende Variante herunterladen und die .bin Datei aus dem ZIP-Archiv als Firmware-Update hochladen.

Die Daten kommen allerdings trotzdem nur im Wechsel.

Das sollte mit diesem PR gelöst sein.

queue every text event until the frame was checked by it checksum. then
process the data directly into the buffer struct. do not clear the
buffer struct, so it will always include the most recent value of a
particular data point.
hand out const& to the data structs. this is possible now that this
struct is "stable", i.e., not reset regularly.
@bikerbiker12
Copy link

@bikerbiker12 @SW-Niko Könnt ihr bitte die Firmware aus dem PR Build Run testen? Einfach die passende Variante herunterladen und die .bin Datei aus dem ZIP-Archiv als Firmware-Update hochladen.

Die Daten kommen allerdings trotzdem nur im Wechsel.

Das sollte mit diesem PR gelöst sein.

Hallo zusammen,

Habe eben die Firmware aus dem PR Build Run per OTA erfolgreich aufgespielt und werde es mal beobachten. Aktuell kann ich berichten, dass nun alle Werte ohne Wechsel angezeigt werden.
Danke euch vielmals für euren Einsatz.
Wird das Problem mit folgenden Firmwareupdates behoben bleiben?

Liebe Grüße Marco

@schlimmchen
Copy link
Member Author

@bikerbiker12 Vielen Dank fürs Testen und deine Rückmeldung!

Wird das Problem mit folgenden Firmwareupdates behoben bleiben?

Ja, damit darfst du rechnen.

@schlimmchen schlimmchen force-pushed the handle-vedirect-fragmentation branch from 31cea34 to 5fa04f9 Compare March 31, 2024 09:54
@SW-Niko
Copy link

SW-Niko commented Apr 1, 2024

Mir ist nichts negatives aufgefallen aber ....
Ich hate bis jetzt kein Problem beim meinem MPPTs (100/20 und 75/15) mit fragmentierten Nachrichten.
Der Test an meinem System ist damit nicht wirklich aussagekräftig.

double precision floating point numbers are not needed to handle
VE.Direct values. handling double is implemented in software and hence
*much* more resource intensive.
the use of a #define is warranted here since it saves a lot of code
duplication and improves code readability.
@schlimmchen schlimmchen force-pushed the handle-vedirect-fragmentation branch from 5fa04f9 to 7a9a934 Compare April 1, 2024 20:17
@schlimmchen
Copy link
Member Author

Der Test an meinem System ist damit nicht wirklich aussagekräftig.

Das sehe ich anders, das heißt nämlich zumindest, dass du keine Regression gefunden hast 😊

Ich habe mir einen bestehenden Emulator umgebastelt um "gesplittete Nachrichten" zu schicken, und damit konnte ich plausibel machen, dass der Ansatz taugt. Und Marco bestätigt das auch im Betrieb an seinem echten Laderegler.

@schlimmchen schlimmchen linked an issue Apr 2, 2024 that may be closed by this pull request
@schlimmchen schlimmchen merged commit 8c6e925 into development Apr 2, 2024
10 checks passed
@schlimmchen schlimmchen deleted the handle-vedirect-fragmentation branch April 2, 2024 19:06
@poolcat4711
Copy link

@schlimmchen - Perfekt, funktioniert auch hier super. 1000 Dank!

Nur als Gedankenspiel - die Systeme die hier gerade im Aufbau sind, haben alle 2 oder noch mehr Victrons. Bisher benötigt man für mehrere Victrons (die günstigere variante ohne eigenem CAN Bus), einen eigenen ESP mit OpendtuOB und muss dann im node red zusammen rechnen. Vielleicht ist es ja ein Klax, mehrere Victrons vorzusehen.

Die Baupläne der Anlage gebe ich dir für den Projekt-Bereich sobald alles durch ist =)

@schlimmchen
Copy link
Member Author

Perfekt, funktioniert auch hier super. 1000 Dank!

Danke für die Rückmeldung.

Vielleicht ist es ja ein Klax, mehrere Victrons vorzusehen.

Meinst du vielleicht #706 (bereits veröffentlicht) und #833? Nein, das war kein Klacks, und es waren drei Entwickler beteiligt, aber ich glaube es gibt jetzt keinen Grund mehr jeden Laderegler mit einem einzelnen OpenDTU-OnBattery zu koppeln. Schlimmstensfalls einer pro Paar (wenn man alle Daten haben möchte) oder einer pro VE.Smart Netzwerk, wenn es "nur" darum geht die Leistung für den DPL zu berücksichtigen.

@poolcat4711
Copy link

Wow seid ihr fix! Ja genau darum geht es - nochmals 1000 Dank & grossartige Arbeit!

Copy link

github-actions bot commented May 3, 2024

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 May 3, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
4 participants