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

Add schedule for planner #16091

Draft
wants to merge 22 commits into
base: master
Choose a base branch
from

Conversation

Maschga
Copy link
Contributor

@Maschga Maschga commented Sep 13, 2024

Fix #5492

Hi,
dieser Draft soll den Support für das Planen für Wochentage hinzufügen.
Dafür fehlt noch folgendes: Ich kenne mich im evcc-System nicht so gut aus. Wenn etwas fehlt, dann bitte mir mitteilen.

  • Backend-Implementierung
  • Tests schreiben
  • Bug fixen: Uncaught (in promise) Maximum recursive updates exceeded in component <ChargingPlanRepetitiveSettingsEntries>. This means you have a reactive effect that is mutating its own dependencies and thus recursively triggering itself. Possible sources include component template, render function, updated hook or watcher source function.. Auslöser ist diese Zeile.
  • (Wie) in Preview Pläne visuell anzeigen?
  • Bug fixen: Siehe Video. Das Dropdown-Menu aktualisiert nur die Daten des dazugehörigen Select-Menus, wenn auf die Checkbox geklickt wird.
bug.mp4
  • Weiteres bitte mitteilen

Vielen Dank für das tolle Projekt!
~ Maschga

@andig
Copy link
Member

andig commented Sep 14, 2024

Was wäre denn hier die Idee fürs API?

@naltatis
Copy link
Member

Wording: Ich würd hier nicht repetetive verwenden. Da steckt von der Bedeutung was "ermüdendes", "monotones" drin. Lass uns repeating nehmen. Sowohl Richtung User als auch im Code.

@naltatis naltatis self-assigned this Sep 14, 2024
@naltatis naltatis self-requested a review September 14, 2024 22:12
@naltatis naltatis added enhancement New feature or request ux User experience/ interface labels Sep 14, 2024
@andig
Copy link
Member

andig commented Sep 15, 2024

Oder einfach Wochenplaner?

@Maschga
Copy link
Contributor Author

Maschga commented Sep 15, 2024

Wie soll ich es jetzt machen? 😅

@naltatis
Copy link
Member

Lass uns bei repeating bleiben.

},
methods: {
fetchRepetitivePlans: async function () {
let response = await api.get(`/loadpoints/${this.id}/plan/repetitive`);
Copy link
Member

Choose a reason for hiding this comment

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

Die Komponente sollte sich nicht selbst die Daten holen. Das Muster was wir standardmäßig verwenden ist, dass wir Schreiboperationen via REST machen die aktualisierten Daten aber über Websockets gepusht werden. Die landen dann im globalen State. Da dieses Feature nur für Fahrzeuge mit SoC verfügbar sein wird sollten wir den Ladeplan auch direkt am Fahrzeug pushen. Siehe hier:

Bildschirmfoto 2024-09-16 um 08 35 18

Damit ist er dann auch gleich über MQTT verfügbar und wir lösen das Problem Datensync zwischen Geräten. Also mehrere offene Browserfenster.

Aktuell veröffentlichen wir den "einmaligen Plan" auch noch nicht am Fahrzeug. Das müssen wir aber in diesem Zuge mit machen. Momentan kommen die aktuellen Plandaten die für die Statusanzeige in der UI und für das Ladeplanformular verwendet werden vom Ladepunkt. Mit der Einführung von mehreren Plänen würde ich am Ladepunkt nur noch die Daten des "als nächstes anstehenden Plans" veröffentlichen. Dass kann ein einmaliger sein, aber auch eine Ladung, die aufgrund einer Wiederholungsregel ansteht. Bislang mussten wir das nicht trennen. Jetzt ist das aber erforderlich.

@@ -285,5 +296,35 @@ export default {
// TODO: handle fahrenheit
return this.fmtNumber(value, 1, "celsius");
},
getShortenedWeekdaysLabel: function (weekdays) {
Copy link
Member

Choose a reason for hiding this comment

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

Hier wären Unittests sicher gut.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@lucode
Copy link

lucode commented Sep 20, 2024

Lass uns bei repeating bleiben.

Die technisch korrekte übersetzung wäre meins wissens "recurrent". ;)

});
test("should format single days and ranges", () => {
expect(fmt.getShortenedWeekdaysLabel([0, 1, 3, 5, 6])).eq("Mo-Di, Do, Sa-So");
expect(fmt.getShortenedWeekdaysLabel([0, 2, 3, 5, 6])).eq("Mo, Mi-Do, Sa-So");
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
expect(fmt.getShortenedWeekdaysLabel([0, 2, 3, 5, 6])).eq("Mo, Mi-Do, Sa-So");
expect(fmt.getShortenedWeekdaysLabel([0, 2, 3, 5, 6])).eq("Mo, Mi, Do, Sa, So");

Ich würd vorschlagen den Trennstrich nur zu nutzen wenn es eine durchgehende Spanne von 3+ Wochentagen gibt. In allen anderen Fällen sollten wir einfach Wochentage auflisten auch wenn das ggf. mal länger ist.

});
test("should format ranges", () => {
expect(fmt.getShortenedWeekdaysLabel([0, 1])).eq("Mo-Di");
expect(fmt.getShortenedWeekdaysLabel([0, 1, 2, 3, 4, 5, 6])).eq("Mo-So");
Copy link
Member

Choose a reason for hiding this comment

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

Typografisch korrekt sind hier Leerzeichen und ein En Dash (). Bei der Preisspanne in den Ladepunkt- oder Batterieeinstellungen haben wir das auch so gemacht.

Klingt wie eine Kleinigkeit, aber wir haben Nutzer (mich nicht ganz ausgenommen), denen diese Details sehr wichtig sind 😄

https://github.com/evcc-io/evcc/blob/master/assets/js/components/SmartCostLimit.vue#L286

Bildschirmfoto 2024-09-23 um 10 00 03

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request ux User experience/ interface
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Planner: add schedule
4 participants