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

Daheimladen: Charging station is enabled only when in 'Charging' state #2858

Merged
merged 1 commit into from
Apr 3, 2022

Conversation

utsavanand2
Copy link
Contributor

Fixes: #2273

Signed-off-by: Utsav Anand utsavanand2@gmail.com

Description:

DaheimLaden charging station needs to return true for Enabled function only when the station is in "Charging" state.
This will get rid of "charger out of sync" errors before starting the charging and after the charging is completed.

Signed-off-by: Utsav Anand <utsavanand2@gmail.com>
@premultiply
Copy link
Member

I think there is a complete misunderstanding about enable() and enabled-State.

enabled is independent from car connection and charging state.

enable() will control if a chargepoint is allowed to charge a car (true) or if it has to stay in or transition to a passive, non-charging state (false).
Connection of a car anything else may never leave the passive state on its own.
To allow any transition to charging state again the charger needs to be re-enabled.

@utsavanand2
Copy link
Contributor Author

utsavanand2 commented Mar 11, 2022

@premultiply The "Charging" state is not an info we're getting from the connection of the car, but rather the state of the station. The implementation is for Enabled which returns a (bool, error) not Enable.

I have tested this fix and it has seemed to work with our station and with some of our users to get rid of errors when the connector is plugged into the car (Preparing) state and right after the charging is finished which is the (Finished) state according to the OCPP spec.

Also can you please explain how a charger can be re-enabled, correct me if I'm wrong but I guess re-enabling means a state in which the charge point can start charging again or a state in which the charge point is charging already?

@premultiply
Copy link
Member

premultiply commented Mar 11, 2022

Enabled is the reading for chargers internal state of Enable().

So if the charger is set into Enable(true) state Enabled has to return true until Enable(false) is called. Of course the charger is only allowed to do any charging activity if Enabled returns true.

To make it even clearer: Enable(false) shall prevent the charger from any PWM-activity on CP line. So any car is not allowed to charge.
Enable() may be called at any time regardless if a car is currently connected.

This has absolutly nothing to do with any charging state itself.
The IEC-Charging states (CP) are returned by Status.

@utsavanand2
Copy link
Contributor Author

utsavanand2 commented Mar 11, 2022

Okay understood @premultiply, so you're suggesting that the implementation of Enabled is incorrect in the first place. What would be the optimum implementation for daheimladen stations in this case then, I would really appreciate if you can provide some code suggestions here.

@premultiply
Copy link
Member

As I do not know anything about the API your product provides I would suggest that you may have a look on implementations for other brands chargers.

For example a quite popular way to disable a charger is to set the current limitation to 0, to enable it a value of more than 6 ampere must be set.
So in this case enabled returns false if the internal current limitation was set to 0, otherwise true.
Check the source code for Heidelberg Chargers for example to have a look in such an implemtation.
But there are even other ways like writing and reading 0 and 1 values to a register or modifying bit values in a flag register.

@utsavanand2
Copy link
Contributor Author

For our existing implementation of the API, we can remotely start/stop the charging of a charger with the API. Instead of checking if the current limit is set to 0, we have a status endpoint which tells us if the charging can be started or stopped. I am wondering how is this implementation going to be any different than the current limitation. I'll also check out other examples as you suggested.

@premultiply
Copy link
Member

An endpoint would be absolutly fine.
My point is why this should logicly depend on charging state?
But I hope my understanding of your api is correct…

@utsavanand2
Copy link
Contributor Author

Our API gives the state output as per the OCPP connector status which could be 'Available', 'Preparing', 'Charging', 'Finishing', and 'Faulted', out of the four states that the REST API would respond with, only when the station is in 'Preparing' state that the charger can start charging an EV, in other states it would refuse to start the charging.

@utsavanand2
Copy link
Contributor Author

cc @andig What do you think?

@andig andig added the enhancement New feature or request label Mar 11, 2022
@utsavanand2
Copy link
Contributor Author

utsavanand2 commented Mar 12, 2022

Hi @premultiply , I took a look into the PR which supports chargers which can communicate via OCPP. I see the Enabled is implemented as if it's TransactionID > 0 then it returns true. If I understand correctly, the transactionID is non zero only when there is a transaction going on and the charging station state is "Charging" which we get from our API.

func (c *OCPP) Enabled() (bool, error) {

@andig
Copy link
Member

andig commented Mar 14, 2022

@utsavanand2 I‘m not sure. The point is that enabled in evcc terms means that we allow the vehicle to charge, independent of if it actually does. Afaikt that is identical to starting a transaction in Ocpp. Insofar the current implementation is correct and this PR is not.

What bugs we is what happens in evcc if a transaction was started that doesn‘t come from evcc, maybe user per app. That SHOULD show enabled and if evcc choses it should be possible to disable. I think this is what you‘ve implemented using the GetCurrentTransaction.

imho the current implementation should work as it is. If not I do not yet understand the problem or ocpp good enough to match both.

@andig
Copy link
Member

andig commented Mar 14, 2022

In other words: when evcc disabled and a user plugs in. The station MUST remain disabled. It must only start a transaction or go to charging state when it is being enabled.

@andig
Copy link
Member

andig commented Mar 14, 2022

If you feel this PR does that we can just give it a try and see if it improves behaviour.

@andig andig added the needs decision Unsure if we should really do this label Mar 15, 2022
@andig
Copy link
Member

andig commented Mar 19, 2022

ping @utsavanand2 how do you want to proceed?

@utsavanand2
Copy link
Contributor Author

ping @utsavanand2 how do you want to proceed?

Hi @andig ! I'll be conducting a test once again with Stephan and I'll paste the logs here just to be very sure before we proceed forward. Is that okay with you?

@utsavanand2
Copy link
Contributor Author

@andig Thank you so much for the detailed explanation on the issue above. I have a follow up to the explanation though, if I have to check that evcc is disabled, why do I have to implement the Enabled function? How do I check if evcc itself is disabled?

In other words: when evcc disabled and a user plugs in. The station MUST remain disabled. It must only start a transaction or go to charging state when it is being enabled.

@andig
Copy link
Member

andig commented Mar 19, 2022

How do I check if evcc itself is disabled?

evcc tells you by calling Enable(false) on the charger in https://github.com/evcc-io/evcc/blob/master/charger/daheimladen.go#L71. At that moment, the charger MUST stop running transactions and MUST should not start any new transactions (e.g. because no sufficient power from PV or because user has selected off oder because vehicle may have reached target soc).

@utsavanand2
Copy link
Contributor Author

What bugs we is what happens in evcc if a transaction was started that doesn‘t come from evcc, maybe user per app. That SHOULD show enabled and if evcc choses it should be possible to disable. I think this is what you‘ve implemented using the GetCurrentTransaction.

@andig If I understand correctly what you mean is that if a user starts the charging from the app, then it should also get that state reflected in evcc, and if required it should be possible for evcc to disable it? If this is what you mean then yes this is what the current implementation provides.

@andig
Copy link
Member

andig commented Mar 19, 2022

If the user starts charging from the App while evcc is in PV mode and has disabled charging then that's a problem. How should two competing algorithms decide which one wins? User MUST decide which algorithm/app he uses. You cannot expect both to harmonise.

@utsavanand2
Copy link
Contributor Author

utsavanand2 commented Mar 19, 2022

If the user starts charging from the App while evcc is in PV mode and has disabled charging then that's a problem. How should two competing algorithms decide which one wins? User MUST decide which algorithm/app he uses. You cannot expect both to harmonise.

@andig Can you provide me with some test cases that we need to check, then I'll check all of those during our test run tomorrow and will work on this PR again if I can.

@andig
Copy link
Member

andig commented Mar 19, 2022

There is however a possible approach. We can (and actually had) an interface that allowed the charger to control the loadpoint. If charger becomes aware of user using vendor app he could update the loadpoint by switching loadpoint's mode, maybe from pv to off or pv to now.

We could re-add this interface if you want to give it a try.

@andig
Copy link
Member

andig commented Mar 19, 2022

@andig Can you provide me with some test cases that we need to check, then I'll check all of those during our test run tomorrow and will work on this PR again if I can.

Not really as I don't have a charger that has app control.

@utsavanand2
Copy link
Contributor Author

There is however a possible approach. We can (and actually had) an interface that allowed the charger to control the loadpoint. If charger becomes aware of user using vendor app he could update the loadpoint by switching loadpoint's mode, maybe from pv to off or pv to now.

We could re-add this interface if you want to give it a try.

Ahh okay, if you remember we do start all transactions from evcc with an idtag of "evcc", all other transactions started from the app or by other features do not use this idtag, this is one way we can tell if the charging was started by evcc or not.

@andig
Copy link
Member

andig commented Mar 19, 2022

You would still need to map non-evcc sessions to evcc operations. Session startet ist probably SetMode(api.ModeNow). What happens when session stops by itself or user? Keep the box in charging mode?

Anyway, not much I can contribute here. If you'd like to try that interface I can re-add is as part of this PR.

@andig andig changed the title Charging station is enabled only when in 'Charging' state Daheimladen: Charging station is enabled only when in 'Charging' state Mar 20, 2022
@utsavanand2
Copy link
Contributor Author

@andig If you would like we can close this PR for now, I conducted some tests with Stephan on the latest evcc release and everything seemed to work fine. If I get a better fix or I notice this problem pops up again, I'll reopen this PR.

@andig andig closed this Mar 22, 2022
@andig andig reopened this Apr 3, 2022
@andig andig merged commit 07676f7 into evcc-io:master Apr 3, 2022
dontbyte pushed a commit to dontbyte/evcc that referenced this pull request Aug 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request needs decision Unsure if we should really do this
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Daheimladen WB produces charger out of sync messages during start and termination of charge.
3 participants