Be more lenient towards bad OCPP implementations

[tvand]

In the analysis for #234 we found that the JSON parser throws an error if the attribute type in GetConfigurationKey is not exactly boolean. In my case, my charge box happens to use strings there.

I thought I could make an attempt to be more lenient towards violations of the standard. Would you please consider including this small fix? The test shows that it does no harm and works as expected.

[andig]

Imho this tends to promote not following standards.

[lorenzodonini]

I am very torn about this. While the PR itself is harmless the main issue I see is: the moment I start giving special treatment to non-conform implementations then there will be more requests coming in and the library will end up becoming a mess of custom StringOrIntOrFloat types, or simply a fest of interface{} types to please everyone.

I wrote the lib using a strongly-typed language specifically for enforcing the standard and never planned to deviate much from it. I am well aware that many wallboxes don't fully follow the standard or have bugs in their implementation, but I also don't see a quick way to support "custom type injection". I would need to think about this.

Is there no way you can get the manufacturer to patch the wallbox software to pass the correct JSON type?

[rogeralsing]

My 2 cents on the topic.
I'm not a user of this lib as I'm mostly on other platforms than Go.
I have however helped multiple companies build their OCPP servers and I've seen some truly odd stuff out there.

Basically, none of the actual hardware vendors conform fully to the specs.
So you can choose to have a theoretically correct library, according to spec.

Or, a library that works with real-world chargers.

Think of it as HTML back in the early days of the web.
Of course, it would have been nice if developers produced proper HTML.
Creating a browser would have been so much easier.
But for adoption, browsers had to conform to all the weirdness out there.

Is there no way you can get the manufacturer to patch the wallbox software to pass the correct JSON type?

Some early boxes aren't even able to automatically update software, I've had customers who had to drive by car to all their clients' homes and manually update the firmware.

Time to market, get your stuff out there, even if incomplete, is a real thing and critical to take market share. people will cut corners when they can.
Business and cashflow will win over strict standards and perfect code.

[tvand]

I further simplified the patch. It is now even less intrusive (does not use a custom type) and even more lenient. I have tested it to work with my charger.

BTW:

Is there no way you can get the manufacturer to patch the wallbox software to pass the correct JSON type?

They haven't even responded to my inquiry.

[andig]

TBO, I wouldn't go this way. If you think about #234, you may be able to parse the charger's message but you have zero idea if it understands what you are sending. Potentially even accepting messages it doesn't understand at all.

[lorenzodonini]

@rogeralsing

Counter-arguments and rationale

Here are a few thoughts regarding your (valid) points.

So you can choose to have a theoretically correct library, according to spec.
Or, a library that works with real-world chargers.

This suggestion sadly doesn’t scale, which is the point I tried to make above. If the library is changed to support a specific vendor, it possibly breaks compatibility with another vendor. If the library starts accomodating every single violation of the spec by including custom types instead of primitive types:

• the code will quickly become unmaintainable
• devs will have a hard time understanding what a field is supposed to be
• the excessive usage of any types will completely defeat the purpose of a strongly typed language

Furthermore we are talking about an incorrect JSON type in this case (there’s only 5 JSON types to begin with, so getting one wrong is imo a big deal and should be fixed by the vendor).

Time to market, get your stuff out there, even if incomplete, is a real thing and critical to take market share. people will cut corners when they can.

I’m perfectly aware and know there are tradeoffs in the real world. At the same time I noticed how many companies decided to implement the OCPP stack in-house, which ultimately leads to deviation from the spec, inconsistencies and bugs. I could argue that if everybody relied on OSS implementations from the start, today most chargers out there would be spec-compliant (this immediately comes to mind).

What I genuinely aim to achieve with this library is to reduce this gap and give developers the tools to be spec-compliant without having to rewrite the entire thing from scratch.

[lorenzodonini]

The constructive bit - RFC

I thought about this a lot and to address the issue at hand I started working on a compromise solution that will allow supporting “custom types” in order to deal with spec violations. Here’s a very rough first idea:

// This type is exported by the ocpp lib
type ExpectedRequest struct {
		ID int64 `json:"id"`
		Foo bool `json:"foo"`
}

// Custom override to support invalid types and fields. 
// The type can have its own un/marshaler function if needed.
type MyCustomRequest struct {
		ID int64 `json:"id"`
		Foo string `json:"foo"`
}

// The hook invoked by the lib for converting a non-compliant payload into an OCPP type
func (r* MyCustomRequest)ToOCPP(targetPayload interface{}) error {
		result, _ := targetPayload.(*ExpectedRequest)
		// convert to expected type and set all fields accordingly
		result.ID = r.ID
		if r.Foo == "true" {
				result.Foo = true
		}
		return nil
}

// The hook invoked by the lib for serializing an outgoing OCPP payload to a non-compliant type
func (r *MyCustomRequest)FromOCPP(payload interface{}) err error {
		req, _ := payload.(*ExpectedRequest)
		// transform to custom type
		r := MyCustomRequest{
				ID: req.ID,
				r.Foo: "false",
		}
		if req.Foo {
				r.Foo = "true"
		}
		// Will be marshaled by the lib
		return nil
}

This could be configurable on a per-charger basis.

The downside is that to support heavily non-conforming vendors it will require writing lots of custom hooks and maintaining those. I guess this price needs to be payed though, if a server implementation is supporting lots of non-conform vendors.

This is just a first draft, I’m still working on how to integrate this in a clean way.

WDYT? Any feedback is welcome

[lorenzodonini]

@tvand Please check my suggestion above and let me know if this would be a valid solution for you.

It will still take me a bit to fully implement and test though, so if you're pressed for time I could just merge this PR. However I would revert it down the road to replace it with the generic solution.

[rogeralsing]

• the code will quickly become unmaintainable
• devs will have a hard time understanding what a field is supposed to be
• the excessive usage of any types will completely defeat the purpose of a strongly typed language

I think much of this could simply be solved using some form of preprocessor before the json parsing is done.
while still keeping the strongly typed nature of the OCPP messages.

e.g. if there were some form of raw string middleware that allows you to transform the input and pass that onto the next step.
then you could likely have middlewares for each vendor and then curate the JSON string before it is parsed at the end.

That is how I see most companies solve this in their inhouse implementations.

[lorenzodonini]

@rogeralsing curating the JSON string for N message types sounds more cumbersome than parsing the JSON into the "wrong" type and then converting it to the correct type. But I totally get the point and I'm trying to come up with a similar general-purpose functionality to solve this.

[andig]

@rogeralsing you‘re making good arguments against doing this- at all ;)