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

feat: add max_current sensor #385

Merged
merged 4 commits into from
Oct 11, 2024
Merged

feat: add max_current sensor #385

merged 4 commits into from
Oct 11, 2024

Conversation

firstof9
Copy link
Owner

fixes #384

Provides max_current sensor.
Change old "max current" selector to Charge Rate to better reflect it's use.

@jstebbins
Copy link

Hi, me again 😁

I think the value you are using here for max current is still not quite providing what I would like. You are returning the max_current from the get status API:
https://openevse.stoplight.io/docs/openevse-wifi-v4/dba95e8966593-get-the-evse-status

The documentation says this value is "the actual max current value defined either by max_current_soft or a running claim". So it may not be the same as MAX CURRENT in OpenEVSE settings. I think you need to return max_current_soft from the get config API.

Copy link

codecov bot commented Oct 11, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Please upload report for BASE (main@ae4d7cb). Learn more about missing BASE report.
Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #385   +/-   ##
=======================================
  Coverage        ?   83.91%           
=======================================
  Files           ?       12           
  Lines           ?      945           
  Branches        ?        0           
=======================================
  Hits            ?      793           
  Misses          ?      152           
  Partials        ?        0           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@firstof9
Copy link
Owner Author

The documentation says this value is "the actual max current value defined either by max_current_soft or a running claim". So it may not be the same as MAX CURRENT in OpenEVSE settings. I think you need to return max_current_soft from the get config API.

They are the same value, I've verified this on my device. When I adjust the max current both values change.

@jstebbins
Copy link

They are the same value, I've verified this on my device. When I adjust the max current both values change.

But if you were to issue a post claim API call with a max_current value that is different than the MAX CURRENT config setting, my contention is that the values would then not be the same. See:
https://openevse.stoplight.io/docs/openevse-wifi-v4/ebc578ffa7ca7-make-update-an-evse-claim

@firstof9
Copy link
Owner Author

I did issue a claim, max_current didn't change only the "charge rate" changed.

@jstebbins
Copy link

I did issue a claim, max_current didn't change only the "charge rate" changed.

Interesting. The OpenEVSE documentation is certainly vague, and I believe in this case flat out wrong then. Empirical testing FTW.

Thanks!

@firstof9
Copy link
Owner Author

I also verified adjusting the selector changes the "Charge Rate" as it's labeled in the UI without it adjusting the "Max Current" 🙂

@firstof9 firstof9 merged commit 11b1732 into main Oct 11, 2024
9 checks passed
@firstof9 firstof9 deleted the fix-384 branch October 11, 2024 15:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature Request]: Provide OpenEVSE Max Current configuration value (i.e. max_current_soft)
2 participants