Ocpp: reduce number of configurable timeouts

charger/ocpp.go

@@ -27,7 +27,6 @@ type OCPP struct {
 	enabled bool
 	current float64
 
-	timeout        time.Duration
 	remoteStart    bool
 	stackLevelZero bool
 	lp             loadpoint.API
@@ -48,8 +47,8 @@ func NewOCPPFromConfig(other map[string]interface{}) (api.Charger, error) {
 		MeterInterval  time.Duration
 		MeterValues    string
 		ConnectTimeout time.Duration // Initial Timeout
-		Timeout        time.Duration // Message Timeout
 
+		Timeout          time.Duration              // TODO deprecated
 		BootNotification *bool                      // TODO deprecated
 		GetConfiguration *bool                      // TODO deprecated
 		ChargingRateUnit types.ChargingRateUnitType // TODO deprecated
@@ -62,8 +61,7 @@ func NewOCPPFromConfig(other map[string]interface{}) (api.Charger, error) {
 		Connector:      1,
 		IdTag:          defaultIdTag,
 		MeterInterval:  10 * time.Second,
-		ConnectTimeout: ocppConnectTimeout,
-		Timeout:        ocppTimeout,
+		ConnectTimeout: 5 * time.Minute,
 	}
 
 	if err := util.DecodeOther(other, &cc); err != nil {
@@ -75,7 +73,7 @@ func NewOCPPFromConfig(other map[string]interface{}) (api.Charger, error) {
 	c, err := NewOCPP(cc.StationId, cc.Connector, cc.IdTag,
 		cc.MeterValues, cc.MeterInterval,
 		stackLevelZero, cc.RemoteStart,
-		cc.ConnectTimeout, cc.Timeout)
+		cc.ConnectTimeout)
 	if err != nil {
 		return c, err
 	}
@@ -124,7 +122,7 @@ func NewOCPPFromConfig(other map[string]interface{}) (api.Charger, error) {
 func NewOCPP(id string, connector int, idtag string,
 	meterValues string, meterInterval time.Duration,
 	stackLevelZero, remoteStart bool,
-	connectTimeout, timeout time.Duration,
+	connectTimeout time.Duration,
 ) (*OCPP, error) {
 	unit := "ocpp"
 	if id != "" {
@@ -151,7 +149,7 @@ func NewOCPP(id string, connector int, idtag string,
 		case <-cp.HasConnected():
 		}
 
-		if err := cp.Setup(meterValues, meterInterval, timeout); err != nil {
+		if err := cp.Setup(meterValues, meterInterval); err != nil {
 			return nil, err
 		}
 	}
@@ -160,7 +158,7 @@ func NewOCPP(id string, connector int, idtag string,
 		return nil, fmt.Errorf("invalid connector: %d", connector)
 	}
 
-	conn, err := ocpp.NewConnector(log, connector, cp, timeout)
+	conn, err := ocpp.NewConnector(log, connector, cp)
 	if err != nil {
 		return nil, err
 	}
@@ -176,7 +174,6 @@ func NewOCPP(id string, connector int, idtag string,
 		idtag:          idtag,
 		remoteStart:    remoteStart,
 		stackLevelZero: stackLevelZero,
-		timeout:        timeout,
 	}
 
 	if cp.HasRemoteTriggerFeature {
@@ -202,11 +199,6 @@ func (c *OCPP) effectiveIdTag() string {
 	return c.idtag
 }
 
-// wait waits for a CP roundtrip with timeout
-func (c *OCPP) wait(err error, rc chan error) error {
-	return ocpp.Wait(err, rc, c.timeout)
-}
-
 // Status implements the api.Charger interface
 func (c *OCPP) Status() (api.ChargeStatus, error) {
 	status, err := c.conn.Status()
@@ -334,7 +326,7 @@ func (c *OCPP) initTransaction() error {
 		request.ConnectorId = &connector
 	})
 
-	return c.wait(err, rc)
+	return ocpp.Wait(err, rc)
 }
 
 func (c *OCPP) setChargingProfile(profile *types.ChargingProfile) error {
@@ -347,7 +339,7 @@ func (c *OCPP) setChargingProfile(profile *types.ChargingProfile) error {
 		rc <- err
 	}, c.conn.ID(), profile)
 
-	return c.wait(err, rc)
+	return ocpp.Wait(err, rc)
 }
 
 // setCurrent sets the TxDefaultChargingProfile with given current
@@ -384,7 +376,7 @@ func (c *OCPP) getScheduleLimit() (float64, error) {
 		rc <- err
 	}, c.conn.ID(), duration)
 
-	err = c.wait(err, rc)
+	err = ocpp.Wait(err, rc)
 
 	return limit, err
 }
@@ -493,7 +485,7 @@ func (c *OCPP) Diagnose() {
 
 		rc <- err
 	}, nil)
-	c.wait(err, rc)
+	ocpp.Wait(err, rc)
 }
 
 var _ loadpoint.Controller = (*OCPP)(nil)

charger/ocpp/connector.go

@@ -27,22 +27,20 @@ type Connector struct {
 
 	meterUpdated time.Time
 	measurements map[types.Measurand]types.SampledValue
-	timeout      time.Duration
 
 	txnCount int // change initial value to the last known global transaction. Needs persistence
 	txnId    int
 	idTag    string
 }
 
-func NewConnector(log *util.Logger, id int, cp *CP, timeout time.Duration) (*Connector, error) {
+func NewConnector(log *util.Logger, id int, cp *CP) (*Connector, error) {
 	conn := &Connector{
 		log:          log,
 		cp:           cp,
 		id:           id,
 		clock:        clock.New(),
 		statusC:      make(chan struct{}, 1),
 		measurements: make(map[types.Measurand]types.SampledValue),
-		timeout:      timeout,
 	}
 
 	err := cp.registerConnector(id, conn)
@@ -90,8 +88,8 @@ func (conn *Connector) WatchDog(timeout time.Duration) {
 
 // Initialized waits for initial charge point status notification
 func (conn *Connector) Initialized() error {
-	trigger := time.After(conn.timeout / 2)
-	timeout := time.After(conn.timeout)
+	trigger := time.After(Timeout / 2)
+	timeout := time.After(Timeout)
 	for {
 		select {
 		case <-conn.statusC:
@@ -153,7 +151,7 @@ func (conn *Connector) NeedsAuthentication() bool {
 // isMeterTimeout checks if meter values are outdated.
 // Must only be called while holding lock.
 func (conn *Connector) isMeterTimeout() bool {
-	return conn.timeout > 0 && conn.clock.Since(conn.meterUpdated) > conn.timeout
+	return conn.clock.Since(conn.meterUpdated) > Timeout
 }
 
 var _ api.CurrentGetter = (*Connector)(nil)

charger/ocpp/connector_core.go

@@ -11,7 +11,7 @@ import (
 // timestampValid returns false if status timestamps are outdated
 func (conn *Connector) timestampValid(t time.Time) bool {
 	// reject if expired
-	if conn.clock.Since(t) > messageExpiry {
+	if conn.clock.Since(t) > Timeout {
 		return false
 	}
 
@@ -84,7 +84,7 @@ func (conn *Connector) StartTransaction(request *core.StartTransactionRequest) (
 	defer conn.mu.Unlock()
 
 	// expired request
-	if request.Timestamp != nil && conn.clock.Since(request.Timestamp.Time) > transactionExpiry {
+	if request.Timestamp != nil && conn.clock.Since(request.Timestamp.Time) > Timeout {
 		res := &core.StartTransactionConfirmation{
 			IdTagInfo: &types.IdTagInfo{
 				Status: types.AuthorizationStatusExpired, // reject
@@ -133,7 +133,7 @@ func (conn *Connector) StopTransaction(request *core.StopTransactionRequest) (*c
 	defer conn.mu.Unlock()
 
 	// expired request
-	if request.Timestamp != nil && conn.clock.Since(request.Timestamp.Time) > transactionExpiry {
+	if request.Timestamp != nil && conn.clock.Since(request.Timestamp.Time) > Timeout {
 		res := &core.StopTransactionConfirmation{
 			IdTagInfo: &types.IdTagInfo{
 				Status: types.AuthorizationStatusExpired, // reject

charger/ocpp/connector_test.go

@@ -24,7 +24,7 @@ type connTestSuite struct {
 
 func (suite *connTestSuite) SetupTest() {
 	suite.cp = NewChargePoint(util.NewLogger("foo"), "abc")
-	suite.conn, _ = NewConnector(util.NewLogger("foo"), 1, suite.cp, time.Minute)
+	suite.conn, _ = NewConnector(util.NewLogger("foo"), 1, suite.cp)
 
 	suite.clock = clock.NewMock()
 	suite.conn.clock = suite.clock

charger/ocpp/const.go

@@ -1,6 +1,10 @@
 package ocpp
 
+import "time"
+
 const (
+	Timeout = 30 * time.Second // default request / response timeout on protocol level
+
 	// Core profile keys
 	KeyMeterValueSampleInterval        = "MeterValueSampleInterval"
 	KeyMeterValuesSampledData          = "MeterValuesSampledData"

charger/ocpp/cp_core.go

@@ -2,18 +2,12 @@ package ocpp
 
 import (
 	"errors"
-	"time"
 
 	"github.com/lorenzodonini/ocpp-go/ocpp1.6/core"
 	"github.com/lorenzodonini/ocpp-go/ocpp1.6/firmware"
 	"github.com/lorenzodonini/ocpp-go/ocpp1.6/types"
 )
 
-const (
-	messageExpiry     = 30 * time.Second
-	transactionExpiry = time.Hour
-)
-
 var (
 	ErrInvalidRequest     = errors.New("invalid request")
 	ErrInvalidConnector   = errors.New("invalid connector")

charger/ocpp/cp_setup.go

@@ -14,7 +14,7 @@ import (
 
 const desiredMeasurands = "Power.Active.Import,Energy.Active.Import.Register,Current.Import,Voltage,Current.Offered,Power.Offered,SoC"
 
-func (cp *CP) Setup(meterValues string, meterInterval time.Duration, timeout time.Duration) error {
+func (cp *CP) Setup(meterValues string, meterInterval time.Duration) error {
 	if err := Instance().ChangeAvailabilityRequest(cp.ID(), 0, core.AvailabilityTypeOperative); err != nil {
 		cp.log.DEBUG.Printf("failed configuring availability: %v", err)
 	}
@@ -98,7 +98,7 @@ func (cp *CP) Setup(meterValues string, meterInterval time.Duration, timeout tim
 		rc <- err
 	}, nil)
 
-	if err := Wait(err, rc, timeout); err != nil {
+	if err := Wait(err, rc); err != nil {
 		return err
 	}
 
@@ -108,9 +108,8 @@ func (cp *CP) Setup(meterValues string, meterInterval time.Duration, timeout tim
 			cp.log.DEBUG.Printf("failed triggering BootNotification: %v", err)
 		}
 
-		// TODO use fixed timeout
 		select {
-		case <-time.After(timeout):
+		case <-time.After(Timeout):
 			cp.log.DEBUG.Printf("BootNotification timeout")
 		case res := <-cp.bootNotificationRequestC:
 			cp.BootNotificationResult = res
@@ -119,13 +118,13 @@ func (cp *CP) Setup(meterValues string, meterInterval time.Duration, timeout tim
 
 	// autodetect measurands
 	if meterValues == "" && meterValuesSampledDataMaxLength > 0 {
-		sampledMeasurands := cp.tryMeasurands(desiredMeasurands, KeyMeterValuesSampledData, timeout)
+		sampledMeasurands := cp.tryMeasurands(desiredMeasurands, KeyMeterValuesSampledData)
 		meterValues = strings.Join(sampledMeasurands[:min(len(sampledMeasurands), meterValuesSampledDataMaxLength)], ",")
 	}
 
 	// configure measurands
 	if meterValues != "" {
-		if err := cp.configure(KeyMeterValuesSampledData, meterValues, timeout); err == nil {
+		if err := cp.configure(KeyMeterValuesSampledData, meterValues); err == nil {
 			meterValuesSampledData = meterValues
 		}
 	}
@@ -137,7 +136,7 @@ func (cp *CP) Setup(meterValues string, meterInterval time.Duration, timeout tim
 		if err := Instance().TriggerMessageRequest(cp.ID(), core.MeterValuesFeatureName); err == nil {
 			// wait for meter values
 			select {
-			case <-time.After(timeout):
+			case <-time.After(Timeout):
 				cp.log.WARN.Println("meter timeout")
 			case <-cp.meterC:
 			}
@@ -146,13 +145,13 @@ func (cp *CP) Setup(meterValues string, meterInterval time.Duration, timeout tim
 
 	// configure sample rate
 	if meterInterval > 0 {
-		if err := cp.configure(KeyMeterValueSampleInterval, strconv.Itoa(int(meterInterval.Seconds())), timeout); err != nil {
+		if err := cp.configure(KeyMeterValueSampleInterval, strconv.Itoa(int(meterInterval.Seconds()))); err != nil {
 			cp.log.WARN.Printf("failed configuring MeterValueSampleInterval: %v", err)
 		}
 	}
 
 	// configure websocket ping interval
-	if err := cp.configure(KeyWebSocketPingInterval, "30", timeout); err != nil {
+	if err := cp.configure(KeyWebSocketPingInterval, "30"); err != nil {
 		cp.log.DEBUG.Printf("failed configuring WebSocketPingInterval: %v", err)
 	}
 
@@ -164,18 +163,18 @@ func (cp *CP) HasMeasurement(val types.Measurand) bool {
 	return hasProperty(cp.meterValuesSample, string(val))
 }
 
-func (cp *CP) tryMeasurands(measurands string, key string, timeout time.Duration) []string {
+func (cp *CP) tryMeasurands(measurands string, key string) []string {
 	var accepted []string
 	for _, m := range strings.Split(measurands, ",") {
-		if err := cp.configure(key, m, timeout); err == nil {
+		if err := cp.configure(key, m); err == nil {
 			accepted = append(accepted, m)
 		}
 	}
 	return accepted
 }
 
 // configure updates CP configuration
-func (cp *CP) configure(key, val string, timeout time.Duration) error {
+func (cp *CP) configure(key, val string) error {
 	rc := make(chan error, 1)
 
 	err := Instance().ChangeConfiguration(cp.id, func(resp *core.ChangeConfigurationConfirmation, err error) {
@@ -186,5 +185,5 @@ func (cp *CP) configure(key, val string, timeout time.Duration) error {
 		rc <- err
 	}, key, val)
 
-	return Wait(err, rc, timeout)
+	return Wait(err, rc)
 }

charger/ocpp/cs.go

@@ -4,7 +4,6 @@ import (
 	"errors"
 	"fmt"
 	"sync"
-	"time"
 
 	"github.com/evcc-io/evcc/util"
 	ocpp16 "github.com/lorenzodonini/ocpp-go/ocpp1.6"
@@ -14,8 +13,7 @@ type CS struct {
 	mu  sync.Mutex
 	log *util.Logger
 	ocpp16.CentralSystem
-	cps     map[string]*CP
-	timeout time.Duration
+	cps map[string]*CP
 }
 
 // Register registers a charge point with the central system.

charger/ocpp/cs_core.go

@@ -21,7 +21,7 @@ func (cs *CS) TriggerResetRequest(id string, resetType core.ResetType) error {
 		rc <- err
 	}, resetType)
 
-	return Wait(err, rc, cs.timeout)
+	return Wait(err, rc)
 }
 
 func (cs *CS) TriggerMessageRequest(id string, requestedMessage remotetrigger.MessageTrigger, props ...func(request *remotetrigger.TriggerMessageRequest)) error {
@@ -35,7 +35,7 @@ func (cs *CS) TriggerMessageRequest(id string, requestedMessage remotetrigger.Me
 		rc <- err
 	}, requestedMessage, props...)
 
-	return Wait(err, rc, cs.timeout)
+	return Wait(err, rc)
 }
 
 func (cs *CS) ChangeAvailabilityRequest(id string, connector int, availabilityType core.AvailabilityType) error {
@@ -49,7 +49,7 @@ func (cs *CS) ChangeAvailabilityRequest(id string, connector int, availabilityTy
 		rc <- err
 	}, connector, availabilityType)
 
-	return Wait(err, rc, cs.timeout)
+	return Wait(err, rc)
 }
 
 // cp actions

charger/ocpp/helper.go

@@ -10,12 +10,12 @@ import (
 )
 
 // Wait waits for a CP roundtrip with timeout
-func Wait(err error, rc chan error, timeout time.Duration) error {
+func Wait(err error, rc chan error) error {
 	if err == nil {
 		select {
 		case err = <-rc:
 			close(rc)
-		case <-time.After(timeout):
+		case <-time.After(Timeout):
 			err = api.ErrTimeout
 		}
 	}

charger/ocpp/instance.go

@@ -46,7 +46,6 @@ func Instance() *CS {
 			log:           log,
 			cps:           make(map[string]*CP),
 			CentralSystem: cs,
-			timeout:       30 * time.Second,
 		}
 
 		ocppj.SetLogger(instance)