Ocpp: fix crash when unconfigured chargepoint connects (evcc-io#10039)

charger/ocpp/const.go

@@ -0,0 +1,20 @@
+package ocpp
+
+const (
+	// Core profile keys
+	KeyNumberOfConnectors = "NumberOfConnectors"
+
+	// Meter profile keys
+	KeyMeterValuesSampledData   = "MeterValuesSampledData"
+	KeyMeterValueSampleInterval = "MeterValueSampleInterval"
+
+	// Smart Charging profile keys
+	KeyChargeProfileMaxStackLevel              = "ChargeProfileMaxStackLevel"
+	KeyChargingScheduleAllowedChargingRateUnit = "ChargingScheduleAllowedChargingRateUnit"
+	KeyChargingScheduleMaxPeriods              = "ChargingScheduleMaxPeriods"
+	KeyConnectorSwitch3to1PhaseSupported       = "ConnectorSwitch3to1PhaseSupported"
+	KeyMaxChargingProfilesInstalled            = "MaxChargingProfilesInstalled"
+
+	// Alfen specific keys
+	KeyAlfenPlugAndChargeIdentifier = "PlugAndChargeIdentifier"
+)

charger/ocpp/cp.go

@@ -15,25 +15,6 @@ import (
 	"github.com/lorenzodonini/ocpp-go/ocpp1.6/types"
 )
 
-const (
-	// Core profile keys
-	KeyNumberOfConnectors = "NumberOfConnectors"
-
-	// Meter profile keys
-	KeyMeterValuesSampledData   = "MeterValuesSampledData"
-	KeyMeterValueSampleInterval = "MeterValueSampleInterval"
-
-	// Smart Charging profile keys
-	KeyChargeProfileMaxStackLevel              = "ChargeProfileMaxStackLevel"
-	KeyChargingScheduleAllowedChargingRateUnit = "ChargingScheduleAllowedChargingRateUnit"
-	KeyChargingScheduleMaxPeriods              = "ChargingScheduleMaxPeriods"
-	KeyConnectorSwitch3to1PhaseSupported       = "ConnectorSwitch3to1PhaseSupported"
-	KeyMaxChargingProfilesInstalled            = "MaxChargingProfilesInstalled"
-
-	// Alfen specific keys
-	KeyAlfenPlugAndChargeIdentifier = "PlugAndChargeIdentifier"
-)
-
 // TODO support multiple connectors
 // Since ocpp-go interfaces at charge point level, we need to manage multiple connector separately
 

charger/ocpp/cs.go

@@ -16,17 +16,17 @@ type CS struct {
 	cps map[string]*CP
 }
 
-// Register registers a chargepoint with the central system.
-// The chargepoint identified by id may already be connected in which case initial connection is triggered.
+// Register registers a charge point with the central system.
+// The charge point identified by id may already be connected in which case initial connection is triggered.
 func (cs *CS) Register(id string, cp *CP) error {
 	cs.mu.Lock()
 	defer cs.mu.Unlock()
 
 	if _, ok := cs.cps[id]; ok && id == "" {
-		return errors.New("cannot have >1 chargepoint with empty station id")
+		return errors.New("cannot have >1 charge point with empty station id")
 	}
 
-	// trigger unknown chargepoint connected
+	// trigger unknown charge point connected
 	if unknown, ok := cs.cps[id]; ok && unknown == nil {
 		cp.connect(true)
 	}
@@ -43,63 +43,67 @@ func (cs *CS) errorHandler(errC <-chan error) {
 	}
 }
 
+// chargepointByID returns a configured charge point identified by id.
 func (cs *CS) chargepointByID(id string) (*CP, error) {
 	cp, ok := cs.cps[id]
 	if !ok {
 		return nil, fmt.Errorf("unknown charge point: %s", id)
 	}
+	if cp == nil {
+		return nil, fmt.Errorf("charge point not configured: %s", id)
+	}
 	return cp, nil
 }
 
+// NewChargePoint implements ocpp16.ChargePointConnectionHandler
 func (cs *CS) NewChargePoint(chargePoint ocpp16.ChargePointConnection) {
 	cs.mu.Lock()
 	defer cs.mu.Unlock()
 
-	if cp, err := cs.chargepointByID(chargePoint.ID()); err != nil {
-		// check for anonymous chargepoint
-		if cp, err := cs.chargepointByID(""); err == nil {
-			cs.log.INFO.Printf("chargepoint connected, registering: %s", chargePoint.ID())
+	// check for configured charge point
+	cp, ok := cs.cps[chargePoint.ID()]
+	if ok {
+		cs.log.DEBUG.Printf("charge point connected: %s", chargePoint.ID())
 
-			// update id
-			cp.RegisterID(chargePoint.ID())
+		// trigger initial connection if charge point is already setup
+		if cp != nil {
+			cp.connect(true)
+		}
 
-			cs.cps[chargePoint.ID()] = cp
-			delete(cs.cps, "")
+		return
+	}
 
-			cp.connect(true)
+	// check for configured anonymous charge point
+	cp, ok = cs.cps[""]
+	if ok && cp != nil {
+		cs.log.INFO.Printf("charge point connected, registering: %s", chargePoint.ID())
 
-			return
-		}
+		// update id
+		cp.RegisterID(chargePoint.ID())
 
-		cs.log.WARN.Printf("chargepoint connected, unknown: %s", chargePoint.ID())
+		cs.cps[chargePoint.ID()] = cp
+		delete(cs.cps, "")
 
-		// register unknown chargepoint
-		// when chargepoint setup is complete, it will eventually be associated with the connected id
-		cs.cps[chargePoint.ID()] = nil
-	} else {
-		cs.log.DEBUG.Printf("chargepoint connected: %s", chargePoint.ID())
+		cp.connect(true)
 
-		// trigger initial connection if chargepoint is already setup
-		if cp != nil {
-			cp.connect(true)
-		}
+		return
 	}
+
+	cs.log.WARN.Printf("unknown charge point connected: %s", chargePoint.ID())
+
+	// register unknown charge point
+	// when charge point setup is complete, it will eventually be associated with the connected id
+	cs.cps[chargePoint.ID()] = nil
 }
 
+// ChargePointDisconnected implements ocpp16.ChargePointConnectionHandler
 func (cs *CS) ChargePointDisconnected(chargePoint ocpp16.ChargePointConnection) {
 	cs.mu.Lock()
 	defer cs.mu.Unlock()
 
+	cs.log.DEBUG.Printf("charge point disconnected: %s", chargePoint.ID())
+
 	if cp, err := cs.chargepointByID(chargePoint.ID()); err != nil {
-		cs.log.ERROR.Printf("chargepoint disconnected: %v", err)
-	} else {
-		cs.log.DEBUG.Printf("chargepoint disconnected: %s", chargePoint.ID())
-
-		if cp == nil {
-			// remove unknown chargepoint
-			delete(cs.cps, chargePoint.ID())
-		} else {
-			cp.connect(false)
-		}
+		cp.connect(false)
 	}
 }

charger/ocpp/cs_core.go

@@ -1,6 +1,8 @@
 package ocpp
 
 import (
+	"fmt"
+
 	"github.com/lorenzodonini/ocpp-go/ocpp1.6/core"
 	"github.com/lorenzodonini/ocpp-go/ocpp1.6/firmware"
 	"github.com/lorenzodonini/ocpp-go/ocpp1.6/remotetrigger"
@@ -73,6 +75,7 @@ func (cs *CS) OnBootNotification(id string, request *core.BootNotificationReques
 		return nil, err
 	}
 
+	fmt.Println("OnBootNotification id", id)
 	return cp.BootNotification(request)
 }
 

charger/ocpp_test.go

@@ -27,50 +27,52 @@ func TestOcpp(t *testing.T) {
 type ocppTestSuite struct {
 	suite.Suite
 	clock *clock.Mock
-	cp    ocpp16.ChargePoint
 }
 
 func (suite *ocppTestSuite) SetupSuite() {
-	// setup cs
-	suite.NotNil(ocpp.Instance())
-
-	// setup cp
 	suite.clock = clock.NewMock()
-	cp := ocpp16.NewChargePoint("test", nil, nil)
+	suite.NotNil(ocpp.Instance())
+}
 
+func (suite *ocppTestSuite) startChargePoint(id string) ocpp16.ChargePoint {
 	// set a handler for all callback functions
-	triggerC := make(chan remotetrigger.MessageTrigger, 1)
-	handler := &ChargePointHandler{triggerC: triggerC}
+	handler := &ChargePointHandler{
+		triggerC: make(chan remotetrigger.MessageTrigger, 1),
+	}
+
+	// create charge point with handler
+	cp := ocpp16.NewChargePoint(id, nil, nil)
 	cp.SetCoreHandler(handler)
 	cp.SetRemoteTriggerHandler(handler)
 
+	// let cs handle the trigger messages
 	go func() {
-		for msg := range triggerC {
-			suite.handleTrigger(msg)
+		for msg := range handler.triggerC {
+			suite.handleTrigger(cp, msg)
 		}
 	}()
 
-	suite.cp = cp
+	return cp
 }
 
-func (suite *ocppTestSuite) handleTrigger(msg remotetrigger.MessageTrigger) {
+func (suite *ocppTestSuite) handleTrigger(cp ocpp16.ChargePoint, msg remotetrigger.MessageTrigger) {
 	switch msg {
 	case core.BootNotificationFeatureName:
-		if res, err := suite.cp.BootNotification("demo", "evcc"); err != nil {
+		if res, err := cp.BootNotification("demo", "evcc"); err != nil {
 			suite.T().Log("BootNotification:", err)
 		} else {
 			suite.T().Log("BootNotification:", res)
 		}
 
 	case core.StatusNotificationFeatureName:
-		if res, err := suite.cp.StatusNotification(ocppTestConnector, core.NoError, core.ChargePointStatusAvailable); err != nil {
+		if res, err := cp.StatusNotification(ocppTestConnector, core.NoError, core.ChargePointStatusAvailable); err != nil {
 			suite.T().Log("StatusNotification:", err)
 		} else {
 			suite.T().Log("StatusNotification:", res)
 		}
 
 	case core.MeterValuesFeatureName:
-		if res, err := suite.cp.MeterValues(1, []types.MeterValue{
+		if res, err := cp.MeterValues(1, []types.MeterValue{
 			{
 				Timestamp: types.NewDateTime(suite.clock.Now()),
 				SampledValue: []types.SampledValue{
@@ -91,20 +93,21 @@ func (suite *ocppTestSuite) handleTrigger(msg remotetrigger.MessageTrigger) {
 
 func (suite *ocppTestSuite) TestConnect() {
 	// start cp client
-	suite.NoError(suite.cp.Start(ocppTestUrl))
-	suite.True(suite.cp.IsConnected())
+	cp := suite.startChargePoint("test")
+	suite.NoError(cp.Start(ocppTestUrl))
+	suite.True(cp.IsConnected())
 
 	// start cp server
 	c, err := NewOCPP("test", ocppTestConnector, "", "", 0, false, false, ocppTestConnectTimeout, ocppTestTimeout)
-	suite.NoError(err)
-
 	if err != nil {
+		suite.NoError(err)
 		return
 	}
 
 	suite.clock.Add(ocppTestTimeout)
 	c.cp.TestClock(suite.clock)
 
+	// status
 	_, err = c.Status()
 	suite.NoError(err)
 
@@ -117,4 +120,13 @@ func (suite *ocppTestSuite) TestConnect() {
 	f, err = c.totalEnergy()
 	suite.NoError(err)
 	suite.Equal(1.2, f)
+
+	// 2nd charge point
+	cp2 := suite.startChargePoint("test2")
+	suite.NoError(cp2.Start(ocppTestUrl))
+	suite.True(cp2.IsConnected())
+
+	// error on unconfigured 2nd charge point
+	_, err = cp2.BootNotification("demo", "evcc")
+	suite.Error(err)
 }