-
Notifications
You must be signed in to change notification settings - Fork 124
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
CS confuses error responses between requests in case of timeout #294
Comments
Here's another log from testing. All timeouts are default except for dispatcher timeout set to 5s for quicker test results:
The client timing out is simulated by letting handler wait in a delay: func (handler *ChargePointHandler) OnChangeAvailability(request *core.ChangeAvailabilityRequest) (confirmation *core.ChangeAvailabilityConfirmation, err error) {
time.Sleep(time.Minute)
return core.NewChangeAvailabilityConfirmation(core.AvailabilityStatusAccepted), nil
} At this point I'm no longer sure if what I'm seeing here is really the real issue or if this is an artefact from testing setup. Log shows that |
If the behaviour here is due to the simulated CP during testing, it would be great to have a different/better way of simulating CP timeouts. For example, the CP implementation could allow returning a sentinel error that is then not relayed to the CS. |
@lorenzodonini if you would consider accepting a bounty for resolving this issue it would be great if you could reach out to info@evcc.io. Thank you! |
Indeed it seems that this is a testing artefact. I have changed the CP
It's now obvious that the test continues without blocking. |
That said: it would be great if we had a means of simulating missing request acceptance or missing reply from either CS or CP. It seems this is currently not possible? |
Not totally sure if this is related but we are starting to see panics due to on checked casting of ocpp resonses. It looks like something can happen and any call can get another calls response and panic. Examples:
@lorenzodonini this may interest you |
@sc-atompower did this start at a specific point in time? Are you on master branch or latest release? It's tricky to reproduce but I'll look into it, since this sounds like a major issue. |
@lorenzodonini it started as soon as we went from v0.16.1-0.20230417193019-3ecaf00b33b6 to github.com/lorenzodonini/ocpp-go v0.18.0 and it just happened again. |
@lorenzodonini we reverted to version 17 and still had this issue. We did note that it occurred after a very long hanging request. We had a client take 57 seconds to process a meter values call. Then we panic.
we are going to revert back to 16 tomorrow. |
Just to clear up the base functionality:
Even assuming there is some glitch caused by timeouts, I don't see how the response can be parsed using the incorrect type. The ParseMessage function returns an error if the incoming response doesn't match a pending (already sent) request. |
@sc-atompower The behavior you're experience would only make sense if two requests of different types have the same ID, and there is some glitch (which I'm investigating). Could you please confirm that:
|
OCPP version:
[x] 1.6
[ ] 2.0.1
I'm submitting a ...
[x] bug report
[ ] feature request
Current behavior:
At evcc-io/evcc#15991 we're experiencing an issue where a message timeout on one message is returned for a different message.
Expected behavior:
When CP does not reply to a CS request, a following (or concurrent) CS request (that the CP replies to) should not receive an error result.
Consider this log:
Steps to reproduce:
Related code:
Other information:
The text was updated successfully, but these errors were encountered: