lorenzodonini / ocpp-go

Open Charge Point Protocol implementation in Go
MIT License
277 stars 126 forks source link

Websocket Disconnect Handler not called #288

Open sc-atompower opened 3 months ago

sc-atompower commented 3 months ago

OCPP version: [x] 1.6 [ ] 2.0.1

I'm submitting a ...

[ x] bug report [x ] feature request

Current behavior:

When encountering the following error:

{"level":"error","ts":1724167327.3348188,"msg":"disconnected from serverwebsocket: close 1006 (abnormal closure): unexpected EOF","logger":"ocppj"}

the websocket disconnect handler is not called

Expected behavior:

wsc.SetDisconnectedHandler(station.disconnectedHandler)
func (s *OcppStation) disconnectedHandler(err error) {
    s.logger.Error("disconnectedHandler: websocket disconnected handler called", zap.Error(err))
}

Currently the websocket logger is a single logger instance shared across all websocket connections. since out ocpp implementation is a middle man relay service we share the same logger across lots of ocpp stations. we can never tell which station is experiencing the disconnect.

It would be nice to have the disconnect handler be able to print the station Id but first we need it to fire in general.

Feature request would be to add a logger per websocket connection or attach context to the websocket logger.

lorenzodonini commented 2 months ago

Callbacks on the ws layer are not meant to be set directly, as they will be overwritten when the OCPP-J client is started. The callback propagates automatically to the top. To get this to work the way you want to:

c := ocppj.NewClient(id, wsc, nil, nil, core.Profile, localauth.Profile, firmware.Profile, reservation.Profile, remotetrigger.Profile, smartcharging.Profile)
c.SetOnDisconnectedHandler(disconnectedHandler)

station := ocpp.NewChargePoint(id, c, wsc)
station.Start(url)

This way your callback should be invoked correctly.

That being said, I'm a bit confused about the motivation: the callback is set on a client object (running on the charging station). The client has a single ID by definition, so passing any additional parameters to that callback seems redundant. Why would this be needed?