From 14df2855bdac5dc307e8b6b4a4982030c4db4ed6 Mon Sep 17 00:00:00 2001 From: andig Date: Sun, 4 Jun 2023 11:28:06 +0200 Subject: [PATCH] chore: separate vehicle mutex (#8299) --- cmd/root.go | 2 +- core/loadpoint.go | 28 ++++++---------------------- core/loadpoint_api.go | 13 +++++-------- core/loadpoint_phases.go | 4 ++-- core/loadpoint_session.go | 4 ++-- core/loadpoint_vehicle.go | 34 +++++++++++++++++++--------------- 6 files changed, 35 insertions(+), 50 deletions(-) diff --git a/cmd/root.go b/cmd/root.go index 132933aff..03f76d54f 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -156,7 +156,7 @@ func runRoot(cmd *cobra.Command, args []string) { go socketHub.Run(pipe.NewDropper(ignoreEmpty).Pipe(tee.Attach()), cache) // setup values channel - valueChan := make(chan util.Param, 1) + valueChan := make(chan util.Param) go tee.Run(valueChan) // capture log messages for UI diff --git a/core/loadpoint.go b/core/loadpoint.go index 5d659a9b7..9b1789981 100644 --- a/core/loadpoint.go +++ b/core/loadpoint.go @@ -104,6 +104,7 @@ type Loadpoint struct { // exposed public configuration sync.Mutex // guard status + vehicleMux sync.Mutex // guard vehicle Mode api.ChargeMode `mapstructure:"mode"` // Charge mode, guarded by mutex Title_ string `mapstructure:"title"` // UI title @@ -680,15 +681,6 @@ func (lp *Loadpoint) setLimit(chargeCurrent float64, force bool) error { } lp.elapseGuard() - // remote stop - // TODO https://github.com/evcc-io/evcc/discussions/1929 - // if car, ok := lp.vehicle.(api.VehicleChargeController); !enabled && ok { - // // log but don't propagate - // if err := car.StopCharge(); err != nil { - // lp.log.ERROR.Printf("vehicle remote charge stop: %v", err) - // } - // } - if err := lp.charger.Enable(enabled); err != nil { return fmt.Errorf("charger %s: %w", status[enabled], err) } @@ -705,15 +697,6 @@ func (lp *Loadpoint) setLimit(chargeCurrent float64, force bool) error { } else { lp.stopWakeUpTimer() } - - // remote start - // TODO https://github.com/evcc-io/evcc/discussions/1929 - // if car, ok := lp.vehicle.(api.VehicleChargeController); enabled && ok { - // // log but don't propagate - // if err := car.StartCharge(); err != nil { - // lp.log.ERROR.Printf("vehicle remote charge start: %v", err) - // } - // } } return nil @@ -761,7 +744,8 @@ func (lp *Loadpoint) targetSocReached() bool { // minSocNotReached checks if minimum is configured and not reached. // If vehicle is not configured this will always return false func (lp *Loadpoint) minSocNotReached() bool { - if lp.vehicle == nil || lp.Soc.min == 0 { + vehicle := lp.GetVehicle() + if vehicle == nil || lp.Soc.min == 0 { return false } @@ -769,7 +753,7 @@ func (lp *Loadpoint) minSocNotReached() bool { return lp.vehicleSoc < float64(lp.Soc.min) } - minEnergy := lp.vehicle.Capacity() * float64(lp.Soc.min) / 100 / soc.ChargeEfficiency + minEnergy := vehicle.Capacity() * float64(lp.Soc.min) / 100 / soc.ChargeEfficiency return minEnergy > 0 && lp.getChargedEnergy() < minEnergy } @@ -1340,7 +1324,7 @@ func (lp *Loadpoint) publishSocAndRange() { // vehicle target soc targetSoc := 100 - if vs, ok := lp.vehicle.(api.SocLimiter); ok { + if vs, ok := lp.GetVehicle().(api.SocLimiter); ok { if limit, err := vs.TargetSoc(); err == nil { targetSoc = int(math.Trunc(limit)) lp.log.DEBUG.Printf("vehicle soc limit: %.0f%%", limit) @@ -1365,7 +1349,7 @@ func (lp *Loadpoint) publishSocAndRange() { lp.SetRemainingEnergy(1e3 * lp.socEstimator.RemainingChargeEnergy(socLimit)) // range - if vs, ok := lp.vehicle.(api.VehicleRange); ok { + if vs, ok := lp.GetVehicle().(api.VehicleRange); ok { if rng, err := vs.Range(); err == nil { lp.log.DEBUG.Printf("vehicle range: %dkm", rng) lp.publish(vehicleRange, rng) diff --git a/core/loadpoint_api.go b/core/loadpoint_api.go index 95b5c870d..d313b5305 100644 --- a/core/loadpoint_api.go +++ b/core/loadpoint_api.go @@ -396,21 +396,18 @@ func (lp *Loadpoint) GetRemainingEnergy() float64 { // GetVehicle gets the active vehicle func (lp *Loadpoint) GetVehicle() api.Vehicle { - lp.Lock() - defer lp.Unlock() + lp.vehicleMux.Lock() + defer lp.vehicleMux.Unlock() return lp.vehicle } // SetVehicle sets the active vehicle func (lp *Loadpoint) SetVehicle(vehicle api.Vehicle) { - // TODO develop universal locking approach - // setActiveVehicle is protected by lock, hence no locking here - - // set desired vehicle + // set desired vehicle (protected by lock, no locking here) lp.setActiveVehicle(vehicle) - lp.Lock() - defer lp.Unlock() + lp.vehicleMux.Lock() + defer lp.vehicleMux.Unlock() // disable auto-detect lp.stopVehicleDetection() diff --git a/core/loadpoint_phases.go b/core/loadpoint_phases.go index b05f52048..40e79a9b4 100644 --- a/core/loadpoint_phases.go +++ b/core/loadpoint_phases.go @@ -112,8 +112,8 @@ func (lp *Loadpoint) maxActivePhases() int { } func (lp *Loadpoint) getVehiclePhases() int { - if lp.vehicle != nil { - return lp.vehicle.Phases() + if vehicle := lp.GetVehicle(); vehicle != nil { + return vehicle.Phases() } return 0 diff --git a/core/loadpoint_session.go b/core/loadpoint_session.go index ee423f738..df0021c4e 100644 --- a/core/loadpoint_session.go +++ b/core/loadpoint_session.go @@ -32,8 +32,8 @@ func (lp *Loadpoint) createSession() { lp.session = lp.db.Session(lp.chargeMeterTotal()) - if lp.vehicle != nil { - lp.session.Vehicle = lp.vehicle.Title() + if vehicle := lp.GetVehicle(); vehicle != nil { + lp.session.Vehicle = vehicle.Title() } if c, ok := lp.charger.(api.Identifier); ok { diff --git a/core/loadpoint_vehicle.go b/core/loadpoint_vehicle.go index bf75fac6d..cdee2e752 100644 --- a/core/loadpoint_vehicle.go +++ b/core/loadpoint_vehicle.go @@ -99,10 +99,9 @@ func (lp *Loadpoint) selectVehicleByID(id string) api.Vehicle { // setActiveVehicle assigns currently active vehicle, configures soc estimator // and adds an odometer task func (lp *Loadpoint) setActiveVehicle(vehicle api.Vehicle) { - lp.Lock() - + lp.vehicleMux.Lock() if lp.vehicle == vehicle { - lp.Unlock() + lp.vehicleMux.Unlock() return } @@ -116,9 +115,14 @@ func (lp *Loadpoint) setActiveVehicle(vehicle api.Vehicle) { lp.coordinator.Acquire(vehicle) to = vehicle.Title() } - lp.log.INFO.Printf("vehicle updated: %s -> %s", from, to) lp.vehicle = vehicle + lp.vehicleMux.Unlock() + + lp.log.INFO.Printf("vehicle updated: %s -> %s", from, to) + + // lock api + lp.Lock() // reset minSoc and targetSoc before change lp.setMinSoc(0) @@ -127,7 +131,7 @@ func (lp *Loadpoint) setActiveVehicle(vehicle api.Vehicle) { // reset target energy lp.setTargetEnergy(0) - // unblock api + // unlock api lp.Unlock() if vehicle != nil { @@ -141,9 +145,9 @@ func (lp *Loadpoint) setActiveVehicle(vehicle api.Vehicle) { lp.socEstimator = soc.NewEstimator(lp.log, lp.charger, vehicle, estimate) lp.publish(vehiclePresent, true) - lp.publish(vehicleTitle, lp.vehicle.Title()) - lp.publish(vehicleIcon, lp.vehicle.Icon()) - lp.publish(vehicleCapacity, lp.vehicle.Capacity()) + lp.publish(vehicleTitle, vehicle.Title()) + lp.publish(vehicleIcon, vehicle.Icon()) + lp.publish(vehicleCapacity, vehicle.Capacity()) lp.applyAction(vehicle.OnIdentified()) lp.addTask(lp.vehicleOdometer) @@ -165,8 +169,8 @@ func (lp *Loadpoint) setActiveVehicle(vehicle api.Vehicle) { lp.updateSession(func(session *db.Session) { var title string - if lp.vehicle != nil { - title = lp.vehicle.Title() + if vehicle != nil { + title = vehicle.Title() } lp.session.Vehicle = title @@ -182,7 +186,7 @@ func (lp *Loadpoint) wakeUpVehicle() { } // vehicle - if vs, ok := lp.vehicle.(api.Resurrector); ok { + if vs, ok := lp.GetVehicle().(api.Resurrector); ok { if err := vs.WakeUp(); err != nil { lp.log.ERROR.Printf("wake-up vehicle: %v", err) } @@ -205,7 +209,7 @@ func (lp *Loadpoint) unpublishVehicle() { // vehicleHasFeature checks availability of vehicle feature func (lp *Loadpoint) vehicleHasFeature(f api.Feature) bool { - v, ok := lp.vehicle.(api.FeatureDescriber) + v, ok := lp.GetVehicle().(api.FeatureDescriber) if ok { ok = slices.Contains(v.Features(), f) } @@ -291,14 +295,14 @@ func (lp *Loadpoint) identifyVehicleByStatus() { } // remove previous vehicle if status was not confirmed - if _, ok := lp.vehicle.(api.ChargeState); ok { + if _, ok := lp.GetVehicle().(api.ChargeState); ok { lp.setActiveVehicle(nil) } } // vehicleOdometer updates odometer func (lp *Loadpoint) vehicleOdometer() { - if vs, ok := lp.vehicle.(api.VehicleOdometer); ok { + if vs, ok := lp.GetVehicle().(api.VehicleOdometer); ok { if odo, err := vs.Odometer(); err == nil { lp.log.DEBUG.Printf("vehicle odometer: %.0fkm", odo) lp.publish(vehicleOdometer, odo) @@ -355,7 +359,7 @@ func (lp *Loadpoint) vehicleSocPollAllowed() bool { // vehicleClimateActive checks if vehicle has active climate request func (lp *Loadpoint) vehicleClimateActive() bool { - if cl, ok := lp.vehicle.(api.VehicleClimater); ok && lp.vehicleClimatePollAllowed() { + if cl, ok := lp.GetVehicle().(api.VehicleClimater); ok && lp.vehicleClimatePollAllowed() { active, err := cl.Climater() if err == nil { if active {