VeDirectFrameHandler behandelt fragmentierte Nachrichten falsch

[schlimmchen]

What happened?

@poolcat4711 berichtet in https://github.com/helgeerbe/OpenDTU-OnBattery/discussions/551#discussioncomment-8562350 plausibel, dass der VeDirectFrameHandler mit fragmentierten Paketen nicht richtig umgeht. In einem Paket stehen Infos, die im anderen fehlen, und andersherum. Da aber der frame handler nach jedem Paket den Buffer zurücksetzt und dann alle Werte aus dem Buffer in die höheren Schichten transportiert, werden so insb. gültige Werte in den höheren Schichten zurückgesetzt.

To Reproduce Bug

Einen Victron MPPT 250|85 per VE.Direct anschließen.

Expected Behavior

Wenn ein Paket vom charge controller verarbeitet wird, dann sollen die darin enthaltenen Werte an die höheren Schichten übermittelt werden, ohne anzunehmen, dass die nicht enthaltenen Werte ihre Gültigkeit verlieren.

Install Method

Pre-Compiled binary from GitHub

What git-hash/version of OpenDTU?

123456

Relevant log/trace output

22:02:39.323 > [VE.Direct] serial input (161 Bytes):
22:02:39.386 > [VE.Direct] 3a 41 32 30 30 31 30 30 33 43 46 36 30 44 30 30
22:02:39.447 > [VE.Direct] 45 42 0a 3a 41 31 32 45 45 30 30 33 45 37 30 39
22:02:39.500 > [VE.Direct] 44 0a 3a 41 44 42 45 44 30 30 38 42 30 35 46 33
22:02:39.554 > [VE.Direct] 0a 3a 41 42 42 45 44 30 30 38 32 30 31 32 30 0a
22:02:39.635 > [VE.Direct] 0d 0a 45 52 52 09 30 0d 0a 4c 4f 41 44 09 4f 4e
22:02:39.726 > [VE.Direct] 0d 0a 52 65 6c 61 79 09 4f 46 46 0d 0a 48 31 39
22:02:39.819 > [VE.Direct] 09 31 31 35 38 31 31 0d 0a 48 32 30 09 31 32 34
22:02:39.875 > [VE.Direct] 0d 0a 48 32 31 09 39 32 32 0d 0a 48 32 32 09 32
22:02:40.016 > [VE.Direct] 32 34 0d 0a 48 32 33 09 35 39 36 0d 0a 48 53 44
22:02:40.066 > [VE.Direct] 53 09 32 35 36 0d 0a 43 68 65 63 6b 73 75 6d 09
22:02:40.158 > [VE.Direct] 0f
22:02:40.222 > [Victron MPPT] Text Event PID: Value: 0XA116
22:02:40.277 > [Victron MPPT] Text Event FWE: Value: 313FF
22:02:40.338 > [Victron MPPT] Text Event SER: Value: HQ2316VWCFY
22:02:40.407 > [Victron MPPT] Text Event V: Value: 49250
22:02:40.544 > [Victron MPPT] Text Event I: Value: 0
22:02:40.653 > [Victron MPPT] Text Event VPV: Value: 3730
22:02:40.755 > [Victron MPPT] Text Event PPV: Value: 0
22:02:40.850 > [Victron MPPT] Text Event MPPT: Value: 0
22:02:40.955 > [Victron MPPT] Text Event CS: Value: 0
22:02:41.041 > [Victron MPPT] Text Event OR: Value: 0X00000001
22:02:41.158 > [VE.Direct] serial input (177 Bytes):
22:02:41.218 > [VE.Direct] 3a 41 32 30 30 31 30 30 33 44 46 36 30 44 30 30
22:02:41.275 > [VE.Direct] 45 41 0a 3a 41 31 32 45 45 30 30 33 41 37 30 41
22:02:41.348 > [VE.Direct] 31 0a 3a 41 44 42 45 44 30 30 38 37 30 35 46 37
22:02:41.402 > [VE.Direct] 0a 3a 41 42 42 45 44 30 30 37 34 30 31 32 45 0a
22:02:41.459 > [VE.Direct] 0d 0a 50 49 44 09 30 78 41 31 31 36 0d 0a 46 57
22:02:41.556 > [VE.Direct] 45 09 33 31 33 46 46 0d 0a 53 45 52 23 09 48 51
22:02:41.655 > [VE.Direct] 32 33 31 36 56 57 43 46 59 0d 0a 56 09 34 39 32
22:02:41.710 > [VE.Direct] 35 30 0d 0a 49 09 30 0d 0a 56 50 56 09 33 37 33
22:02:41.880 > [VE.Direct] 30 0d 0a 50 50 56 09 30 0d 0a 4d 50 50 54 09 30
22:02:41.964 > [VE.Direct] 0d 0a 43 53 09 30 0d 0a 4f 52 09 30 78 30 30 30
22:02:42.022 > [VE.Direct] 30 30 30 30 31 0d 0a 43 68 65 63 6b 73 75 6d 09
22:02:42.106 > [VE.Direct] 46
22:02:42.288 > [Victron MPPT] Text Event ERR: Value: 0
22:02:42.386 > [Victron MPPT] Text Event LOAD: Value: ON
22:02:42.473 > [Victron MPPT] Text Event RELAY: Value: OFF
22:02:42.578 > [Victron MPPT] Text Event H19: Value: 115811
22:02:42.694 > [Victron MPPT] Text Event H20: Value: 124
22:02:42.804 > [Victron MPPT] Text Event H21: Value: 922
22:02:42.904 > [Victron MPPT] Text Event H22: Value: 224
22:02:43.107 > [Victron MPPT] Text Event H23: Value: 596
22:02:43.208 > [Victron MPPT] Text Event HSDS: Value: 256

Anything else?

Der von @poolcat4711 vorgeschlagene Quick-Fix (temporären Buffer gar nicht zurücksetzen) ist nicht ideal, weil dann beispielsweise nicht auffällt, dass nur jedes zweite Paket richtig dekodiert wurde. Besser wäre es, alle Daten gepaart mit einem Timestamp vorzuhalten, an dem man erkennen kann, ob ein Wert gültig ist und wenn ja, wann er kam. Das hat dann aber auch Auswirkungen auf die Implementierung für den SmartShunt, weil diese Komponenten sich Code teilen... Ein Data Store wie der fürs JK BMS wäre wohl gut, aber kostet auch Zeit den zu adaptieren und einzusetzen.

[SW-Niko]

Hallo @schlimmchen , bitte die folgenden Informationen mit Vorsicht behandeln. Ich beschäftige mich erst seit kurzem mit programmieren. Mir sind beim VEDirectFrameHandler 3 Dinge aufgefallen.

1. Die Überprüfung von Zeitpunkten "if (_lastByteMillis + 500 < millis())" kann nach ca 50 Tagen zu einem Überlauf führen. Siehe: . https://www.norwegiancreations.com/2018/10/arduino-tutorial-avoiding-the-overflow-issue-when-using-millis-and-micros/ Besser ist es die Zeitdauer zu überprüfen. "if(millis() - _lastByteMillis > 500)".
2. Die Zeitdauer von 500ms reicht mach mal nicht, weil wir nicht garantieren können in spätestens 500ms, nach dem nächsten Byte zu schauen.
3. Es kann auch zu einem Pufferüberlauf im UART speicher kommen.

Aber, alle 3 Punkte sollten nicht zu falschen Daten führen. Das solle die Frame checksum abfangen.

Update: Jetzt habe ich das Problem verstanden. Das passiert ja bei gültigen Frames (checksum ok). Wenn der Fix nicht super eilig ist, dann kann ich das angehen wenn ich mit den VE.Direct hex nachrichten fertig bin.

[SW-Niko]

Ok, jetzt habe ich Zeit und eine Idee wie man das relativ einfach fixen kann.

[github-actions[bot]]

This issue 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.