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

Don't stop solar charging if it leads to immediate restart #70

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

Conversation

TFleury
Copy link

@TFleury TFleury commented Dec 2, 2024

Avoid start / stop cycles when using solar charging.

@dingo35
Copy link
Collaborator

dingo35 commented Dec 5, 2024

I am trying to find a test case that this PR solves....

So my settings are StartCurrent = -4A and ImportCurrent = 0A, StopTimer = 1min; so now I am simulating:
MainsMeter = 3x2A: nothing happens since StartCurrent has not been met;
MainsMeter = 3x-2A: EV starts charging, and the charge current increases as there seems to be more then enough current
MainsMeter = 3x1A: now charge current decreases to 6A, and then the stoptimer starts running, since we are importing more then we are supposed to;
MainsMeter = 3x-1A: the stoptimer stops running since we have enough power;

So in what scenario do you experience "immediate restarts" after solar charging stops?

@bobosch
Copy link
Collaborator

bobosch commented Dec 5, 2024

Set StartCurrent to -1A
Car starts charging as soon as solar exports. Car draws 6 A, so new import is 5 A - Car stops charging without this patch, again -1 A is available, car starts charging...

@bobosch
Copy link
Collaborator

bobosch commented Dec 5, 2024

It depends on your definition of "immediate". In this case it means 1 minute.

@dingo35
Copy link
Collaborator

dingo35 commented Dec 5, 2024

Ok, so this is your proposed change:

  •            if (ActiveEVSE && StopTime && (IsumImport > 0)) {
    

to

  •           if (ActiveEVSE && StopTime && IsumImport > 0 && Isum > ((uint16_t)MinCurrent - (uint16_t)StartCurrent) * 10) {
    

The formula you propose as an extra condition to block charging doesnt take into account the number of charging ActiveEVSEs; this would make the line:

  •           if (ActiveEVSE && StopTime && IsumImport > 0 && Isum > (ActiveEVSE * MinCurrent - StartCurrent) * 10) {
    
    (no need to cast uint16_t, let the compiler do its work)

Now experience learns these kind of changes introduce new bugs because everybody looks at it from his own configuration; are we missing any other configuration dependencies?

@dingo35
Copy link
Collaborator

dingo35 commented Dec 5, 2024

Ok another one: this formula assumes 1 phase charging, to also incorporate 3 phase charging it should read:

  •      if (ActiveEVSE && StopTime && IsumImport > 0 && Isum > (ActiveEVSE * MinCurrent * Set_Nr_Of_Phases_Charging() - StartCurrent) * 10) {
    

@bobosch
Copy link
Collaborator

bobosch commented Dec 5, 2024

That is the reason why I make StartCurrent configurable per phase in V2 SmartEVSE/SmartEVSE-2@6da500b
It makes no senso to use the sum current for StartCurrent, because every car would act different

@bobosch
Copy link
Collaborator

bobosch commented Dec 5, 2024

Ok another one: this formula assumes 1 phase charging, to also incorporate 3 phase charging it should read:

  •      if (ActiveEVSE && StopTime && IsumImport > 0 && Isum > (ActiveEVSE * MinCurrent * Set_Nr_Of_Phases_Charging() - StartCurrent) * 10) {
    

The StartCurrent should be also multiplied by the number of charging phases, My version in V2 is
(Isum > ((signed int)SumPhases * ((signed int)MinCurrent + (signed int)StartCurrent) * 10))
https://github.com/SmartEVSE/SmartEVSE-2/blob/924fa01bf457b7a153bba64c6422cab11725850a/SmartEVSE2.X/EVSE.c#L997C20-L997C111
StartCurrent is a signed int and negative

@dingo35
Copy link
Collaborator

dingo35 commented Dec 5, 2024

AFAIK Startcurrent was always implemented as the sum of all phases; it is even defined in the documentation:

START set the current on which the EV should start Solar charging:
-0 -48A (sum of all phases)

@dingo35
Copy link
Collaborator

dingo35 commented Dec 5, 2024

"It makes no senso to use the sum current for StartCurrent, because every car would act different" ;
Its not about the car, its about the grid (usage).

IMHO, Solar Charging is all about how much you draw from the grid, and how much you deliver to the grid. AFAIK all P1-meters and all electric providers measure and calculate over the sum of all phases.
Also, in the original v3 firmware Startcurrent was compared to Isum, so implicitly defined also as the sum of all phases.
Why would you break backwards compatibility ?

@bobosch
Copy link
Collaborator

bobosch commented Dec 5, 2024

The original comment for StartCurrent in evse.h is // Start charging when surplus current on one phase exceeds 4A (Solar)
It was changed while implementing because the number of phases was not known in previous versions.
How much is drawn from the grid is configured with ImportCurrent (sum of phases)

@dingo35
Copy link
Collaborator

dingo35 commented Dec 5, 2024

Configuring the number of phases is not a good idea; on a 3phase wired EVSE it depends on the EV that is connecting, and even if you would know the capabilities of the EV, it decides dynamically what phases it uses and how much of the advertised current it is actually taking.

E.g. some Tesla's start single phase and after a minute or so decide to add the other two phases.

In the v3 code we try to derive the nr of phases from the settings, and if that is not possible, measure it through the EVMeter, if present.
Its not ideal but IMHO the best you can do...

@bobosch
Copy link
Collaborator

bobosch commented Dec 6, 2024

Yes, the number of used phases is detected automatically in V2 branch, either with EV electric meter measurement after 40 seconds of charging, or with alternating between 6 A and 8 A charging current multiple times and mains measurement.
SmartEVSE/SmartEVSE-2@8799f83
SmartEVSE/SmartEVSE-2@66026dd

I have a Tesla (M3), it uses all phases immediatly. But nevertheless when the detection fails (e.g. detecting one phase instead of three) it is the same behaviour as without this patch: Charging until StopTimer ends, wait one minute and charge again...

@dingo35
Copy link
Collaborator

dingo35 commented Dec 6, 2024

Ok I, is this the same piece of detection code that was in the v3 dev branch in December 2023, or is that some other development?

" StartCurrent is a signed int and negative" ; we are in the v3 repo here, so lets not confuse v2 variables with v3:
uint16_t StartCurrent = START_CURRENT;
.... and thus it is a positive value, although in the display a - sign is displayed before it....

Back to the original PR: lets agree to disagree on the definition of StartCurrent; I committed the change:
94ca08ef4b.zip

@bobosch and all others in this thread, could you test please?

@TFleury
Copy link
Author

TFleury commented Dec 6, 2024

I tested the PR for few days before submitting and it works as expected.
I only have single phased setup and single phase EV (Nissan Leaf).

@dingo35
Copy link
Collaborator

dingo35 commented Dec 6, 2024

@TFleury Thx for your input, appreciated!

Now waiting for the guys with multiple 3 phase chargers connected :-)

@bobosch
Copy link
Collaborator

bobosch commented Dec 7, 2024

Sorry, can't test, I have only V2 running here :-)

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