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

fix #452 reevaluate claims if amps > pilot #532

Merged
merged 5 commits into from
Feb 6, 2023
Merged

Conversation

KipK
Copy link
Collaborator

@KipK KipK commented Jan 29, 2023

Until the bug is corrected on evse fw, this solves the issue and prevent the charge_current going to max when Evse goes from Disabled to Active state ignoring claims & pilot values
fix #452 and should also fix #513 fix #388 fix #481

Edit: It seems those bugs are related to PP_AUTO_AMPACITY beeing set. Posted an issue there: lincomatic/open_evse#156

So this is not really an OpenEvse bug, but more that OpenEvse and Wifi module don't share the non tethered chargers checking cable ampacity state and we have confusions here.

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

Actually I don't think getAmps is what you should be checking against, that is the live amp reading. What may be an idea is calling openevse.getCurrentCapacity after a state change/current change then doing the check to see if the pilot on the EVSE is the same as what the Monitor thinks it should be, so somewhere here, https://github.com/OpenEVSE/ESP32_WiFi_V4.x/blob/master/src/evse_monitor.cpp#L478, and here, https://github.com/OpenEVSE/ESP32_WiFi_V4.x/blob/master/src/evse_monitor.cpp#L280

@KipK
Copy link
Collaborator Author

KipK commented Feb 1, 2023

Do you think then we should reevaluate claim or just set the pilot to evse ?

@jeremypoulter
Copy link
Collaborator

I think just setting the pilot should be sufficient

@KipK
Copy link
Collaborator Author

KipK commented Feb 1, 2023

I have rewrote it on getChargeCurrentAndVoltageFromEvse()

Can't make the change using openevse.getCurrentCapacity , i'm lost with those callbacks.
But it works good as it, ( still the 2-3 sec glitch )
Can you merge for now as it solves important issues, and remove it later when you have time for a proper way of doing it?

@KipK
Copy link
Collaborator Author

KipK commented Feb 1, 2023

Ok I got it thanks.
I've made the change, gotta test first.

@KipK
Copy link
Collaborator Author

KipK commented Feb 1, 2023

Jiust tested but it doesn't work now. Amps stays at 32

@KipK
Copy link
Collaborator Author

KipK commented Feb 1, 2023

I have added some debug:

pilot = 32
getPilot() = 6
####  Pilot differents set again

So the check works ok, but setCurrentCapacity seems to not have effect here.

…r are the same, if not reset.

Temporary fix EmonEvse firmware and other compiled with PP_AUTO_AMPACITY where starting at full charge instead of configured one.
@KipK
Copy link
Collaborator Author

KipK commented Feb 2, 2023

Ok it works now. I've rebased the commits in one.
Ready to merge here

@KipK
Copy link
Collaborator Author

KipK commented Feb 2, 2023

After few tests, I can say it reacts faster than the claim approach I did first. I don't see spikes over the set limit and it stabilise quiet fast.

Until we got a proper integration of auto_ampacity between openevse fw and ESP32 one, this should fit.

src/evse_monitor.cpp Outdated Show resolved Hide resolved
src/evse_monitor.cpp Outdated Show resolved Hide resolved
src/evse_monitor.cpp Outdated Show resolved Hide resolved
src/evse_monitor.cpp Outdated Show resolved Hide resolved
@jeremypoulter jeremypoulter merged commit 1059f05 into OpenEVSE:master Feb 6, 2023
@KipK KipK deleted the #452 branch February 24, 2023 05:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants