From d09177b4b321a750eb2461bd5717ef439dfd37d5 Mon Sep 17 00:00:00 2001 From: andig Date: Mon, 17 Mar 2025 21:17:46 +0100 Subject: [PATCH] Config UI: remove references when deleting devices (#19832) Co-authored-by: Michael Geers --- core/keys/loadpoint.go | 3 + core/loadpoint/api.go | 24 ++++-- core/loadpoint/mock.go | 84 ++++++++++++------ core/loadpoint_api.go | 65 ++++++++++++-- server/http_config_device_handler.go | 45 ++++++++-- server/http_config_loadpoint_handler.go | 35 +++++++- tests/config-loadpoint.spec.js | 110 ++++++++++++++++++++++++ 7 files changed, 319 insertions(+), 47 deletions(-) diff --git a/core/keys/loadpoint.go b/core/keys/loadpoint.go index 3892880d4..f3de8c324 100644 --- a/core/keys/loadpoint.go +++ b/core/keys/loadpoint.go @@ -5,6 +5,9 @@ const ( Title = "title" // loadpoint title Mode = "mode" // charge mode DefaultMode = "defaultMode" // default charge mode + Charger = "charger" // charger ref + Meter = "meter" // meter ref + DefaultVehicle = "vehicle" // default vehicle ref Priority = "priority" // priority MinCurrent = "minCurrent" // min current MaxCurrent = "maxCurrent" // max current diff --git a/core/loadpoint/api.go b/core/loadpoint/api.go index 8622961b0..c8e04458f 100644 --- a/core/loadpoint/api.go +++ b/core/loadpoint/api.go @@ -26,16 +26,24 @@ type API interface { // references // - // GetCharger returns the loadpoint charger - GetChargerName() string - // GetMeter returns the loadpoint meter - GetMeterName() string - // GetCircuitName returns the loadpoint circuit name - GetCircuitName() string + // TODO SetCircuitRef + + // GetChargerRef returns the loadpoint charger + GetChargerRef() string + // SetChargerRef sets the loadpoint charger + SetChargerRef(string) + // GetMeterRef returns the loadpoint meter + GetMeterRef() string + // SetMeterRef sets the loadpoint meter + SetMeterRef(string) + // GetCircuitRef returns the loadpoint circuit name + GetCircuitRef() string // GetCircuit returns the loadpoint circuit GetCircuit() api.Circuit - // GetDefaultVehicle returns the loadpoint default vehicle - GetDefaultVehicle() string + // GetDefaultVehicleRef returns the loadpoint default vehicle + GetDefaultVehicleRef() string + // SetDefaultVehicleRef sets the loadpoint default vehicle + SetDefaultVehicleRef(string) // // settings diff --git a/core/loadpoint/mock.go b/core/loadpoint/mock.go index 80275e26e..0f902e8a9 100644 --- a/core/loadpoint/mock.go +++ b/core/loadpoint/mock.go @@ -167,18 +167,18 @@ func (mr *MockAPIMockRecorder) GetChargePowerFlexibility(rates any) *gomock.Call return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetChargePowerFlexibility", reflect.TypeOf((*MockAPI)(nil).GetChargePowerFlexibility), rates) } -// GetChargerName mocks base method. -func (m *MockAPI) GetChargerName() string { +// GetChargerRef mocks base method. +func (m *MockAPI) GetChargerRef() string { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GetChargerName") + ret := m.ctrl.Call(m, "GetChargerRef") ret0, _ := ret[0].(string) return ret0 } -// GetChargerName indicates an expected call of GetChargerName. -func (mr *MockAPIMockRecorder) GetChargerName() *gomock.Call { +// GetChargerRef indicates an expected call of GetChargerRef. +func (mr *MockAPIMockRecorder) GetChargerRef() *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetChargerName", reflect.TypeOf((*MockAPI)(nil).GetChargerName)) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetChargerRef", reflect.TypeOf((*MockAPI)(nil).GetChargerRef)) } // GetCircuit mocks base method. @@ -195,18 +195,18 @@ func (mr *MockAPIMockRecorder) GetCircuit() *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetCircuit", reflect.TypeOf((*MockAPI)(nil).GetCircuit)) } -// GetCircuitName mocks base method. -func (m *MockAPI) GetCircuitName() string { +// GetCircuitRef mocks base method. +func (m *MockAPI) GetCircuitRef() string { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GetCircuitName") + ret := m.ctrl.Call(m, "GetCircuitRef") ret0, _ := ret[0].(string) return ret0 } -// GetCircuitName indicates an expected call of GetCircuitName. -func (mr *MockAPIMockRecorder) GetCircuitName() *gomock.Call { +// GetCircuitRef indicates an expected call of GetCircuitRef. +func (mr *MockAPIMockRecorder) GetCircuitRef() *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetCircuitName", reflect.TypeOf((*MockAPI)(nil).GetCircuitName)) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetCircuitRef", reflect.TypeOf((*MockAPI)(nil).GetCircuitRef)) } // GetDefaultMode mocks base method. @@ -223,18 +223,18 @@ func (mr *MockAPIMockRecorder) GetDefaultMode() *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetDefaultMode", reflect.TypeOf((*MockAPI)(nil).GetDefaultMode)) } -// GetDefaultVehicle mocks base method. -func (m *MockAPI) GetDefaultVehicle() string { +// GetDefaultVehicleRef mocks base method. +func (m *MockAPI) GetDefaultVehicleRef() string { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GetDefaultVehicle") + ret := m.ctrl.Call(m, "GetDefaultVehicleRef") ret0, _ := ret[0].(string) return ret0 } -// GetDefaultVehicle indicates an expected call of GetDefaultVehicle. -func (mr *MockAPIMockRecorder) GetDefaultVehicle() *gomock.Call { +// GetDefaultVehicleRef indicates an expected call of GetDefaultVehicleRef. +func (mr *MockAPIMockRecorder) GetDefaultVehicleRef() *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetDefaultVehicle", reflect.TypeOf((*MockAPI)(nil).GetDefaultVehicle)) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetDefaultVehicleRef", reflect.TypeOf((*MockAPI)(nil).GetDefaultVehicleRef)) } // GetDisableDelay mocks base method. @@ -349,18 +349,18 @@ func (mr *MockAPIMockRecorder) GetMaxPhaseCurrent() *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetMaxPhaseCurrent", reflect.TypeOf((*MockAPI)(nil).GetMaxPhaseCurrent)) } -// GetMeterName mocks base method. -func (m *MockAPI) GetMeterName() string { +// GetMeterRef mocks base method. +func (m *MockAPI) GetMeterRef() string { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GetMeterName") + ret := m.ctrl.Call(m, "GetMeterRef") ret0, _ := ret[0].(string) return ret0 } -// GetMeterName indicates an expected call of GetMeterName. -func (mr *MockAPIMockRecorder) GetMeterName() *gomock.Call { +// GetMeterRef indicates an expected call of GetMeterRef. +func (mr *MockAPIMockRecorder) GetMeterRef() *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetMeterName", reflect.TypeOf((*MockAPI)(nil).GetMeterName)) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetMeterRef", reflect.TypeOf((*MockAPI)(nil).GetMeterRef)) } // GetMinCurrent mocks base method. @@ -669,6 +669,18 @@ func (mr *MockAPIMockRecorder) SetBatteryBoost(enable any) *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetBatteryBoost", reflect.TypeOf((*MockAPI)(nil).SetBatteryBoost), enable) } +// SetChargerRef mocks base method. +func (m *MockAPI) SetChargerRef(arg0 string) { + m.ctrl.T.Helper() + m.ctrl.Call(m, "SetChargerRef", arg0) +} + +// SetChargerRef indicates an expected call of SetChargerRef. +func (mr *MockAPIMockRecorder) SetChargerRef(arg0 any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetChargerRef", reflect.TypeOf((*MockAPI)(nil).SetChargerRef), arg0) +} + // SetDefaultMode mocks base method. func (m *MockAPI) SetDefaultMode(arg0 api.ChargeMode) { m.ctrl.T.Helper() @@ -681,6 +693,18 @@ func (mr *MockAPIMockRecorder) SetDefaultMode(arg0 any) *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetDefaultMode", reflect.TypeOf((*MockAPI)(nil).SetDefaultMode), arg0) } +// SetDefaultVehicleRef mocks base method. +func (m *MockAPI) SetDefaultVehicleRef(arg0 string) { + m.ctrl.T.Helper() + m.ctrl.Call(m, "SetDefaultVehicleRef", arg0) +} + +// SetDefaultVehicleRef indicates an expected call of SetDefaultVehicleRef. +func (mr *MockAPIMockRecorder) SetDefaultVehicleRef(arg0 any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetDefaultVehicleRef", reflect.TypeOf((*MockAPI)(nil).SetDefaultVehicleRef), arg0) +} + // SetDisableDelay mocks base method. func (m *MockAPI) SetDisableDelay(delay time.Duration) { m.ctrl.T.Helper() @@ -767,6 +791,18 @@ func (mr *MockAPIMockRecorder) SetMaxCurrent(arg0 any) *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetMaxCurrent", reflect.TypeOf((*MockAPI)(nil).SetMaxCurrent), arg0) } +// SetMeterRef mocks base method. +func (m *MockAPI) SetMeterRef(arg0 string) { + m.ctrl.T.Helper() + m.ctrl.Call(m, "SetMeterRef", arg0) +} + +// SetMeterRef indicates an expected call of SetMeterRef. +func (mr *MockAPIMockRecorder) SetMeterRef(arg0 any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetMeterRef", reflect.TypeOf((*MockAPI)(nil).SetMeterRef), arg0) +} + // SetMinCurrent mocks base method. func (m *MockAPI) SetMinCurrent(arg0 float64) error { m.ctrl.T.Helper() diff --git a/core/loadpoint_api.go b/core/loadpoint_api.go index 3fe7be5ef..1e40eae53 100644 --- a/core/loadpoint_api.go +++ b/core/loadpoint_api.go @@ -8,31 +8,84 @@ import ( "github.com/evcc-io/evcc/api" "github.com/evcc-io/evcc/core/keys" "github.com/evcc-io/evcc/core/loadpoint" + "github.com/evcc-io/evcc/core/settings" "github.com/evcc-io/evcc/core/wrapper" ) var _ loadpoint.API = (*Loadpoint)(nil) -// GetCharger returns the loadpoint charger -func (lp *Loadpoint) GetChargerName() string { +func (lp *Loadpoint) isConfigurable() bool { + _, ok := lp.settings.(*settings.ConfigSettings) + return ok +} + +// GetChargerRef returns the loadpoint charger +func (lp *Loadpoint) GetChargerRef() string { + lp.RLock() + defer lp.RUnlock() return lp.ChargerRef } +// SetChargerRef sets the loadpoint charger +func (lp *Loadpoint) SetChargerRef(ref string) { + if !lp.isConfigurable() { + lp.log.ERROR.Println("cannot set charger ref: not configurable") + return + } + + lp.Lock() + defer lp.Unlock() + lp.ChargerRef = ref + lp.settings.SetString(keys.Charger, ref) +} + // GetMeter returns the loadpoint meter -func (lp *Loadpoint) GetMeterName() string { +func (lp *Loadpoint) GetMeterRef() string { + lp.RLock() + defer lp.RUnlock() return lp.MeterRef } +// SetMeter sets the loadpoint meter +func (lp *Loadpoint) SetMeterRef(ref string) { + if !lp.isConfigurable() { + lp.log.ERROR.Println("cannot set meter ref: not configurable") + return + } + + lp.Lock() + defer lp.Unlock() + lp.MeterRef = ref + lp.settings.SetString(keys.Meter, ref) +} + // GetCircuitName returns the loadpoint circuit -func (lp *Loadpoint) GetCircuitName() string { +func (lp *Loadpoint) GetCircuitRef() string { + lp.RLock() + defer lp.RUnlock() return lp.CircuitRef } -// GetDefaultVehicle returns the loadpoint default vehicle -func (lp *Loadpoint) GetDefaultVehicle() string { +// GetDefaultVehicleRef returns the loadpoint default vehicle +func (lp *Loadpoint) GetDefaultVehicleRef() string { + lp.RLock() + defer lp.RUnlock() return lp.VehicleRef } +// SetDefaultVehicleRef returns the loadpoint default vehicle +func (lp *Loadpoint) SetDefaultVehicleRef(ref string) { + if !lp.isConfigurable() { + lp.log.ERROR.Println("cannot set default vehicle ref: not configurable") + return + } + + lp.Lock() + defer lp.Unlock() + lp.VehicleRef = ref + lp.settings.SetString(keys.DefaultVehicle, ref) +} + // GetTitle returns the loadpoint title func (lp *Loadpoint) GetTitle() string { lp.RLock() diff --git a/server/http_config_device_handler.go b/server/http_config_device_handler.go index 6f00eefd8..71a6e731e 100644 --- a/server/http_config_device_handler.go +++ b/server/http_config_device_handler.go @@ -362,17 +362,26 @@ func updateDeviceHandler(w http.ResponseWriter, r *http.Request) { jsonResult(w, res) } -func deleteDevice[T any](id int, h config.Handler[T]) error { - name := config.NameForID(id) - +func configurableDevice[T any](name string, h config.Handler[T]) (config.ConfigurableDevice[T], error) { dev, err := h.ByName(name) if err != nil { - return err + return nil, err } configurable, ok := dev.(config.ConfigurableDevice[T]) if !ok { - return errors.New("not configurable") + return nil, errors.New("not configurable") + } + + return configurable, nil +} + +func deleteDevice[T any](id int, h config.Handler[T]) error { + name := config.NameForID(id) + + configurable, err := configurableDevice(name, h) + if err != nil { + return err } if err := configurable.Delete(); err != nil { @@ -384,6 +393,8 @@ func deleteDevice[T any](id int, h config.Handler[T]) error { // deleteDeviceHandler deletes a device from database by class func deleteDeviceHandler(w http.ResponseWriter, r *http.Request) { + h := config.Loadpoints() + vars := mux.Vars(r) class, err := templates.ClassString(vars["class"]) @@ -402,12 +413,36 @@ func deleteDeviceHandler(w http.ResponseWriter, r *http.Request) { case templates.Charger: err = deleteDevice(id, config.Chargers()) + // cleanup references + for _, dev := range h.Devices() { + lp := dev.Instance() + if lp.GetChargerRef() == config.NameForID(id) { + lp.SetChargerRef("") + } + } + case templates.Meter: err = deleteDevice(id, config.Meters()) + // cleanup references + for _, dev := range h.Devices() { + lp := dev.Instance() + if lp.GetMeterRef() == config.NameForID(id) { + lp.SetMeterRef("") + } + } + case templates.Vehicle: err = deleteDevice(id, config.Vehicles()) + // cleanup references + for _, dev := range h.Devices() { + lp := dev.Instance() + if lp.GetDefaultVehicleRef() == config.NameForID(id) { + lp.SetDefaultVehicleRef("") + } + } + case templates.Circuit: err = deleteDevice(id, config.Circuits()) } diff --git a/server/http_config_loadpoint_handler.go b/server/http_config_loadpoint_handler.go index d4eb65b56..3eb2d2824 100644 --- a/server/http_config_loadpoint_handler.go +++ b/server/http_config_loadpoint_handler.go @@ -19,10 +19,10 @@ import ( func getLoadpointStaticConfig(lp loadpoint.API) loadpoint.StaticConfig { return loadpoint.StaticConfig{ - Charger: lp.GetChargerName(), - Meter: lp.GetMeterName(), - Circuit: lp.GetCircuitName(), - Vehicle: lp.GetDefaultVehicle(), + Charger: lp.GetChargerRef(), + Meter: lp.GetMeterRef(), + Circuit: lp.GetCircuitRef(), + Vehicle: lp.GetDefaultVehicleRef(), } } @@ -242,6 +242,33 @@ func deleteLoadpointHandler() http.HandlerFunc { return } + // cleanup references + lp, err := configurableDevice(config.NameForID(id), h) + if err != nil { + jsonError(w, http.StatusBadRequest, err) + return + } + + instance := lp.Instance() + + if dev, err := configurableDevice(instance.GetChargerRef(), config.Chargers()); err == nil { + if err := deleteDevice(dev.ID(), config.Chargers()); err != nil { + jsonError(w, http.StatusBadRequest, err) + return + } + + setConfigDirty() + } + + if dev, err := configurableDevice(instance.GetMeterRef(), config.Meters()); err == nil { + if err := deleteDevice(dev.ID(), config.Meters()); err != nil { + jsonError(w, http.StatusBadRequest, err) + return + } + + setConfigDirty() + } + setConfigDirty() if err := deleteDevice(id, h); err != nil { diff --git a/tests/config-loadpoint.spec.js b/tests/config-loadpoint.spec.js index ace65cf57..6ddb85553 100644 --- a/tests/config-loadpoint.spec.js +++ b/tests/config-loadpoint.spec.js @@ -29,6 +29,18 @@ async function addDemoCharger(page) { await expect(modal).not.toBeVisible(); } +async function addDemoMeter(page, power = "0") { + const lpModal = page.getByTestId("loadpoint-modal"); + await lpModal.getByRole("button", { name: "Add dedicated charger meter" }).click(); + + const modal = page.getByTestId("meter-modal"); + await expect(modal).toBeVisible(); + await modal.getByLabel("Manufacturer").selectOption("Demo meter"); + await modal.getByLabel("Power").fill(power); + await modal.getByRole("button", { name: "Save" }).click(); + await expect(modal).not.toBeVisible(); +} + async function addVehicle(page, title) { await page.getByRole("button", { name: "Add vehicle" }).click(); const modal = page.getByTestId("vehicle-modal"); @@ -282,4 +294,102 @@ test.describe("loadpoint", async () => { await page.goto("/"); await expect(page.getByRole("button", { name: "Fast" })).toHaveClass(/active/); }); + + test("delete vehicle references", async ({ page }) => { + await start(CONFIG_EMPTY); + await page.goto("/#/config"); + await enableExperimental(page); + + // add vehicle, add loadpoint, add charger + await addVehicle(page, "Porsche"); + await addVehicle(page, "Tesla"); + await newLoadpoint(page, "Garage"); + await addDemoCharger(page); + const lpModal = page.getByTestId("loadpoint-modal"); + await lpModal.getByLabel("Default vehicle").selectOption("Porsche"); + await lpModal.getByRole("button", { name: "Save" }).click(); + await expect(lpModal).not.toBeVisible(); + + // delete vehicle + await page.getByTestId("vehicle").nth(0).getByRole("button", { name: "edit" }).click(); + const vehicleModal = page.getByTestId("vehicle-modal"); + await vehicleModal.getByRole("button", { name: "Delete" }).click(); + await expect(vehicleModal).not.toBeVisible(); + + // restart + await restart(CONFIG_EMPTY); + await page.reload(); + + // check loadpoint default vehicle + await page.getByTestId("loadpoint").getByRole("button", { name: "edit" }).click(); + await expect(lpModal).toBeVisible(); + await expect(lpModal.getByLabel("Default vehicle")).toHaveValue(""); + }); + + test("delete charger references", async ({ page }) => { + await start(CONFIG_EMPTY); + await page.goto("/#/config"); + await enableExperimental(page); + + // add loadpoint, add charger + await newLoadpoint(page, "Garage"); + await addDemoCharger(page); + const lpModal = page.getByTestId("loadpoint-modal"); + await lpModal.getByRole("button", { name: "Save" }).click(); + await expect(lpModal).not.toBeVisible(); + + // delete charger + await page.getByTestId("loadpoint").getByRole("button", { name: "edit" }).click(); + await expect(lpModal).toBeVisible(); + await lpModal.getByRole("textbox", { name: "Charger" }).click(); + const chargerModal = page.getByTestId("charger-modal"); + await chargerModal.getByRole("button", { name: "Delete" }).click(); + await expect(chargerModal).not.toBeVisible(); + + // restart without saving loadpoint + await restart(CONFIG_EMPTY); + await page.reload(); + + // check loadpoint default vehicle + await page.getByTestId("loadpoint").getByRole("button", { name: "edit" }).click(); + await expect(lpModal).toBeVisible(); + await expect(lpModal.getByRole("textbox", { name: "Title" })).toHaveValue("Garage"); + await expect(lpModal).toContainText("Configuring a charger is required."); + }); + + test("delete meter references", async ({ page }) => { + await start(CONFIG_EMPTY); + await page.goto("/#/config"); + await enableExperimental(page); + + // add loadpoint, add charger + await newLoadpoint(page, "Garage"); + await addDemoCharger(page); + await addDemoMeter(page, "11000"); + const lpModal = page.getByTestId("loadpoint-modal"); + await lpModal.getByRole("button", { name: "Save" }).click(); + await expect(lpModal).not.toBeVisible(); + await expect(page.getByTestId("loadpoint")).toContainText("11.0 kW"); + + // delete charger + await page.getByTestId("loadpoint").getByRole("button", { name: "edit" }).click(); + await expect(lpModal).toBeVisible(); + await lpModal.getByRole("textbox", { name: "Meter" }).click(); + const meterModal = page.getByTestId("meter-modal"); + await meterModal.getByRole("button", { name: "Delete" }).click(); + await expect(meterModal).not.toBeVisible(); + + // restart without saving loadpoint + await restart(CONFIG_EMPTY); + await page.reload(); + + // check loadpoint default vehicle + await expect(page.getByTestId("loadpoint")).not.toContainText("11.0 kW"); + await page.getByTestId("loadpoint").getByRole("button", { name: "edit" }).click(); + await expect(lpModal).toBeVisible(); + await expect(lpModal.getByRole("textbox", { name: "Title" })).toHaveValue("Garage"); + await expect( + lpModal.getByRole("button", { name: "Add dedicated charger meter" }) + ).toBeVisible(); + }); });