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

UI configurable vehicle settings (onIdentify replacement) #9684

Closed
wants to merge 1 commit into from

Conversation

naltatis
Copy link
Member

@naltatis naltatis commented Sep 2, 2023

Determine minSoc and targetSoc based on system state (check vehicle, check loadpoint, use defaults) instead of event-based copying of values.

fixes #9641

@@ -184,6 +184,10 @@ type Vehicle interface {
IconDescriber
Title() string
SetTitle(string)
DefaultTargetSoc() int
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
DefaultTargetSoc() int
MaxSoc() int

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Der Wert ist- bezogen auf das Fahrzeug- auch nicht mehr "default" als andere Werte?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generell: wir sollten uns mal einigen, ob getter oder setter einen Präfix bekommen. Im Moment ists Wildwuchs.

@@ -184,6 +184,10 @@ type Vehicle interface {
IconDescriber
Title() string
SetTitle(string)
DefaultTargetSoc() int
SetDefaultTargetSoc(int)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
SetDefaultTargetSoc(int)
SetMaxSoc(int)

MinSoc_ int `mapstructure:"minSoc"`
Identifiers_ []string `mapstructure:"identifiers"`
Features_ []api.Feature `mapstructure:"features"`
OnIdentify api.ActionConfig `mapstructure:"onIdentify"`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Brauchts das jetzt noch? Für den Modus? Den könnten wir dann auch direkt noch in ein API setzen.

@@ -59,6 +59,10 @@ type API interface {
GetTargetSoc() int
// SetTargetSoc sets the charge target soc
SetTargetSoc(int)
// GetDefaultTargetSoc returns the vehicles default target soc
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// GetDefaultTargetSoc returns the vehicles default target soc
// GetSessionTargetSoc returns the vehicles default target soc

Konsistenz!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Getter/Setter Präfix

@andig
Copy link
Member

andig commented Sep 17, 2023

@naltatis ich habe #9641 (comment) nochmal überarbeitet. M.E. sollten wir diesen PR aufsplitten in:

  • Defaultwerte am Fahrzeug (aus dem Yaml wie heute)
  • Session Soc am Loadpoint (mittels API wie heute)

Für Config UI brauchts es dann auch ein API fürs Fahrzeug, das wäre aber ein zweiter Schritt. Das würde auch einen partiellen revert von #8115 bedeuten, was m.E. aber verschmerzbar ist.

Benefit: erster Schritt einfacher und schneller. Was meinst Du?

@naltatis
Copy link
Member Author

Ich verstehen noch nicht genau wie du das aufteilen würdest. Was genau wäre denn die Änderung die wir in Schritt 1 machen? Das Kopieren ersetzen durch das Stufenweise Abfragen (1. Session Wert am LP, 2. Default-Wert am Fahrzeug, 3. Fallback 100%)?

@github-actions github-actions bot added the stale Outdated and ready to close label Oct 9, 2023
@github-actions github-actions bot closed this Oct 15, 2023
@andig andig mentioned this pull request Oct 15, 2023
14 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale Outdated and ready to close
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Set soc limit to a default value when a vehicle is connected
2 participants