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

feature(OCPP1.6): forward VendorWarning appropriately #949

Merged
merged 8 commits into from
Nov 12, 2024

Conversation

a-w50
Copy link
Contributor

@a-w50 a-w50 commented Nov 7, 2024

  • this change will treat the VendorWarning the same way as VendorError, so that also the error.message becomes visible in the OCPP StatusNotification message

Describe your changes

Issue ticket number and link

Checklist before requesting a review

  • I have performed a self-review of my code
  • I have made corresponding changes to the documentation
  • I read the contribution documentation and made sure that my changes meet its requirements

@Pietfried Pietfried self-assigned this Nov 7, 2024
Copy link
Contributor

@Pietfried Pietfried left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good and well documented! There is one conflict to resolve.

nit: I'd prefer if you keep using the terms EVerest and StatusNotification.req like in the rest of the file

- this change will treat the `VendorWarning` the same way as
  `VendorError`, so that also the `error.message` becomes visible in the
  OCPP StatusNotification message

Signed-off-by: aw <aw@pionix.de>
Comment on lines 284 to 292
* status notification charge point ``errorCode`` will always be
``OtherError``
* status notification ``status`` will reflect the present status of the
charge point
* status notification ``info`` -> origin of everest error
* status notification ``vendorErrorCode`` -> everest error type and
subtype (the error type is simplified, meaning, that its leading part,
the interface name, is stripped)
* status notification ``vendorId`` -> everest error message
Copy link
Contributor

@Pietfried Pietfried Nov 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* status notification charge point ``errorCode`` will always be
``OtherError``
* status notification ``status`` will reflect the present status of the
charge point
* status notification ``info`` -> origin of everest error
* status notification ``vendorErrorCode`` -> everest error type and
subtype (the error type is simplified, meaning, that its leading part,
the interface name, is stripped)
* status notification ``vendorId`` -> everest error message
* **StatusNotification.req** property ``errorCode`` will always be
``OtherError``
* **StatusNotification.req** property ``status`` will reflect the present status of the
charge point
* **StatusNotification.req** property ``info`` -> origin of EVerest error
* **StatusNotification.req** property ``vendorErrorCode`` -> EVerest error type and
subtype (the error type is simplified, meaning, that its leading part,
the interface name, is stripped)
* **StatusNotification.req** property ``vendorId`` -> EVerest error message


The fields **info**, **vendorId**, and **vendorErrorCode** are set based on the error type and the provided error properties. Please see the
definition of `get_error_info` to see how the **StatusNotification.req** is constructed based on the given error.
For all other errors, raised in everest, the following mapping to an
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
For all other errors, raised in everest, the following mapping to an
For all other errors, raised in EVerest, the following mapping to an

- the forwarding of everest errors to ocpp status notifications has been
  generalized
- the generic mapping from an everest error to an ocpp status
  notification is done now as follows:
  - status notification charge point error code will always be `OtherError`
  - status notification info -> origin of everest error
  - status notification vendor error code -> everest error type and
    subtype (the error type is simplified, meaning, that its leading
    part, the interface name, is stripped)
  - status notification vendor id -> everest error message
- the main choice for using the status notification vendor id for the
  error message is that it can carry the largest string (255
  characters), whereas the other fields only allow up to 50 characters

Signed-off-by: aw <aw@pionix.de>
Signed-off-by: aw <aw@pionix.de>
Signed-off-by: aw <aw@pionix.de>
Signed-off-by: aw <aw@pionix.de>
Signed-off-by: aw <aw@pionix.de>
Signed-off-by: aw <aw@pionix.de>
Signed-off-by: aw <aw@pionix.de>
@a-w50 a-w50 force-pushed the feature/forward_vendor_warning_appropriately branch from 30d43f8 to 0212d72 Compare November 12, 2024 08:54
@a-w50 a-w50 merged commit 23d3dc6 into main Nov 12, 2024
10 checks passed
@a-w50 a-w50 deleted the feature/forward_vendor_warning_appropriately branch November 12, 2024 09:30
hikinggrass pushed a commit that referenced this pull request Nov 21, 2024
* feature(OCPP1.6): forward VendorWarning appropriately
- the forwarding of everest errors to ocpp status notifications has been
  generalized
- the generic mapping from an everest error to an ocpp status
  notification is done now as follows:
  - status notification charge point error code will always be `OtherError`
  - status notification info -> origin of everest error
  - status notification vendor error code -> everest error type and
    subtype (the error type is simplified, meaning, that its leading
    part, the interface name, is stripped)
  - status notification vendor id -> everest error message
- the main choice for using the status notification vendor id for the
  error message is that it can carry the largest string (255
  characters), whereas the other fields only allow up to 50 characters

Signed-off-by: aw <aw@pionix.de>
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