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 support for loading and storing device configuration from/to database #6199

Merged
merged 56 commits into from
Aug 17, 2023

Conversation

andig
Copy link
Member

@andig andig commented Feb 11, 2023

Refs #6029

This PR adds database storage for devices. Any device from the database is appended to the respective configProvider.

  • make devices ordered (order of config file, then order by database id)
  • add GET config/devices/<class> api getting devices
  • add PUT config/devices/<class>/<id> api for creating updating device (https://stackoverflow.com/questions/28459418/use-of-put-vs-patch-methods-in-rest-api-real-life-scenarios)
  • add POST config/devices/<class> api for creating new device
  • add DELETE config/devices/<class> @andig
  • add name key for db-based config (use db:<id>)
  • ensure PUT api processes correct database ids
  • use products api and single template requests in UI (fixes Experimental: unsichtbare Einträge in Herstellerliste bei Fahrzeug hinzufügen #6239)
  • stable sorting of device response
  • add UI to create and update vehicles @naltatis
  • when switching templates (e.g. VW ID vs others) keep populated field values for same keys @naltatis (don't think that's always wanted because fieldnames might mean different things in different templates. Stying with the safe "reset everything" way for now.
  • reflect updated values (PUT) in further get calls (update global config, not only database) @andig
  • identify configurable devices, use device id in api (GET/PUT/DELETE) @andig
  • use id for POST/DELETE in UI @naltatis
  • handle secrets (don't send to ui, keep internally, use when testing and secret param empty/missing) @andig
  • refactor database api
  • reduce pr diff
  • optional: track dirty state requiring restart
  • optional: handle errors during device creation differently from testing

Screenshots

navigation
Bildschirmfoto 2023-06-21 um 21 01 00

config overview (vehicles only at the moment)
Bildschirmfoto 2023-06-21 um 20 55 27

add/edit vehicle forms
Bildschirmfoto 2023-06-21 um 20 58 03

Bildschirmfoto 2023-06-21 um 20 58 27

Bildschirmfoto 2023-06-21 um 20 58 57

@andig andig added the enhancement New feature or request label Feb 11, 2023
@andig andig marked this pull request as draft February 11, 2023 14:59
@andig andig mentioned this pull request Feb 11, 2023
22 tasks
@andig andig force-pushed the feature/load-store-devices branch from 696c32b to 6cc3dfb Compare February 11, 2023 20:17
@andig andig marked this pull request as ready for review February 11, 2023 23:45
@andig andig force-pushed the feature/load-store-devices branch from bd5b2bf to 79c0178 Compare February 12, 2023 12:19
@andig andig marked this pull request as draft February 18, 2023 13:27
@naltatis
Copy link
Member

naltatis commented Mar 8, 2023

Ich hab versucht die UI hier anzuschließen, hab allerdings noch ein paar grundsätzliche Probleme mit diesem Stand

1. Fehler beim Starten wenn ein Fahrzeug in der DB angelegt wurde

POST http://localhost:7070/api/config/devices/vehicle
{ "template": "offline", "title": "Hallo" }

liefert mir einen 200 mit einer Response

{ "result": { "id": 1 } }

Das passt soweit. Unter der id 1 ist dann auch ein Eintrag in der DB. Starte ich evcc dann allerdings neu bekomme ich einen Fehler:

[main  ] FATAL 2023/03/08 16:53:33 cannot create charger 'db:1': cannot create charger 'template': template not found: offline

Hier versucht er scheinbar einen Charger zu initialisieren. Ich habe aber ein Vehicle erzeugt. Nach Löschen des Eintrags in der DB gehts wieder.

2. Bestehendes Fahrzeug kann nicht editiert werden
Ich habe es nicht hinbekommen ein bestehendes Fahrzeug per PUT zu verändern.

PUT http://localhost:7070/api/config/devices/vehicle/1
{ "template": "offline", "title": "Hallo v2" }

Hier bekomme ich 400 Bad Request und ein invalid type.
Wenn ich den Code richtig lese ist die id im Pfad momentan nicht die DB-ID sondern ein Index in der gesamten Fahrzeugliste und über den Namen wir die DB-ID ermittelt. Ich würd hier gerne direkt auf die Datenbank-ID. Ich hab für die Fahrzeug den rowID Lookup probeweise mal ausgebaut. Das hat meinen Fehler aber leider nicht behoben (record not found). Vielleicht magst du da noch mal drauf schauen was da schief geht.

3. IDs in Fahrzeugliste aufnehmen
Momentan liefert der Endpunkt für die Fahrzeugliste alle Fahrzeuge (yaml+db) aus. Das find ich gut. Allerdings sollten wir die Einträge, die aus der DB kommen, also editierter sind, kennzeichnen. Würde vorschlagen denen ein ID-Feld zu verpassen.

GET http://localhost:7070/api/config/devices/vehicle
{
  "result": [
    {
      "title": "weißes Model 3",
      "type": "tesla"
      ...
    },
    ...
    {
  >>> "id": 1, <<<
      "title": "Hallo",
      "type": "template",
      "template": "offline",
    },
  ]
}

4. Maybe Cascade Delete
Bei der devices zu device_details Beziehung fänd ich gut wenn wir ein on delete cascade https://www.sqlite.org/foreignkeys.html#fk_actions an dem Fremdschlüssel packen. Hatte beim Testen mehrfache den Fall, dass der Datenstand inkonsistent war weil ich nicht immer devices und device_details in sync hatte. Ich kenn den Go OR Mapper nicht. Falls das problematisch ist können wir das auch so lassen wie es ist. Normale User sollten ja nicht in der DB rumfummeln müssen.

@andig
Copy link
Member Author

andig commented Mar 9, 2023

  1. Maybe Cascade Delete

Done, but due to go-gorm/gorm#5559 you'll need to drop the FKs or tables once.

@andig
Copy link
Member Author

andig commented Mar 9, 2023

  1. Fehler beim Starten wenn ein Fahrzeug in der DB angelegt wurde

Ist auch klar und behoben:

case config.Vehicle:
	var v api.Vehicle
	if v, err = vehicle.NewFromConfig(named.Type, req); err == nil {
		id, err = config.AddDevice( --> config.Charger <-- , named.Type, req)

Das ist in Summe noch viel zuviel Code und damit auch zu fehleranfällig- da muss ein besseres API here.

  1. Bestehendes Fahrzeug kann nicht editiert werden

Das ist subtiler, da muss ich nochmal rein schauen.

UPDATE

Wenn ich den Code richtig lese ist die id im Pfad momentan nicht die DB-ID sondern ein Index in der gesamten Fahrzeugliste und über den Namen wir die DB-ID ermittelt. Ich würd hier gerne direkt auf die Datenbank-ID. I

Nimm mal bitte den Index in der Liste- das sollte gehen, jdfls in meinem Test nachdem ein Vehicle auch als solches angelegt wurde.

@andig
Copy link
Member Author

andig commented Mar 10, 2023

  1. Bestehendes Fahrzeug kann nicht editiert werden

Das ist subtiler, da muss ich nochmal rein schauen.

Ursache auch gefunden. Bei PUT wird die Datenbank aktualisiert, aber nicht die schon geladene Config. Nach Neustart scheinen die richtigen Werte zu kommen. Sollte vllt. gut genug sein um mit Prototyping weiter zu machen, schreit aber danach da mal aufzuräumen.

@andig andig force-pushed the feature/load-store-devices branch from a65b3aa to 29d95a1 Compare March 10, 2023 17:03
@andig andig changed the title Add support for loading and storing configuration from database Add support for loading and storing device configuration from/to database Mar 10, 2023
@andig andig force-pushed the feature/load-store-devices branch from 3235252 to ea45947 Compare March 13, 2023 11:15
@naltatis
Copy link
Member

naltatis commented Apr 2, 2023

I removed the "Add vehicle" UI from the main screen and moved it to a new separate configuration page. This page is accessible through the menu when experimental 🧪 is enabled.

I've also added a lot of not-yet-working page structure (grid, pv, ...) to show how the settings could be structured. Currently only vehicle is functioning.

Bildschirmfoto 2023-04-03 um 00 18 41

Bildschirmfoto 2023-04-03 um 00 18 54

config.ui.mp4

@andig Update calls work and write to the database. However the updated values (e.g. title) are not updated in the go runtime. You need to restart the server in order to see new values.
I also notices strange resorting of vehicles on server restart. Maybe we need explizit sorting (by id) to make this stable.

@andig andig force-pushed the feature/load-store-devices branch from ea45947 to 872af34 Compare April 4, 2023 12:16
@andig
Copy link
Member Author

andig commented Apr 4, 2023

Rebased

@naltatis
Copy link
Member

naltatis commented Apr 4, 2023

Hab ich doch in der Tat vergessen zu pushen. Sag doch was :D

@naltatis
Copy link
Member

@andig hattest du Zeit dir meine Änderungen mal anzuschauen?

@github-actions github-actions bot added the stale Outdated and ready to close label May 2, 2023
@github-actions github-actions bot closed this May 7, 2023
@andig andig reopened this May 7, 2023
@github-actions github-actions bot removed the stale Outdated and ready to close label May 8, 2023
@andig andig force-pushed the feature/load-store-devices branch from 1726571 to 9a10a47 Compare May 20, 2023 13:56
@andig andig force-pushed the feature/load-store-devices branch from 125e626 to 8835cc7 Compare May 21, 2023 10:41
util/templates/defaults.yaml Outdated Show resolved Hide resolved
util/templates/defaults.yaml Outdated Show resolved Hide resolved
cmd/tariff.go Show resolved Hide resolved
@andig
Copy link
Member Author

andig commented Aug 13, 2023

Merging hidden properties (aka passwords) is done. Available for testing device if id is specified.

@andig andig force-pushed the feature/load-store-devices branch from 64bb392 to a359293 Compare August 17, 2023 17:55
@andig andig marked this pull request as ready for review August 17, 2023 17:55
@andig andig merged commit 0854849 into master Aug 17, 2023
@andig andig deleted the feature/load-store-devices branch August 17, 2023 17:59
@StevieC121176
Copy link

StevieC121176 commented Aug 18, 2023

Ich habe heute einige meiner Fahrzeuge aus der .yaml entfernt und über die UI Config neu angelegt. Das hat sehr gut funktioniert. Look & Feel ist hervorragend.
Eine Kleinigkeit fand ich allerdings etwas irritierend: Mein E-Bike hat eine Kapazität von 0,750 kWh. Überall in EVCC wird auch "," angezeigt. Nur bei der Eingabe der Kapazität muss ich 0.75 eingeben. Warum hier mit "." anstatt "," Zumal der "." auch bei 1000er genutzt wird "4.760 km"
Was noch schön wäre, diese Zusatzinfos wie im 1. Post.

@naltatis
Copy link
Member

Danke für das Feedback und guter Punkt mit der nummerneingabe. Eingabe sollte auf jeden Fall anhand der aktuellen locale interpretiert werden. Magst du nen Bug dazu anlegen?

@StevieC121176
Copy link

erl.

2 Kleinigkeiten sind mir noch aufgefallen:

-im Darkmode ist die Buttonbeschriftung "Abbrechen" kaum zu erkennen (Schriftfarbe)

IMG_1376
IMG_1377

  • Der Gleichheit wegen wäre es evtl. besser anstatt des "<" Symbols das "Home" Symbol wie im Ladelog zu verwenden. Führt in beiden Fällen zum Homebildschirm

IMG_1378
IMG_1379

@naltatis
Copy link
Member

@StevieC121176 Danke. Ja die Breadcrumb ist in der Zwischenzeit geredesigned worden. Das kommt davon wenn ein PR so lange läuft. Aber ein einfacher Fix :D

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

Successfully merging this pull request may close these issues.

Experimental: unsichtbare Einträge in Herstellerliste bei Fahrzeug hinzufügen
3 participants