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

Kostal: fix setting battery limit #10899

Merged
merged 3 commits into from
Nov 26, 2023
Merged

Kostal: fix setting battery limit #10899

merged 3 commits into from
Nov 26, 2023

Conversation

andig
Copy link
Member

@andig andig commented Nov 24, 2023

TODO

  • timeout (max 60s)

/cc @premultiply das ist ein hässlicher Hack für float32lswfirst. Eigentlich müssten wir die ganze Modbus Integration umbiegen und das Bestimmen der eigentlichen Operation bis zum Schluss lassen oder auch für Writes die entsprechenden Transformationen einführen.

@andig andig added the bug Something isn't working label Nov 24, 2023
@premultiply
Copy link
Member

Sieh es doch erstmal nur als proof-of-concept an. Ums schön machen bzw. die Verallgemeinerung können wir uns kümmern wenn das überhaupt so funktioniert.

@deadrabbit87
Copy link
Contributor

Auch auf die Gefahr hin, dass ich nerve:

Ihr könntet 1036 als alleinige Adresse verwenden. Schreibt man hier 0, macht der WR gar nichts mehr und sperrt die Entladung / Ladung. Bei -100 lädt er mit der technisch max. möglichen Leistung und bei 100 entlädt er die Batterie. Ich sehe nur die 100 als kritisch an, weil man da ja dann von der Batterie ins Netz einspeisen könnte...

Was rein das schreiben der Adresse betrifft funktioniert der PR so wie er soll. Es muss nur minimum alle 60 s passieren, sonst fällt der WR auf die interne Regelung zurück.

@premultiply
Copy link
Member

Die Anforderung war und Zielsetzung ist nur das Entladen zu unterbinden, nicht alles abzuschalten.

@andig
Copy link
Member Author

andig commented Nov 24, 2023

Wir möchten keine Leistungssteuerung machen. Außerdem soll bei "hold" weiterhin Ladung möglich sein. Wenn 1042 das tut würde ich mir lieber etwas für den Timeout einfallen lassen.

@premultiply ich denke für den WDT an sowas wie ein neuer Plugin watchdog das einen timeout Wert bekommt. Geschrieben wird dann immer der letzte Wert alle timeout/2. Um das "nicht mehr" schreiben in den Griff zu bekommen könnten wir dem noch ein reset value verpassen. Wenn dieses geschrieben würde (oder wird, ist egal) dann wird die Goroutine für den WDT beendet. Wäre hier also der Fall, wenn minsoc geschrieben würde. Wie findest Du die Idee?

Was rein das schreiben der Adresse betrifft funktioniert der PR so wie er soll. Es muss nur minimum alle 60 s passieren, sonst fällt der WR auf die interne Regelung zurück.

@deadrabbit87 mich wundert das immer noch. Gilt das auch für 1042 so wie wir das jetzt verwenden? Welchen minsoc würde der WR denn nutzen wenn nicht den eingestellten?

@deadrabbit87
Copy link
Contributor

Das ist ein Argument...

@andig
Copy link
Member Author

andig commented Nov 24, 2023

Dann probier gerne mal den PR bevor ich ihn merge. Im nightly morgen mit

evcc meter -b normal|hold|charge

@andig andig mentioned this pull request Nov 24, 2023
1 task
@deadrabbit87
Copy link
Contributor

deadrabbit87 commented Nov 24, 2023

Der WR fällt dann zurück auf den in der Weboberfläche eingestellten Wert. In meinem Fall ist 5 % eingetragen.

Hier ein Screenshot:
grafik
Wie an der Fehlermeldung zu sehen, kann man den Timeout auf max. 60 s stellen.

Man sieht das dann auch in der Statusoberfläche, wenn von extern gesteuert wird:
grafik

Und so wenn ohne:
grafik

Hier auch noch der Auszug aus dem trace log von dem hier gebauten PR:

[modbus] TRACE 2023/11/24 13:37:19 modbus: send 00 75 00 00 00 0b 47 10 04 12 00 02 04 00 00 41 a0
[warp  ] TRACE 2023/11/24 13:37:19 recv warp2/Garage/evse/low_level_state: '{"led_state":4,"cp_pwm_duty_cycle":267,"adc_values":[3858,2974,119,140,2102,3844,122],"voltages":[11672,5973,-12432,-12297,1693,12390,-12413],"resistances":[849,688],"gpio":[false,true,false,false,true,true,false,false,false,false,false,false,false,false,false,false,true,false,false,false,false,false,false,false],"charging_time":4949,"time_since_state_change":4949,"uptime":2175976642,"time_since_dc_fault_check":69028189}'
[modbus] TRACE 2023/11/24 13:37:19 modbus: recv 00 75 00 00 00 06 47 10 04 12 00 02
[cache ] TRACE 2023/11/24 13:37:19 batteryMode: hold

Warum ihr hier 41 a0 schreibt würde ich noch gerne verstehen.

@andig
Copy link
Member Author

andig commented Nov 24, 2023

Deshalb, aber lsw first:

Screenshot 2023-11-24 at 22 55 16

@andig
Copy link
Member Author

andig commented Nov 24, 2023

Watchdog kommt wenn #10906 drin ist, da gabs den Bedarf auch.

@deadrabbit87
Copy link
Contributor

Aber warum den Wert Dezimalwert 20 und nicht 100?

@deadrabbit87
Copy link
Contributor

Wieder mal DAU Frage, aber liegt das daran?
grafik
Schon oder?

@premultiply
Copy link
Member

premultiply commented Nov 24, 2023

Könnte gut sein.
Sollte eigentlich bei Modbus immer Big Endian sein.

@deadrabbit87
Copy link
Contributor

Nach allem was ich so lese, ist das nicht spezifiziert.

Hilf es euch was, wenn ich das mit ABCD mal teste?

@deadrabbit87
Copy link
Contributor

Dann probier gerne mal den PR bevor ich ihn merge. Im nightly morgen mit

Wie kann ich diesen Branch auf das aktuelle Nightly bringen? Dann kann ich gerne testen.

@andig
Copy link
Member Author

andig commented Nov 25, 2023

Hilf es euch was, wenn ich das mit ABCD mal teste?

das kannst du mit jedem beliebigen Modbus Tool testen. Ansonsten wie immer: repo clonen, Branch auschecken, bauen. Wenn nicht möglich auf Merge warten.

@deadrabbit87
Copy link
Contributor

Ich glaub wir reden aneinander vorbei, oder, was noch wahrscheinlicher ist, ich kenn mich da mit GitHub zu wenig aus. In diesem Branch ist dieser PR #10901 noch nicht drin.

Daher bringt das nix, wenn ich den auschecke und baue, weil ich das so

evcc meter -b normal|hold|charge

eben icht testen kann.

Diese PR ohne cli habe ich ja schon getestet, siehe dazu auch die E-Mail von mir.

@andig andig merged commit c6a94c1 into master Nov 26, 2023
6 checks passed
@andig andig deleted the fix/kostal branch November 26, 2023 04:53
@andig
Copy link
Member Author

andig commented Nov 26, 2023

BigEndian ist jetzt mandatory

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants