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

FIXES TYPO IN OccurrenceConstraintViolation #331

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Sharath1804
Copy link

@Sharath1804 Sharath1804 commented Jan 8, 2025

Proposed changes

Currently when there are errors in ocpp message central system responds to the chargers with OccurrenceConstraintViolation error code for example, a missing value for a measurand in the MeterValues.Req.

However, there is an extra "r" in the error code and is a deviation from the OCPP spec. This also causes few of the chargers to indefinitely send the same faulty OCPP message until its cache gets exhausted and needs to be fixed.
Error code from the documentation - https://downloads.regulations.gov/FHWA-2022-0008-0403/attachment_6.pdf

As part of this change the type is fixed and the status has been changed from OccurrenceConstraintViolation to OccurenceConstraintViolation.

NOTE: OccurrenceConstraintViolation is a typo in both OCPP 1.6 and OCPP 2.0.1. The right error code for both the versions is OccurenceConstraintViolation.

Types of changes

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation Update (if none of the other choices apply)

Checklist

  • I have read the CONTRIBUTING doc
  • I have signed the CLA
  • Lint and unit tests pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)
  • Any dependent changes have been merged and published in downstream modules

Further comments

NA

Currently we respond to the chargers with OccurrenceConstraintViolation
when there are issues with the ocpp message itself for example,
a missing value for a measurand in the MeterValues.Req.

However, there is an extra "r" in the message status and is a deviation
from the OCPP spec. This also causes few of the chargers to indefinitely
send the same faulty OCPP message until its cache gets exhausted and needs
to be fixed.

As part of this change the type is fixed and the status has been changed
from OccurrenceConstraintViolation to OccurenceConstraintViolation.
@lorenzodonini
Copy link
Owner

Thanks for spotting. This is similar to #211.

Basically:

  • v1.6 should keep the version with the typo OccurenceConstraintViolation
  • v2 should use the correct version without the typo OccurrenceConstraintViolation

To not break the v2 implementation, the same approach as in the linked PR is needed. Could you adjust the changes accordingly? Otherwise I will take care of it

@Sharath1804
Copy link
Author

Sharath1804 commented Jan 19, 2025

Thanks for spotting. This is similar to #211.

Basically:

  • v1.6 should keep the version with the typo OccurenceConstraintViolation
  • v2 should use the correct version without the typo OccurrenceConstraintViolation

To not break the v2 implementation, the same approach as in the linked PR is needed. Could you adjust the changes accordingly? Otherwise I will take care of it

Hello,

As per #211 we have two different error codes for OCPP 1.6 and OCPP 2.0.1 i.e., FormationViolation vs FormatViolation respectively. However OccurenceConstraintViolation holds good for both OCPP 1.6 and OCPP 2.0.1.

Please find the attached screen grabs from OCPP 1.6 and OCPP 2.0.1 spec indicating the same value for both the OCPP versions.
Screenshot 2025-01-19 at 11 48 34 AM
Screenshot 2025-01-19 at 11 48 53 AM

@lorenzodonini
Copy link
Owner

@Sharath1804 The first document you linked is not in sync with the actual ocpp-j spec (it's a separate document, which is where the RPC error codes are defined). I blame it on a typo on that specific section you quoted.

Here's the actual spec updated to its latest version. I also double-checked with an older version I had lying around and the value is unchanged since the inception of v2.

In other words: my previous statement holds and we have to support two different constants, one for v1.6 and one for v2.

Screenshot 2025-01-19 at 21 01 52

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