EVerest / everest-core

Apache License 2.0
92 stars 68 forks source link

Added in patch which supports PNC #750

Open louisg1337 opened 2 months ago

louisg1337 commented 2 months ago

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

SebaLukas commented 2 months ago

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 commented 2 months ago

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 commented 5 days ago

@louisg1337 Any thoughts on my suggestion?