lorenzodonini / ocpp-go

Open Charge Point Protocol implementation in Go
MIT License
262 stars 125 forks source link

Allow Many Pending Request #22

Closed Houtmann closed 3 years ago

Houtmann commented 4 years ago

Hi I think you should allow many pending Request, because a chargepoint can send two or more request in very close time, and and the server may be busy answering

hasPendingRequest

lorenzodonini commented 4 years ago

Hey there, it's on my backlog, I simply haven't had the time to implement it yet.

Originally I didn't add this feature because:

But I do understand the pain of having to handle a message queue within the application logic. I will work on adding this feature whenever possible.

Houtmann commented 4 years ago

Maybe I can try to help if i find time. I have write real private OCPP client (in python) on chargepoint, and disconnected management it's important. In my company,a Chargepoint can operate offline and report events when the connection is resumed

lorenzodonini commented 4 years ago

Sure, I wouldn't mind some help.

Let's split it into smaller features. I will implement the message queue (because I know what code needs to be modified), which is also the topic of this issue. For each follow-up issue, let's create a separate issue and branch. You can take a look at the PR I will create for this. Also feel free to submit PRs for any follow-up issues, in case you want to contribute directly 😄

Houtmann commented 4 years ago

I'll look forward to your pull request ;). If you want contribution maybe, you can create some issues, and i will look :)

lorenzodonini commented 4 years ago

@Houtmann feel free to check out https://github.com/lorenzodonini/ocpp-go/pull/29. The PR is quite big, since I had to update lots of tests and the examples. The new functionality can be found entirely within:

Like I mentioned in the description, there's still a few limitations, which I would update in a separate PR. I should be able to do that in the coming week already.

Houtmann commented 4 years ago

I watched the pull request, my idea may be too late, but why not bring the Message Queue to the level of the websocket ? so the queue is the same for all types of messages (CALL, CALL_RESULT)

lorenzodonini commented 4 years ago

I thought about it, but by queuing everything within the websocket package/layer, you lose important information about the message (e.g. the message ID). This information is needed when getting a response, since the response needs to be matched to the original request. Information about the original request needs to be saved inside the ocppj layer anyways, so might as well handle the queue there. Also, OCPP specifies a request-response mechanism, in which only one request may be sent at a time (but you can send as many responses/errors as you want). Only the ocppj layer knows, whether a request or a response is being queued (websocket layer only understands byte arrays).

In short, I believe it would get messy to put that logic inside the websocket layer, and I would need to duplicate some state inside the ocppj layer anyways. Or did I miss something?

Houtmann commented 3 years ago

no you hadn't forgotten anything, it's my misinterpretation. I think I can close this issue