Fix vehicle detection not started when default vehicle assigned on connect (#3957)

core/loadpoint.go

@@ -4,6 +4,7 @@ import (
 	"errors"
 	"fmt"
 	"math"
+	"reflect"
 	"regexp"
 	"strings"
 	"sync"
@@ -114,15 +115,15 @@ type LoadPoint struct {
 	MaxCurrent    float64       // Max allowed current. Physically ensured by the charger
 	GuardDuration time.Duration // charger enable/disable minimum holding time
 
-	enabled                bool      // Charger enabled state
+	enabled             bool      // Charger enabled state
-	phases                 int       // Charger active phases, guarded by mutex
+	phases              int       // Charger active phases, guarded by mutex
-	measuredPhases         int       // Charger physically measured phases
+	measuredPhases      int       // Charger physically measured phases
-	chargeCurrent          float64   // Charger current limit
+	chargeCurrent       float64   // Charger current limit
-	guardUpdated           time.Time // Charger enabled/disabled timestamp
+	guardUpdated        time.Time // Charger enabled/disabled timestamp
-	socUpdated             time.Time // SoC updated timestamp (poll: connected)
+	socUpdated          time.Time // SoC updated timestamp (poll: connected)
-	vehicleConnected       time.Time // Vehicle connected timestamp
+	vehicleDetect       time.Time // Vehicle connected timestamp
-	vehicleConnectedTicker *clock.Ticker
+	vehicleDetectTicker *clock.Ticker
-	vehicleID              string
+	vehicleID           string
 
 	charger     api.Charger
 	chargeTimer api.ChargeTimer
@@ -414,21 +415,11 @@ func (lp *LoadPoint) evVehicleConnectHandler() {
 		lp.socEstimator.Reset()
 	}
 
-	// flush all vehicles before updating state
+	// set default or start detection
-	lp.log.DEBUG.Println("vehicle api refresh")
+	lp.vehicleDefaultOrDetect()
-	provider.ResetCached()
-
-	// start detection if we have associated vehicles
-	if lp.vehicle == lp.defaultVehicle {
-		lp.setActiveVehicle(lp.defaultVehicle)
-	} else if len(lp.vehicles) > 0 {
-		lp.startVehicleDetection()
-	}
 
 	// immediately allow pv mode activity
 	lp.elapsePVTimer()
-
-	lp.pushEvent(evVehicleConnect)
 }
 
 // evVehicleDisconnectHandler sends external start event
@@ -550,9 +541,6 @@ func (lp *LoadPoint) Prepare(uiChan chan<- util.Param, pushChan chan<- push.Even
 	// activate default vehicle (allows poll mode: always)
 	if lp.defaultVehicle != nil {
 		lp.setActiveVehicle(lp.defaultVehicle)
-	} else if len(lp.vehicles) > 0 {
-		// start detection if we have associated vehicles
-		lp.startVehicleDetection()
 	}
 
 	// read initial charger state to prevent immediately disabling charger
@@ -793,7 +781,8 @@ func (lp *LoadPoint) selectVehicleByID(id string) api.Vehicle {
 	return nil
 }
 
-// setActiveVehicle assigns currently active vehicle and configures soc estimator
+// setActiveVehicle assigns currently active vehicle, configures soc estimator
+// and adds an odometer task
 func (lp *LoadPoint) setActiveVehicle(vehicle api.Vehicle) {
 	lp.Lock()
 	defer lp.Unlock()
@@ -823,14 +812,12 @@ func (lp *LoadPoint) setActiveVehicle(vehicle api.Vehicle) {
 		lp.publish("vehicleTitle", lp.vehicle.Title())
 		lp.publish("vehicleCapacity", lp.vehicle.Capacity())
 
-		// release lock to unblock api
+		// unblock api
 		lp.Unlock()
+		lp.applyAction(vehicle.OnIdentified())
+		lp.Lock()
 
 		lp.addTask(lp.vehicleOdometer)
-		lp.applyAction(vehicle.OnIdentified())
-
-		// re-apply lock to match defer above
-		lp.Lock()
 
 		lp.progress.Reset()
 	} else {
@@ -874,29 +861,15 @@ func (lp *LoadPoint) unpublishVehicle() {
 	lp.setRemainingDuration(-1)
 }
 
-// startVehicleDetection resets connection timer and starts api refresh timer
-func (lp *LoadPoint) startVehicleDetection() {
-	lp.vehicleConnected = lp.clock.Now()
-	lp.vehicleConnectedTicker = lp.clock.Ticker(vehicleDetectInterval)
-}
-
-// stopVehicleDetection expires the connection timer and ticker
-func (lp *LoadPoint) stopVehicleDetection() {
-	lp.vehicleConnected = time.Time{}
-	if lp.vehicleConnectedTicker != nil {
-		lp.vehicleConnectedTicker.Stop()
-	}
-}
-
 // vehicleUnidentified checks if there are associated vehicles and starts discovery period
 func (lp *LoadPoint) vehicleUnidentified() bool {
 	res := len(lp.vehicles) > 0 && lp.vehicle == nil &&
-		lp.clock.Since(lp.vehicleConnected) < vehicleDetectDuration
+		lp.clock.Since(lp.vehicleDetect) < vehicleDetectDuration
 
 	// request vehicle api refresh while waiting to identify
 	if res {
 		select {
-		case <-lp.vehicleConnectedTicker.C:
+		case <-lp.vehicleDetectTicker.C:
 			lp.log.DEBUG.Println("vehicle api refresh")
 			provider.ResetCached()
 		default:
@@ -906,6 +879,35 @@ func (lp *LoadPoint) vehicleUnidentified() bool {
 	return res
 }
 
+// vehicleDefaultOrDetect will assign and update default vehicle or start detection
+func (lp *LoadPoint) vehicleDefaultOrDetect() {
+	if lp.defaultVehicle != nil {
+		if lp.vehicle != lp.defaultVehicle {
+			lp.setActiveVehicle(lp.defaultVehicle)
+		} else {
+			// default vehicle is already active, update odometer anyway
+			// need to do this here since setActiveVehicle would short-circuit
+			lp.addTask(lp.vehicleOdometer)
+		}
+	} else if len(lp.vehicles) > 0 {
+		// flush all vehicles before detection starts
+		lp.log.DEBUG.Println("vehicle api refresh")
+		provider.ResetCached()
+
+		// reset connection timer and starts api refresh timer
+		lp.vehicleDetect = lp.clock.Now()
+		lp.vehicleDetectTicker = lp.clock.Ticker(vehicleDetectInterval)
+	}
+}
+
+// stopVehicleDetection expires the connection timer and ticker
+func (lp *LoadPoint) stopVehicleDetection() {
+	lp.vehicleDetect = time.Time{}
+	if lp.vehicleDetectTicker != nil {
+		lp.vehicleDetectTicker.Stop()
+	}
+}
+
 // identifyVehicleByStatus validates if the active vehicle is still connected to the loadpoint
 func (lp *LoadPoint) identifyVehicleByStatus() {
 	if len(lp.vehicles) == 0 {
@@ -947,14 +949,18 @@ func (lp *LoadPoint) updateChargerStatus() error {
 	if prevStatus := lp.GetStatus(); status != prevStatus {
 		lp.setStatus(status)
 
-		// changed from empty (initial startup) - set connected without sending message
+		// changed to A - disconnected - don't send on startup
-		if prevStatus == api.StatusNone {
+		if status == api.StatusA && prevStatus != api.StatusNone {
-			lp.connectedTime = lp.clock.Now()
+			lp.bus.Publish(evVehicleDisconnect)
-			lp.publish("connectedDuration", time.Duration(0))
 		}
 
-		// changed from A - connected
+		// changed to B - connected - don't send on startup
-		if prevStatus == api.StatusA {
+		if status == api.StatusB && prevStatus != api.StatusC {
+			if prevStatus != api.StatusNone {
+				// send connected message if not startup
+				lp.pushEvent(evVehicleConnect)
+			}
+
 			lp.bus.Publish(evVehicleConnect)
 		}
 
@@ -965,11 +971,6 @@ func (lp *LoadPoint) updateChargerStatus() error {
 			lp.bus.Publish(evChargeStop)
 		}
 
-		// changed to A - disconnected - don't send on startup
-		if status == api.StatusA && prevStatus != api.StatusNone {
-			lp.bus.Publish(evVehicleDisconnect)
-		}
-
 		// update whenever there is a state change
 		lp.bus.Publish(evChargeCurrent, lp.chargeCurrent)
 	}
@@ -1451,11 +1452,20 @@ func (lp *LoadPoint) publishSoCAndRange() {
 
 // addTask adds a single task to the queue
 func (lp *LoadPoint) addTask(task func()) {
-	lp.tasks.Enqueue(task)
+	// test guard
+	if lp.tasks != nil {
+		// don't add twice
+		if t, ok := lp.tasks.Peek(); ok &&
+			reflect.ValueOf(t).Pointer() == reflect.ValueOf(task).Pointer() {
+			return
+		}
+		lp.tasks.Enqueue(task)
+	}
 }
 
 // processTasks executes a single task from the queue
 func (lp *LoadPoint) processTasks() {
+	// test guard
 	if lp.tasks != nil {
 		if task, ok := lp.tasks.Dequeue(); ok {
 			task.(func())()

core/loadpoint_test.go

@@ -881,6 +881,16 @@ func TestDefaultVehicle(t *testing.T) {
 		t.Errorf("expected %v, got %v", title(dflt), title(lp.vehicle))
 	}
 
+	// set non-default vehicle during disconnect - should be default on connect
+	lp.tasks.Clear()
+	lp.evVehicleConnectHandler()
+	if lp.vehicle != dflt {
+		t.Errorf("expected %v, got %v", title(dflt), title(lp.vehicle))
+	}
+	if l := lp.tasks.Size(); l != 1 {
+		t.Error("expected task in queue, got none")
+	}
+
 	// guest connected
 	lp.setActiveVehicle(nil)
 	if lp.vehicle != nil {
@@ -945,3 +955,75 @@ func TestApplyVehicleDefaults(t *testing.T) {
 	lp.evVehicleDisconnectHandler()
 	assertConfig(lp, od)
 }
+
+func TestReconnectVehicle(t *testing.T) {
+	ctrl := gomock.NewController(t)
+	clck := clock.NewMock()
+
+	type vehicleT struct {
+		*mock.MockVehicle
+		*mock.MockChargeState
+	}
+
+	vehicle := &vehicleT{mock.NewMockVehicle(ctrl), mock.NewMockChargeState(ctrl)}
+	vehicle.MockVehicle.EXPECT().Title().Return("vehicle").AnyTimes()
+	vehicle.MockVehicle.EXPECT().Capacity().AnyTimes()
+	vehicle.MockVehicle.EXPECT().Phases().AnyTimes()
+	vehicle.MockVehicle.EXPECT().OnIdentified().AnyTimes()
+	vehicle.MockVehicle.EXPECT().SoC().Return(0.0, nil).AnyTimes()
+
+	charger := mock.NewMockCharger(ctrl)
+	charger.EXPECT().Status().Return(api.StatusB, nil).AnyTimes()
+
+	lp := &LoadPoint{
+		log:         util.NewLogger("foo"),
+		bus:         evbus.New(),
+		clock:       clck,
+		charger:     charger,
+		chargeMeter: &Null{}, // silence nil panics
+		chargeRater: &Null{}, // silence nil panics
+		chargeTimer: &Null{}, // silence nil panics
+		wakeUpTimer: NewTimer(),
+		MinCurrent:  minA,
+		MaxCurrent:  maxA,
+		phases:      1,
+		Mode:        api.ModeNow,
+		vehicles:    []api.Vehicle{vehicle},
+	}
+
+	attachListeners(t, lp)
+
+	// mode now
+	charger.EXPECT().MaxCurrent(int64(maxA))
+	// sync charger
+	charger.EXPECT().Enabled().Return(true, nil)
+
+	// vehicle not updated yet
+	vehicle.MockChargeState.EXPECT().Status().Return(api.StatusA, nil)
+
+	lp.Update(0, false, false)
+	ctrl.Finish()
+
+	// detection started
+	if lp.vehicleDetect != lp.clock.Now() {
+		t.Error("vehicle detection not started")
+	}
+
+	// vehicle not detected yet
+	if lp.vehicle != nil {
+		t.Error("vehicle should be <nil>")
+	}
+
+	// sync charger
+	charger.EXPECT().Enabled().Return(true, nil)
+	// vehicle not updated yet
+	vehicle.MockChargeState.EXPECT().Status().Return(api.StatusB, nil)
+
+	lp.Update(0, false, false)
+	ctrl.Finish()
+
+	// vehicle detected
+	if lp.vehicle != vehicle {
+		t.Error("vehicle should be detected")
+	}
+}

core/progress.go

@@ -13,7 +13,8 @@ func NewProgress(min, step float64) *Progress {
 }
 
 func (p *Progress) NextStep(value float64) bool {
-	if value >= p.current {
+	// test guard
+	if p != nil && value >= p.current {
 		for p.current <= value {
 			p.current += p.step
 		}
@@ -25,5 +26,8 @@ func (p *Progress) NextStep(value float64) bool {
 }
 
 func (p *Progress) Reset() {
-	p.current = p.min
+	// test guard
+	if p != nil {
+		p.current = p.min
+	}
 }