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

New InputFilter class #590

Merged
merged 37 commits into from
May 21, 2023
Merged

New InputFilter class #590

merged 37 commits into from
May 21, 2023

Conversation

KipK
Copy link
Collaborator

@KipK KipK commented Apr 1, 2023

Following #587 issue + discussion
Changed for a new generic inputFilter class not dependant of fixed sample frequency.
This will calculate automatically previous filter ratios depending of data submission delta ( thanks to @pdhoogh for taking time to explain what looked like dark magic to me )
Should probably fix some of the issues posted with graphic showing unprevisible divert results

shaper ok
Divert ok
UI2 gui ok (shaper branch)
UI1 gui ok (OpenEVSE/openevse_wifi_gui#98)

@soloam
Copy link

soloam commented Apr 1, 2023

This is great! Thank you! When will it hit a release? Thank you

@KipK KipK force-pushed the shaper branch 2 times, most recently from de02af9 to 1fcd5d8 Compare April 1, 2023 17:01
@jeremypoulter
Copy link
Collaborator

I think the reason for this change got lost in between the other conversations, could you clarify the use case? I am not totally sure this is an appropriate change. To my mind the primary use case for the shaper is for things like load balancing and controlling the maximum current that can be delivered.

It is much more of a safty feature and should be fast to react, both in terms of switching the current down and back up again. That is the reason we created this as a separate module rather than trying to make the divert fit that use case.

At a minimum, I think to accept this PR it would need to be configurable and the default to be off, but waiting to hear the input on the use case.

@soloam
Copy link

soloam commented Apr 1, 2023

Like discussed, the main use case for this is to ensure that a safety feature is not responsible for a fast wear out of the relay! Attenuating the number of times that the relay goes on and off when the power consumption is going up and down! Like stated, a low pass filter would help, only when the relay is off and need to go on, because when the power is lowering, the ideia is that is instant! It's preferable that the power goes down and only some minutes latter goes up... Than waiting or delaying lowering the power and risk a power cut!

Tks for all the time and attention dedicated to this issue

@jeremypoulter
Copy link
Collaborator

Then IMHO the solution is to add the smoothing on activation as with the divert, so off instantly then apply the smothing to the recovery. I don't even think we should apply any filtering to the reduction of the current at all, even optional.

@jeremypoulter
Copy link
Collaborator

That being said is it not reasonable to expect that the EVSE should be installed in situations where it is gauranteed at least 6A? Hense the Shapper having to turn off the current should be an exception should it not?

@soloam
Copy link

soloam commented Apr 1, 2023

In my personal situation I get a lot turn off while charging and continuing to use the house to other activities! And I have 32 Amps... A lot of people here in my country don't go over 20! In that situations the shaper is a must have! Ensuring that while the house is consuming more that the charger is lower or off... And when late at night that most of the house is idle, the charger can go full power!

@jeremypoulter
Copy link
Collaborator

Ok point accepted, 32A for the entier property is fairly easy to use up if that is typical in some locations. I very much think that the correct solution to this issue is a configurable smoothing on the attack. I have never found a case yet where an arbitory time is the correct solution to a problem.

@soloam
Copy link

soloam commented Apr 1, 2023

At this point I'm good with either solution... If that results on my relay not going on and off several times because of momentary spikes or lows!

@KipK
Copy link
Collaborator Author

KipK commented Apr 1, 2023

I think the reason for this change got lost in between the other conversations, could you clarify the use case? I am not totally sure this is an appropriate change. To my mind the primary use case for the shaper is for things like load balancing and controlling the maximum current that can be delivered.

It is much more of a safty feature and should be fast to react, both in terms of switching the current down and back up again. That is the reason we created this as a separate module rather than trying to make the divert fit that use case.

At a minimum, I think to accept this PR it would need to be configurable and the default to be off, but waiting to hear the input on the use case.

It already goes to disable immediatly. It just takes 5mn to start again or more if needed until it's stable enough to start.
This is temporary to have at least this for the current prerelease until I find time to do a proper smoothing out and make it configurable.
edit: now filtered out.

I also have only 39A available and can override easily with my eletric heaters in winter. But it never got to the point of beeing under 6A , so couldn't test it

@KipK
Copy link
Collaborator Author

KipK commented Apr 2, 2023

@jeremypoulter , thinking of future shaper rewrite to prepare it for handling thermal throttling too and beeing more configurable.
Do you think I should move shaper class to evseManager like I did for energyMeter ?

edit: I'm doing some test now, and I see an important point. Live power data are not updated in a regular interval.
Main counter on my side can shoot data update immediately when there's change, but can also send an update after one minutes ( could be more I've set it to 1mn ) if power have not changed more than 50w.
So the smoothing algorithm used in divert won't work in this case. We need to integrate the time interval inthe calculation isn't it ?
What would be the best way doing this ? ( also @pdhoogh ? some math to share ? )

edit: don't merge yet, I have almost done doing the filtering and configurable part. Just have to test

@KipK
Copy link
Collaborator Author

KipK commented Apr 2, 2023

I did this to take update interval in the loop:

`

uint32_t updt_time = (millis() - _timer) / 1000;

// default filtering would consider an update each 10sec. 
// Calculating the smoothing factor considering current interval.
double factor = (updt_time * current_shaper_smoothing_factor) / 10.0;

_smoothed_live_pwr = (_live_pwr * factor) + (_smoothed_live_pwr * (1 - factor));`

@jeremypoulter
Copy link
Collaborator

I wouldn't do either, the thermal throttling should just be a separate module and both should create claims so can live outside of the EVSE manager and call it

@pdhoogh
Copy link

pdhoogh commented Apr 2, 2023

The math should be the same as already discussed elsewhere. The module needs to know the update interval of the power data in minutes and the time constant then needs to be a configurable value also expressed in minutes. Zero means no filter (immediate action). Every time the module wants to update the current setpoint it should get the last update interval in minutes and use this formula to come up with the filter factor:
image
DeltaT is the update interval in minutes. Tau is the time constant of the exponential filter. From that formula it is clear a divide by zero needs to be avoided, so tau = zero must lead to a filter factor of 1 (no filter) and skip the computation of the formula above. A practical low limit for tau like 0.1 is a good value to avoid very large numbers generated by the division operation needed for the exponential. So the data validation should allow zero for tau, but values between zero and 0.1 should result in using 0.1. A time constant smaller than 6 seconds (0.1 minutes) does not make much sense with a sample rate of ten seconds or more (Shannon et. al.)

The result alpha is the filter factor used by the OpenEVSE today. So the implementation of the filter does not need to change. Only that the filter factor is now calculated instead of entered directly.

I have not yet tried the @jeremypoulter idea of using the attack and decay filter factors yet because the weather here is so bad my PV installation barely produces enough power to cover my home's low power state:
image
So unless we go to LIDL for groceries where I can charge the EV for free, we're back to using the Diesel Merc as long as the weather stays like this. Home import power is too expensive here for charging the EV, diesel is cheaper.

From @jeremypoulter simulation graphs I now think I better understand how the filters work and if my understanding is correct, I believe what @jeremypoulter proposes, using the attack/decay, is close to what is requested:

  • The actual amp setpoint calculated from the +I/-E is unfiltered and aims to balance power immediately
  • The Amps available is filtered using attack / decay exponential low pass filters
  • The filtered Amps available is compared to the 6 amps minimum level to decide if the OpenEVSE is enabled or Disabled
  • There is still the possibility for a chattering relay if the filtered Amps available hovers around 6 Amps. Therefore a configurable minimum charge time is implemented. That minimizes "wasting PV power". And allows sufficient time for the EV to complete its charge startup procedure before cutting out the power again, this otherwise possibly leading to the EV going in "Charge Error" mode.

Elsewhere on this site we have a discussion where I request to add a configurable minimum disable time. So we adapt to new legislation and tariff structures. Or avoid using more power than the home installation allows. This should not be hard, as it almost exists today. There is a random timer for enabling the OpenEVSE to implement some kind of reacceleration scheme, avoiding all OpenEVSE's to enable at the same time. The configurable minimum disable time can be just the minimum value used in the random number generator.

Is my understanding correct @jeremypoulter ?

Based on the above understanding I have the attack filter factor set to 0.033 (5 minutes time constant) and the decay filter factor set to 0.5 (about 15 seconds time constant), allowing for some debouncing while high power duration is too short to trip safeties or get registered as a 15 minute sustained peak.

@KipK
Copy link
Collaborator Author

KipK commented Apr 2, 2023

Do we really need to let the Tau beeing configurable ? ( or just set in #define , i use 10 sec as time reference interval )

@pdhoogh
Copy link

pdhoogh commented Apr 2, 2023

Yes, Tau needs to be configurable.

I will try to clarify. First recapping above:

Every time the module wants to update the current setpoint it should get the last update interval in minutes and use this formula to come up with the filter factor:
image
DeltaT is the update interval in minutes. Tau is the time constant of the exponential filter. From that formula it is clear a divide by zero needs to be avoided, so tau = zero must lead to a filter factor of 1 (no filter) and skip the computation of the formula above.

Tau is the exponential filter constant in minutes. This is essentially the same intended functionality as the filter factors that are configurable now for attack and decay. Only the new function for these configurable parameters would become the filter time constants in minutes rather than the filter factors straight.

In the time domain, the effect of the filter using only the filter factor as @jeremypoulter described depends on the rate at which the calculation is executed. This can easily be understood as follows: in the current implementation, when the filter calc is run, the new filtered value for a filter factor of 0.1, is calculated as 90% of the previous filtered value + 10% of the current unfiltered value. Every time this runs, the filtered value is updated accordingly. So one can see that the filtered value will move twice as fast in the time domain if the calculation is done at half the period. E.g. if you calc this every five seconds, the filtered value will move twice as fast as if the calc is done every ten seconds. So the behavior in the time domain also depends on the calc period.

To transform the response of the filtered value to a fixed behavior in the time domain, the filter factor needs to be compensated for the calculation period. E.g. the filter factor needs to be changed so that the effect of a 10% share in ten seconds is the same as two x% shares in 2 times 5 seconds.

So yes @KipK you could fix DeltaT to 10 seconds (0.166 minutes), but that assumes the update frequency of the data comes in exactly every ten seconds. And as you described before, that is not always the case. There is a timer that keeps track of the MQTT update period already. You can use that value just before the last update, in minutes, for the deltaT.

The exponential function is needed for accuracy with the smaller time constants, as the simplified expression is an approximation that is only valid for tau much larger than DeltaT. Below is a graph showing the exponential and the approximation
image
It works well for a time constant of 1 minute or longer
image
But for faster filters, that deteriorates and that may be the practical range for at least the decay value
image

@KipK
Copy link
Collaborator Author

KipK commented Apr 2, 2023

@pdhoogh

So yes @KipK you could fix DeltaT to 10 seconds (0.166 minutes), but that assumes the update frequency of the data comes in exactly every ten seconds.

in fact I did it a bit differently.
The 10 sec ( EVSE_SHAPER_FILTER_TAU ) is just a constant for the calculation, but then I change the smoothing factor proportional to the time difference with EVSE_SHAPER_FILTER_TAU
I've tested it and seems to works good like this
I can spam data in short intervals or lagging data, it doesn't change the way it reacts

Any cons ?

#define EVSE_SHAPER_FILTER_TAU 10 // 10 sec

double factor = (updt_time * current_shaper_smoothing_factor) / EVSE_SHAPER_FILTER_TAU;
_smoothed_live_pwr = (_live_pwr * factor) + (_smoothed_live_pwr * (1 - factor));

This should be done ( or @pdhoogh formula ) for Divert too. For now Divert code depends too of a fixed data time interval

@KipK
Copy link
Collaborator Author

KipK commented Apr 2, 2023

@jeremypoulter I've rebased the PR with filtering and configurable UI is ok on shaper branch too.
Tested for what I could, seems all ok.

we now can just use timer based pause if the filter ratio is set to 1 using current_shaper_min_pause_time,
or use filtering + current_shaper_min_pause_time just in case the filter has wrong settings.

@pdhoogh
Copy link

pdhoogh commented Apr 2, 2023

@pdhoogh

So yes @KipK you could fix DeltaT to 10 seconds (0.166 minutes), but that assumes the update frequency of the data comes in exactly every ten seconds.

in fact I did it a bit differently. The 10 sec ( EVSE_SHAPER_FILTER_TAU ) is just a constant for the calculation, but then I change the smoothing factor proportional to the time difference with EVSE_SHAPER_FILTER_TAU I've tested it and seems to works good like this I can spam data in short intervals or lagging data, it doesn't change the way it reacts

Do I see correctly that you switched the variable names and see tau as the calc period, not the filter time constant?

@KipK
Copy link
Collaborator Author

KipK commented Apr 2, 2023

no EVSE_SHAPER_FILTER_TAU is a constant of 10 sec ( ideal world update refresh )

current_shaper_smoothing_factor is configurable over ui, default set to 0.03
min 0.01
1 would mean it doesn't filter.

"updt_time" is the real time we got between the last 2 update

And there's also a current_shaper_min_pause_time set in the config, to add a minimal pause.

@KipK
Copy link
Collaborator Author

KipK commented Apr 2, 2023

this is how it looks on ui

image

@jeremypoulter
Copy link
Collaborator

Was thinking about this a bit, normally you would have a higher on value than off to stop this, eg off at 6A on at 6.5A. might be worth a try for both the shaper and the divert. Probably should think about splitting out the filtering/set point code into a common class...

@KipK
Copy link
Collaborator Author

KipK commented Apr 2, 2023

taking time to read @pdhoogh filtering explanations above ( quiet interesting thanks for that )

I'm trying to implement that now

src/divert.cpp Show resolved Hide resolved
src/app_config.cpp Outdated Show resolved Hide resolved
@jeremypoulter
Copy link
Collaborator

The observed behavior with "Required PV power ratio" = 0.6 is that it initially works as I expect, but when the available current goes above 6 Amps and later drops back to e.g. 4 Amps, the charger switches to 16 Amps (max current). That was very painful during my test, as the drop in available current was due to solar dropping off and coincidentally the swimming pool starting its cleaning cycle and gobble up 4600 Watts... the charger obliged with adding 3700 Watts, so I was importing 8000... OUCH!

The PV ration needs looking at, it is controlling a few different things that probably should be split up, but when less than or equal to 1 it does two things:

  1. changes the trigger current (the point at which the charge is enabled) to apercentage of the min current (6A). so for example with a value of 0.5 the trigger current will be 3A and the charge will be enabled the the smoothed current is greater than that

with the default
image
with a PV ratio of 0.5
image

  1. changes the point at which the charge current rounds to the next level with 1 being very conservative and not jumping to the next level untill there is sufficiant PV current to satisfy it and lower values jumping earlier at the expense of more grid import

PV ratio of 1
image

PV ratio 0.5
image

Then for values > 1 and < 2 it controls the amount of grid import power to leave in 'reserve'. with a value of 1.1 (the default) being 100w, 1.5 being 500w.

IMHO I think these three functions should be split out into separate config values as I don't think they are always related

@KipK
Copy link
Collaborator Author

KipK commented May 17, 2023

IMHO I think these three functions should be split out into separate config values as I don't think they are always related

damn, we were trying to simplify the setup, and will end with more parameters than before :)

@KipK
Copy link
Collaborator Author

KipK commented May 18, 2023

Here are the presettings I propose then :

presets = [
		{
			name: "Default",
			desc: "Slowly decrease charge rate, using grid to compensate, but increase faster when energy is going back.",
			id: 0,
			divert_attack_smoothing_time: 20,
			divert_decay_smoothing_time: 600,
			divert_min_charge_time: 600,
			divert_PV_ratio: 1.1
		},
		{
			name: "No waste",
			desc: "No waste of produced energy. Same as default but will import more from the grid to not waste any solar energy",
			id: 1,
			divert_attack_smoothing_time: 20,
			divert_decay_smoothing_time: 600,
			divert_min_charge_time: 600,
			divert_PV_ratio: 0.5
		},
		{
			name: "No import",
			desc: "Try to limit grid usage. Will slow down the charge rate quickly, but increase slower when energy is going back.",
			id: 2,
			divert_attack_smoothing_time: 300,
			divert_decay_smoothing_time: 20,
			divert_min_charge_time: 600,
			divert_PV_ratio: 1.1
		}
    ]

@pdhoogh
Copy link

pdhoogh commented May 18, 2023

I don't need more presets. The intention is now clear to me and I'm fine with it.

@jeremypoulter
Copy link
Collaborator

Here are the presettings I propose then :

@KipK I think those will work. Can you update the expected test results then I think this is good to go.

@KipK
Copy link
Collaborator Author

KipK commented May 20, 2023

I'll try to find some time tomorrow to update those.

@jeremypoulter
Copy link
Collaborator

Maybe we could write a script to generate those tests....

@KipK
Copy link
Collaborator Author

KipK commented May 21, 2023

Maybe we could write a script to generate those tests....

yes if you can, I see there's a lot of data to update manually on this file.

@KipK
Copy link
Collaborator Author

KipK commented May 21, 2023

we're good to go.

jeremypoulter
jeremypoulter previously approved these changes May 21, 2023
@jeremypoulter
Copy link
Collaborator

Good to go when the UI changes are merged

@KipK
Copy link
Collaborator Author

KipK commented May 21, 2023

Good to go when the UI changes are merged

@jeremypoulter I've merged inputFilter changes in master gui, we're ready to go on this one. I'll rebase newflags thereafter.

@KipK KipK requested a review from jeremypoulter May 21, 2023 19:45
@jeremypoulter jeremypoulter merged commit 8ce9db4 into OpenEVSE:master May 21, 2023
@jeremypoulter
Copy link
Collaborator

Thanks @KipK and @pdhoogh

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

5 participants