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

do not set Scheduler claim if evse module communication is not ready #518

Merged
merged 1 commit into from
Jan 21, 2023

Conversation

KipK
Copy link
Collaborator

@KipK KipK commented Jan 16, 2023

fix #517

@jeremypoulter
Copy link
Collaborator

In all honesty you can just not use the value from the EVSE manager and just hard code the value to 80 or higher, will have the same effect.... Or actually set the default hardware max to 80. All will have the same effect. The claim engine will limit the value to the actual max.

@KipK
Copy link
Collaborator Author

KipK commented Jan 16, 2023

I displays the claims/target in a tab in the UI, to keep it clear for users I'd rather have ok values than hardcoded ones displayed.

This issue probably hide some others. I temporise on scheduler, but it's probably on the evseManager side that things should be handled isn't it ?

@KipK
Copy link
Collaborator Author

KipK commented Jan 16, 2023

@jeremypoulter I think we are close to solve many other issues as #452 , seeing my latest comment here #517 (comment)

If it's the case this PR has no reason te be merged and we should investigate on evse_manager

@jeremypoulter
Copy link
Collaborator

I think the correct way to solve this would befor the scheduler (and all the other modules) to wait for the evse to be active before sending the claims... but that being said the EVSE managershould also handle the claims at any point.

IMHO setting to a fixed value is not a problem, 80A is the maximum allowed by the spec so not just a random value, and after startup will get reset. potentially I could hide this in the web layer or EVSE layer. but for this issue specifically I think it is a better fix than polling until the value is read correctly.

I will also check on why 0 is returned under certain conditions, if that is just the default before the correct value is read then again 80 is as good as 0 (well probably better)

@jeremypoulter
Copy link
Collaborator

Yeah I think it is just a startup timing issue, I think setting the defailt valueto 80 would work:
https://github.com/OpenEVSE/ESP32_WiFi_V4.x/blob/master/src/evse_monitor.cpp#L164-L167 probably should be

  _min_current(6),
  _pilot(6),
  _max_configured_current(80),
  _max_hardware_current(80),

After the EVSE starts up the claims will be re-evaluated and the value will be clamped (or at least it should be)

@KipK
Copy link
Collaborator Author

KipK commented Jan 16, 2023

I think the correct way to solve this would befor the scheduler (and all the other modules) to wait for the evse to be active before sending the claims... but that being said the EVSE managershould also handle the claims at any point.

Yeah this would imply patching all the services, and deserve evseManager purpose no ?
I'd rather go for fixing evsemanager evaluateClaim and let it handle that, and hope it fixes some other trailing issues.

After the EVSE starts up the claims will be re-evaluated and the value will be clamped (or at least it should be)
Edit: I've tested changing the default value to 80 and yes it then reevaluate and goes to 32 .
PR updated

@jeremypoulter jeremypoulter merged commit 3c82b26 into OpenEVSE:master Jan 21, 2023
@KipK
Copy link
Collaborator Author

KipK commented Jan 22, 2023

There's still something strange, I can see the claim not being reevaluated sometime. It stays at 80.

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.

Timer set charge_current claim with 0 amp
2 participants