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

RCT: add battery control #18178

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
Open

RCT: add battery control #18178

wants to merge 10 commits into from

Conversation

andig
Copy link
Member

@andig andig commented Jan 11, 2025

Depends on mlnoga/rct#18

@andig andig added the devices Specific device support label Jan 11, 2025
@andig andig changed the title Feat/rct battery RCT: add battery control Jan 11, 2025
@andig
Copy link
Member Author

andig commented Jan 12, 2025

@Maschga could you give this PR a try? I'm not sure I'm calling the right functions for hold/normal. Would be great if you could test this with an actual device.

@andig andig marked this pull request as ready for review January 12, 2025 13:56
@Maschga
Copy link
Contributor

Maschga commented Jan 12, 2025

Normal looks good. 👍

Why do you use SOCTargetConstant for Hold?
According to do-gooder it should be SOCTargetExternal. Is SOCTargetConstant better? What is the effect?
Source:

return m.conn.Write(rct.PowerMngSocStrategy, []byte{rct.SOCTargetConstant})

@andig
Copy link
Member Author

andig commented Jan 12, 2025

What is the effect?

I wouldn't know. It sounded as if the RCT would then hold the soc constant. However, that's also not what we want. Hold should prevent discharge, but still allow charge. Not sure how to best do this. I guess SOCTargetExternal with given Soc would still have the same problem.

@Maschga
Copy link
Contributor

Maschga commented Jan 12, 2025

Is it evcc possible to calculate how much surplus is available in addition to charging the e-car and how many kw could still charge the battery? Then this could be added using power_mng.battery_power_extern.

@andig
Copy link
Member Author

andig commented Jan 12, 2025

much surplus is available in addition to charging the e-car and how many kw could still charge the battery? Then this could be added using power_mng.battery_power_extern.

Why? Battery does that itself?!

@Maschga
Copy link
Contributor

Maschga commented Jan 12, 2025

I have set power_mng.soc_strategy to Constant. This results in the target Soc of the battery being set to 50%. RCT then wanted to discharge the battery, which is currently full.

@andig
Copy link
Member Author

andig commented Jan 12, 2025

So: what to use for hold (but still charge surplus)? You'll need to tell me- I can't test this...

@TobiasHuber1980
Copy link
Contributor

TobiasHuber1980 commented Jan 12, 2025

Keine Ahnung, aber vielleicht hilft es euch ja...

External battery control is possible when the SoC target selection is set to "Extern":

Parameter: 0xF168B748 // power_mng.soc_strategy | t_enum (uint8_t) | SoC target selection

The following six values are possible:

0 – SoC target = SoC

1 – Const

2 – Extern

3 – Average Battery voltage

4 – Intern (default)

5 – Timetable

The power_mng.battery_power_extern parameter allows external control of the desired battery power:

0xBD008E29 // power_mng.battery_power_extern | t_float | Battery target power [W] (positive = discharge)

@Maschga
Copy link
Contributor

Maschga commented Jan 12, 2025

You could set power_mng.soc_min to the current SOC value of the battery. This will prevent the battery from discharging, but the battery can still charge if there is excess power available.

@andig
Copy link
Member Author

andig commented Jan 12, 2025

Welches Datenformat braucht da der Parameter? 1 Byte? Float32?

@Maschga
Copy link
Contributor

Maschga commented Jan 12, 2025

Laut rctclient ist das ein float.
So sieht der Code im PR aus:

// SetSocMin sets the minimum SOC target (power_mng.soc_min) with the given value
func (c *Connection) SetSocMin(min float32) error {
	if min < 0.00 || min > 1.00 {
		return fmt.Errorf("invalid SOC min value: %.2f, valid range is 0.00 to 1.00", min)
	}

	// Round to 2 decimal places
	min = float32(math.Round(float64(min)*100) / 100)

	data := make([]byte, 4)
	binary.BigEndian.PutUint32(data, math.Float32bits(min))

	if err := c.Write(PowerMngSocMin, data); err != nil {
		return fmt.Errorf("failed to set SOC min: %w", err)
	}

	return nil
}

@Maschga
Copy link
Contributor

Maschga commented Jan 12, 2025

Das Runden der Zahl kannst du weglassen. Das ändere ich noch im PR.

@andig
Copy link
Member Author

andig commented Jan 12, 2025

Du kannst nochmal probieren. Wenn das nicht funzt gerne nach Bedarf anpassen.

@Maschga
Copy link
Contributor

Maschga commented Jan 12, 2025

Kannst du für minsoc einen Default von 7% (also 0.07) und für maxsoc einen Default von 97% (also 0.97) angeben?

@andig
Copy link
Member Author

andig commented Jan 12, 2025

Das kannst du im Template ändern.

@andig
Copy link
Member Author

andig commented Jan 12, 2025

Braucht man für charge überhaupt external? Sonst könnte man da auch einfach nur den minsoc setzen und müsste gar nicht mit dem Modus rumfrickeln...

@Maschga
Copy link
Contributor

Maschga commented Jan 12, 2025

@andig Kannst du mir für diesen PR Schreibzugriff geben? Würde gerne eine Implementierung committen.

@Maschga Maschga mentioned this pull request Jan 12, 2025
@Maschga
Copy link
Contributor

Maschga commented Jan 12, 2025

Hab's mit einem separaten PR gelöst.

* Update dependency

* fix BatteryCharge

* add defaults

* add comment
@Maschga
Copy link
Contributor

Maschga commented Jan 12, 2025

Super, das muss dann nur noch endgültig getestet werden.

@andig
Copy link
Member Author

andig commented Jan 12, 2025

Kannst du das testen?

@andig
Copy link
Member Author

andig commented Jan 12, 2025

Argh. Warum brauchts diese ganzen Werte? Ich möchte die nicht setzen falls es nicht absolut notwendig ist. Ich würde das gerne rückgängig machen, es sei denn es gäbe einen zwingenden Grund dafür.

@Maschga
Copy link
Contributor

Maschga commented Jan 13, 2025

Um RCT zum Laden aus dem Netz zu bekommen, muss man ein bisschen herumexperimentieren. Das ist leider kein Feature von RCT.
Ich kann noch eine andere Möglichkeit ausprobieren, vielleicht muss man mit der nicht so viele Werte setzen. Aber ich glaube, dass für das Laden immer mindestens zwei oder drei Werte gesetzt werden müssen.

@Maschga
Copy link
Contributor

Maschga commented Jan 13, 2025

Wird NewRCT() alle 30s im Intervall aufgerufen oder nur, wenn sich der Wert von api.BatteryMode ändert?

@andig
Copy link
Member Author

andig commented Jan 13, 2025

NewRCT wird nur beim Start von evcc aufgerufen.

@andig andig marked this pull request as draft January 13, 2025 07:49
@Maschga
Copy link
Contributor

Maschga commented Jan 13, 2025

Ach, Entschuldigung, ich meinte die Funktion batteryMode.

@andig
Copy link
Member Author

andig commented Jan 13, 2025

Die wird wie die meisten Funktionen in evcc nur aufgerufen wenn es Änderungen gibt.

@Maschga
Copy link
Contributor

Maschga commented Jan 13, 2025

Ist es möglich, dass von BatteryCharge direkt auf BatteryHold gesprungen wird oder gibt es Fälle, die nicht eintreten können?

@andig
Copy link
Member Author

andig commented Jan 13, 2025

Alle Kombinationen sind möglich, z.b. wenn sich die Preise ändern oder ein Ladevorgang startet.

* Update dependency

* fix BatteryCharge

* add defaults

* add comment

* change battery mode logic
@Maschga
Copy link
Contributor

Maschga commented Jan 14, 2025

Das ist die beste Lösung, die ich finden konnte.
Es sollte funktionieren, ich konnte es bisher nur mit der RCT App am Gerät testen, aber noch nicht mit evcc selbst.
Die Aufrufe sind aber die gleichen.

@Maschga
Copy link
Contributor

Maschga commented Jan 14, 2025

@andig Soll es noch mit evcc getestet werden? Ich habe keinen dynamischen Tarif.

@Maschga
Copy link
Contributor

Maschga commented Jan 15, 2025

Soll ich noch einen PR erstellen, um den Lint-Test zu fixen?

@andig
Copy link
Member Author

andig commented Jan 15, 2025

Fixed. You can avoid this using VScode with Go plugin.

meter/rct.go Show resolved Hide resolved
meter/rct.go Show resolved Hide resolved
@andig andig marked this pull request as ready for review January 15, 2025 09:18
@andig andig self-assigned this Jan 15, 2025
@Maschga
Copy link
Contributor

Maschga commented Jan 20, 2025

@andig Mir fällt noch eines auf: Die RCT-Batterie macht periodisch Kalibrierungen. Wenn die Batterie gerade in diesem Modus ist (sei es Kalibrierung oder Balancing), dann wäre es super, wenn evcc auf BatteryNormal geht, ansonsten wird die Kalibrierung gestört.
Das lässt sich damit abfragen:
0x71765BD8 INT32 INT32 battery.status Battery status

Hier werden die zwei wichtigen Werte erklärt.

Ich kann gerne im mlnoga-PR die Werte zum Lesen hinzufügen, aber ich weiß nicht, wo die Logik in evcc am Besten hinpasst. Kannst du mir da helfen?

@andig
Copy link
Member Author

andig commented Jan 20, 2025

Leider nein- das passt nicht in die Logik. Man könnte allenfalls den Start von Charge/Hold mit Fehlermeldung verhindern.

@Maschga
Copy link
Contributor

Maschga commented Jan 20, 2025

Dass die Batterie die Kalibrierung ungestört machen kann, wäre sehr wichtig.
Dann auch gerne die Implementierung mit Fehlermeldungen.

@Maschga
Copy link
Contributor

Maschga commented Jan 20, 2025

Dann gibt es wahrscheinlich auch keine Logik, um zum Beispiel 1 Minute bevor die Kalibrierung startet in den BatteryNormal Modus zu wechseln?

@Maschga
Copy link
Contributor

Maschga commented Jan 20, 2025

Und meine letzter Gedanke: Vielleicht sollte im Fall, dass der Inselmodus eingetreten ist, die Batterie auch nur im Modus BatteryNormal bleiben. Sorry für die ganzen Spezialfälle.

@Maschga
Copy link
Contributor

Maschga commented Jan 20, 2025

Kann ich dafür einen PR machen?

@andig
Copy link
Member Author

andig commented Jan 20, 2025

Für "Modus nicht starten" gerne. Einen Hintergrundprozess der Modi wieder beendet würde ich nicht sehen oder wäre zumindest nicht RCT-spezifisch. Dafür fehlt mir noch der echte Bedarf.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
devices Specific device support
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants