-
Notifications
You must be signed in to change notification settings - Fork 163
Description
📜 Description
Hi Lorenzo,
We've encountered a recurring panic in the OCPP library related to interface conversion errors when handling asynchronous responses. Here's a sample of the error:
panic: interface conversion: ocpp.Response is *remotetrigger.TriggerMessageConfirmation, not *smartcharging.SetChargingProfileConfirmation
panic: interface conversion: ocpp.Response is *core.HeartbeatConfirmation, not *core.MeterValuesConfirmation
This issue occurred on at least two charging stations—one of them crashed up to 20 times per day due to this.
Investigation
We reviewed previous issues on the GitHub repo, including:
- Panic when TriggerMessage and Change/Get Configuration are called at the same time #67: Panic when
TriggerMessageandChange/GetConfigurationare called concurrently - CS confuses error responses between requests in case of timeout #294: CS confuses error responses between requests in case of timeout
We traced the root cause to how the callback queue works in callbackqueue.go.
Problem
The current implementation queues callbacks per ClientID, but dequeues them in FIFO order. This assumes that responses always arrive in sequence, which doesn't hold if a station delays or times out.
In timeout scenarios:
- The callback is dequeued due to timeout.
- A delayed response later arrives.
- Its callback no longer exists, so the wrong handler is invoked.
- This results in an interface conversion panic.
Our Solution
We modified the CallbackQueue to maintain callbacks by both ClientID and request type. That way, callbacks are queued and dequeued by type, avoiding mismatched handlers.
type CallbackQueue struct {
callbacksMutex sync.RWMutex
// Old:
// callbacks map[string][]func(confirmation ocpp.Response, err error)
// New:
callbacks map[string]map[RequestType][]func(confirmation ocpp.Response, err error)
}This change has completely eliminated the panic in our environment.
Alternative Idea
A different approach could be queuing by message ID instead, and validating callback presence before handling.
Next Steps
Would you be open to reviewing this solution for inclusion in the library? I’m happy to submit a pull request if you think it’s a good direction.
Thanks again for your great work on this project!
Best regards,
👟 Reproduction steps
- The callback is dequeued due to timeout.
- A delayed response later arrives.
- Its callback no longer exists, so the wrong handler is invoked.
- This results in an interface conversion panic.
👍 Expected behavior
Callback queue dequeue should find answer by type to avoid panic, not only dequeuing the last message. A different approach could be queuing by message ID instead, and validating callback presence before handling.
👎 Actual Behavior
Callback queue dequeue uses last Message only, this leads to panic if that one doesn't match.
What OCPP version are you using?
- OCPP 1.6
- OCPP 2.0.1
Are you using any OCPP extensions?
- OCPP 1.6 Security
- OCPP 1.6 Plug and Charge
release version
No response
📃 Provide any additional context for the Bug.
No response
👀 Have you spent some time to check if this bug has been found before?
- I checked and didn't find a similar issue