diff --git a/charger/ocpp.go b/charger/ocpp.go index 75b22dc1f9..40f0cc85fa 100644 --- a/charger/ocpp.go +++ b/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) diff --git a/charger/ocpp/connector.go b/charger/ocpp/connector.go index 3f545801ae..d032cf9560 100644 --- a/charger/ocpp/connector.go +++ b/charger/ocpp/connector.go @@ -27,14 +27,13 @@ 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, @@ -42,7 +41,6 @@ func NewConnector(log *util.Logger, id int, cp *CP, timeout time.Duration) (*Con 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) diff --git a/charger/ocpp/connector_core.go b/charger/ocpp/connector_core.go index 9a9a423386..4531ac9ca3 100644 --- a/charger/ocpp/connector_core.go +++ b/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 diff --git a/charger/ocpp/connector_test.go b/charger/ocpp/connector_test.go index ebe0af0c7d..cc6bd8cd8c 100644 --- a/charger/ocpp/connector_test.go +++ b/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 diff --git a/charger/ocpp/const.go b/charger/ocpp/const.go index 872f0fd679..be8458eaec 100644 --- a/charger/ocpp/const.go +++ b/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" diff --git a/charger/ocpp/cp_core.go b/charger/ocpp/cp_core.go index 3d0fea73c0..8dee46f316 100644 --- a/charger/ocpp/cp_core.go +++ b/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") diff --git a/charger/ocpp/cp_setup.go b/charger/ocpp/cp_setup.go index 3d43811903..b8499926bb 100644 --- a/charger/ocpp/cp_setup.go +++ b/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,10 +163,10 @@ 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) } } @@ -175,7 +174,7 @@ func (cp *CP) tryMeasurands(measurands string, key string, timeout time.Duration } // 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) } diff --git a/charger/ocpp/cs.go b/charger/ocpp/cs.go index 84f8ff414a..70e0d19869 100644 --- a/charger/ocpp/cs.go +++ b/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. diff --git a/charger/ocpp/cs_core.go b/charger/ocpp/cs_core.go index b60eed03bb..6351bb8a20 100644 --- a/charger/ocpp/cs_core.go +++ b/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 diff --git a/charger/ocpp/helper.go b/charger/ocpp/helper.go index 7e2f087b16..7a996680b6 100644 --- a/charger/ocpp/helper.go +++ b/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 } } diff --git a/charger/ocpp/instance.go b/charger/ocpp/instance.go index a78a5a5826..94624a367b 100644 --- a/charger/ocpp/instance.go +++ b/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) diff --git a/charger/ocpp_test.go b/charger/ocpp_test.go index ebf9838923..dfe602c9a1 100644 --- a/charger/ocpp_test.go +++ b/charger/ocpp_test.go @@ -18,7 +18,6 @@ import ( const ( ocppTestUrl = "ws://localhost:8887" ocppTestConnectTimeout = 10 * time.Second - ocppTestTimeout = 3 * time.Second ) func TestOcpp(t *testing.T) { @@ -59,6 +58,13 @@ func (suite *ocppTestSuite) startChargePoint(id string, connectorId int) ocpp16. func (suite *ocppTestSuite) handleTrigger(cp ocpp16.ChargePoint, connectorId int, msg remotetrigger.MessageTrigger) { switch msg { + case core.BootNotificationFeatureName: + if res, err := cp.BootNotification("model", "vendor"); err != nil { + suite.T().Log("BootNotification:", err) + } else { + suite.T().Log("BootNotification:", res) + } + case core.ChangeAvailabilityFeatureName: fallthrough @@ -96,12 +102,12 @@ func (suite *ocppTestSuite) TestConnect() { suite.Require().True(cp1.IsConnected()) // 1st charge point- local - c1, err := NewOCPP("test-1", 1, "", "", 0, false, true, ocppTestConnectTimeout, ocppTestTimeout) + c1, err := NewOCPP("test-1", 1, "", "", 0, false, true, ocppTestConnectTimeout) suite.Require().NoError(err) // status and meter values { - suite.clock.Add(ocppTestTimeout) + suite.clock.Add(ocpp.Timeout) c1.conn.TestClock(suite.clock) // status @@ -145,11 +151,11 @@ func (suite *ocppTestSuite) TestConnect() { suite.Require().True(cp2.IsConnected()) // 2nd charge point - local - c2, err := NewOCPP("test-2", 1, "", "", 0, false, true, ocppTestConnectTimeout, ocppTestTimeout) + c2, err := NewOCPP("test-2", 1, "", "", 0, false, true, ocppTestConnectTimeout) suite.Require().NoError(err) { - suite.clock.Add(ocppTestTimeout) + suite.clock.Add(ocpp.Timeout) c2.conn.TestClock(suite.clock) // status @@ -187,12 +193,12 @@ func (suite *ocppTestSuite) TestAutoStart() { suite.Require().True(cp1.IsConnected()) // 1st charge point- local - c1, err := NewOCPP("test-3", 1, "", "", 0, false, false, ocppTestConnectTimeout, ocppTestTimeout) + c1, err := NewOCPP("test-3", 1, "", "", 0, false, false, ocppTestConnectTimeout) suite.Require().NoError(err) // status and meter values { - suite.clock.Add(ocppTestTimeout) + suite.clock.Add(ocpp.Timeout) c1.conn.TestClock(suite.clock) } diff --git a/charger/ocpp_timeout.go b/charger/ocpp_timeout.go deleted file mode 100644 index a05f511537..0000000000 --- a/charger/ocpp_timeout.go +++ /dev/null @@ -1,18 +0,0 @@ -package charger - -import ( - "testing" - "time" -) - -var ( - ocppConnectTimeout = 5 * time.Minute - ocppTimeout = 2 * time.Minute -) - -func init() { - if testing.Testing() { - ocppConnectTimeout = 1 * time.Second - ocppTimeout = 1 * time.Second - } -} diff --git a/util/templates/defaults.yaml b/util/templates/defaults.yaml index b6623df254..c877a85aca 100644 --- a/util/templates/defaults.yaml +++ b/util/templates/defaults.yaml @@ -388,10 +388,7 @@ presets: - name: timeout advanced: true type: duration - default: 30s - description: - de: Zeitlimit für die Beantwortung von einzelnen OCPP-Nachrichten - en: Timeout for individual OCPP messages + deprecated: true - name: meterinterval advanced: true type: duration