Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion charger/ocpp/connector.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,8 @@ type Connector struct {
txnId int
idTag string

remoteIdTag string
remoteIdTag string
remoteStarted bool // guards RemoteStartTransaction to once per auth cycle

meterInterval time.Duration
}
Expand Down
21 changes: 14 additions & 7 deletions charger/ocpp/connector_core.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,19 +48,26 @@ func (conn *Connector) OnStatusNotification(request *core.StatusNotificationRequ
conn.assumeMeterStopped()
}

// leaving Preparing status ends the current auth cycle
if applied && request.Status != core.ChargePointStatusPreparing {
conn.remoteStarted = false
}

if conn.isWaitingForAuth() {
if conn.remoteIdTag != "" {
// dispatch asynchronously: RemoteStartTransactionRequest issues a
// synchronous CS→CP request whose response is read by this same
// goroutine, so a blocking call would deadlock the WebSocket read
// loop (cf. ocpp_test_handler.go).
switch {
case conn.remoteIdTag == "":
conn.log.DEBUG.Printf("waiting for local authentication")

case !conn.remoteStarted:
// guard against re-firing on repeated Preparing notifications
conn.remoteStarted = true

// dispatch asynchronously
go func(idTag string) {
if err := conn.RemoteStartTransactionRequest(idTag); err != nil {
conn.log.ERROR.Printf("RemoteStartTransaction: %v", err)
}
}(conn.remoteIdTag)
} else {
conn.log.DEBUG.Printf("waiting for local authentication")
}
}

Expand Down
39 changes: 39 additions & 0 deletions charger/ocpp/connector_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,45 @@ func (suite *connTestSuite) TestOnStatusNotificationClearsStaleTxn() {
suite.True(suite.conn.NeedsAuthentication(), "Preparing after Available should require authentication")
}

// TestOnStatusNotificationRemoteStartGuard ensures RemoteStartTransaction is
// issued at most once per auth cycle: repeated Preparing notifications must not
// re-fire it, while a fresh cycle (leaving and re-entering Preparing) re-arms it.
func (suite *connTestSuite) TestOnStatusNotificationRemoteStartGuard() {
suite.conn.remoteIdTag = "evcc"

preparing := func(offset time.Duration) {
_, err := suite.conn.OnStatusNotification(&core.StatusNotificationRequest{
ConnectorId: 1,
Status: core.ChargePointStatusPreparing,
ErrorCode: core.NoError,
Timestamp: types.NewDateTime(suite.clock.Now().Add(offset)),
})
suite.NoError(err)
}

// first Preparing arms the guard
preparing(0)
suite.True(suite.conn.remoteStarted, "guard must be set after first Preparing")

// repeated Preparing must not re-arm (guard stays set)
preparing(time.Second)
suite.True(suite.conn.remoteStarted, "guard must stay set on repeated Preparing")

// leaving Preparing (e.g. cable unplugged) ends the auth cycle
_, err := suite.conn.OnStatusNotification(&core.StatusNotificationRequest{
ConnectorId: 1,
Status: core.ChargePointStatusAvailable,
ErrorCode: core.NoError,
Timestamp: types.NewDateTime(suite.clock.Now().Add(2 * time.Second)),
})
suite.NoError(err)
suite.False(suite.conn.remoteStarted, "guard must reset when leaving Preparing")

// next Preparing re-arms a fresh RemoteStartTransaction
preparing(3 * time.Second)
suite.True(suite.conn.remoteStarted, "guard must re-arm on the next auth cycle")
Comment on lines +237 to +249

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion (testing): Add coverage for other non-Preparing states that should also reset the RemoteStart guard

The production change resets remoteStarted on any transition away from Preparing, not only when going to Available. To fully exercise this behavior, please add cases that transition from Preparing to other statuses (e.g. Charging, SuspendedEV, SuspendedEVSE, or an error state) and assert that the guard resets in each case, possibly via a small table-driven test.

Suggested change
// leaving Preparing (e.g. cable unplugged) ends the auth cycle
_, err := suite.conn.OnStatusNotification(&core.StatusNotificationRequest{
ConnectorId: 1,
Status: core.ChargePointStatusAvailable,
ErrorCode: core.NoError,
Timestamp: types.NewDateTime(suite.clock.Now().Add(2 * time.Second)),
})
suite.NoError(err)
suite.False(suite.conn.remoteStarted, "guard must reset when leaving Preparing")
// next Preparing re-arms a fresh RemoteStartTransaction
preparing(3 * time.Second)
suite.True(suite.conn.remoteStarted, "guard must re-arm on the next auth cycle")
// leaving Preparing (e.g. cable unplugged) ends the auth cycle
_, err := suite.conn.OnStatusNotification(&core.StatusNotificationRequest{
ConnectorId: 1,
Status: core.ChargePointStatusAvailable,
ErrorCode: core.NoError,
Timestamp: types.NewDateTime(suite.clock.Now().Add(2 * time.Second)),
})
suite.NoError(err)
suite.False(suite.conn.remoteStarted, "guard must reset when leaving Preparing")
// leaving Preparing for any other status must also end the auth cycle and reset the guard
transitions := []struct {
name string
status core.ChargePointStatus
code core.ChargePointErrorCode
}{
{"Charging", core.ChargePointStatusCharging, core.NoError},
{"SuspendedEV", core.ChargePointStatusSuspendedEV, core.NoError},
{"SuspendedEVSE", core.ChargePointStatusSuspendedEVSE, core.NoError},
{"FaultedWithError", core.ChargePointStatusFaulted, core.InternalError},
}
for _, tt := range transitions {
// re-arm from Preparing
preparing(0)
suite.True(suite.conn.remoteStarted, "guard must be set before transitioning to %s", tt.name)
_, err := suite.conn.OnStatusNotification(&core.StatusNotificationRequest{
ConnectorId: 1,
Status: tt.status,
ErrorCode: tt.code,
Timestamp: types.NewDateTime(suite.clock.Now().Add(2 * time.Second)),
})
suite.NoError(err)
suite.False(suite.conn.remoteStarted, "guard must reset when leaving Preparing to %s", tt.name)
}
// next Preparing re-arms a fresh RemoteStartTransaction
preparing(3 * time.Second)
suite.True(suite.conn.remoteStarted, "guard must re-arm on the next auth cycle")

}

// TestOnStatusNotificationKeepsActiveTxn ensures that an active transaction is
// not cleared by transient status notifications other than Available.
func (suite *connTestSuite) TestOnStatusNotificationKeepsActiveTxn() {
Expand Down
Loading