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

Added in patch which supports PNC #750

Closed

Conversation

louisg1337
Copy link

Describe your changes

Hello everyone, I am apart of the team over at NREL who has been working on the SIL demos in everest/everest-demo. Recently, @shankari went to the CharIN Testival and showcased EVerest by running a custom demo that was tailored to have PnC functioning, found here. To get PnC to work, she had to apply a handful of patches to different parts of EVerest, all which can be found here. We now would like to slowly integrate these patches into EVerest so that it can fully support PnC.

This patch in particular involves adding in support for a Contract payment method to PyEvJosev.

Testing Done

As mentioned above, this patch is one of many, so it has only been tested in conjunction with the other patches in the SIL demo. The particular demo which has PnC working can be found linked above. To run this demo, I would recommend...

  1. Clone repo and check out branch.
    git clone https://github.com/US-JOET/everest-demo.git
    git checkout -b test-demo origin/charin-e2e-demo  
    
  2. Run the software simulation demo.
    bash demo-iso15118-2-ac-plus-ocpp.sh -r $(pwd) -b test-demo -3   
    
  3. Wait until the EVerest software boots up and you see the following.
    2024-06-18 18:23:22.836788 [INFO] ocpp:OCPP201     :: Received BootNotificationResponse: {
        "currentTime": "2024-06-18T18:23:22.000Z",
        "interval": 300,
        "status": "Accepted"
    }
    
  4. Navigate to http://localhost:1880/ui/, choose from the Car Simulation dropdown menu AC ISO15118-2 Plug&Charge, then plug the car in and observe it authenticate on its own.

Issue ticket number and link

The issue can be found here in everest-demo.

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

@SebaLukas
Copy link
Contributor

Hello :)

thank you very much for your contribution. However, after looking through it, I have decided not to accept this PR and close it in a few days.

Because, the changes this PR describes, I deleted from everest-core a few months ago. The payment option in the iso_start_v2g_session cmd had no effect on Josev at that time.
I have seen that there is another PR in the Josev fork and that the option selected here is actually also used by Josev.

However, I find this approach less realistic and less effective than the current situation.

At the moment, the Josev EV side itself decides whether to select PnC. Certain conditions must be met: TLS must be active and the EVSE must also offer PnC as a payment option. PnC only works in the SIL if these conditions are met. The config also makes it possible to test certain scenarios, such as TLS + EIM.

This PR can lead to strange behavior on the EVCC side if, for example, PnC is selected and the charger only supports EIM. This cannot happen with the current status. For me, this means that another potential source of error is being added here when executing the SIL.

I'm sorry, but I see no reason why the payment option should be added again.

@SebaLukas
Copy link
Contributor

After a brief discussion with @corneliusclaussen, I can understand why you would want to select PnC or EIM via node-red when EVerest is running. And don't always want to restart everest with a different config.

I still don't like the possibility of setting the payment option directly for the car sim.

As an alternative, however, I could imagine that you can set a payment option priority using the iso_start_v2g_session cmd.
A priority list could be defined in Josev. E.g. EIM: 1, PnC: 2 (small number -> higher priority) The same principle applies to the SupportAppProtocol message.
I.e. if Josev can select both options (EIM and PnC), Josev selects the relevant payment option depending on the priority. The cmd can be used to change the priority. E.g. iso_start_ DC, PnC -> EIM: 2, PnC: 1
This has the advantage that if the priority is set to PnC but the Charger can only do EIM, PnC is ignored by Josev and EIM is selected. And you can "change" the payment options during running EVerest via node-red.

The options for not sending a payment option via the cmd do not work. The interface system does not allow optional arguments. Therefore the iso15118_ev interface should also be adapted here. All node-red yaml files in the config folder would also have to be adapted.

What do you think of the suggestion?

@SebaLukas SebaLukas self-assigned this Jul 2, 2024
@SebaLukas SebaLukas added the post-release Tag that this PR should not go into the current merge window for the upcoming release label Jul 5, 2024
@corneliusclaussen corneliusclaussen force-pushed the support-payment-jsevmanager-patch branch from 59cd1be to 50f4142 Compare July 19, 2024 10:17
@corneliusclaussen corneliusclaussen removed the post-release Tag that this PR should not go into the current merge window for the upcoming release label Jul 19, 2024
@SebaLukas
Copy link
Contributor

@louisg1337 Any thoughts on my suggestion?

@corneliusclaussen
Copy link
Contributor

Is this still being worked on or should we close it?

@SebaLukas
Copy link
Contributor

I'll close this next friday if nobody answers

@SebaLukas
Copy link
Contributor

So I close this one. If there is a need, then please reopen a comment on my proposal.

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.

3 participants