Skip to content
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

Add dateparse and DefaultTimeFormat #34

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

Houtmann
Copy link

@Houtmann Houtmann commented Oct 25, 2020

I dot this pull request because in reality when you write an OCPP central system server, you may have more than one brand of chargepoint on the same central system.
For example in my company, we have 4 brands of chargepoint connect on the server.
And in fact chargepoint's brands do not meet the standards DateTime.

The library dateparse allows us to deal with this problem;

Add datetime_test.go
@lorenzodonini
Copy link
Owner

lorenzodonini commented Oct 27, 2020

Hey Had,
you make a good point there.

I originally thought of keeping the included batteries to a minimum and enforce the specifications, which clearly say that:

  • ISO 8601 MUST be used for OCPP 1.6
  • RFC 3339 SHALL be used for OCPP 2.0.x (except for meter-aligned data, which still follows ISO 8601)

Imho charge point vendors should conform to these standards, however it makes sense to provide some flexibility for devs.

It would be really nice, if you could add the same change for the ocpp2.0/types package directly (the code is basically the same).

@Houtmann
Copy link
Author

All right, I'll do that. As I said, 4 different brands, 4 different date formats...

@lorenzodonini
Copy link
Owner

Just checked out your branch and ran the unit tests.
Some of them are now failing, apparently because the datetime equality checks are not passing.

Can you make look into it and figure out why? Otherwise I'll do it as soon as I have some time.

@Houtmann
Copy link
Author

Houtmann commented Nov 5, 2020

Yes I'll look into the problem

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants