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

HA Integration use of max_current vs charge_current #120

Closed
fhteagle opened this issue Jan 22, 2023 · 43 comments
Closed

HA Integration use of max_current vs charge_current #120

fhteagle opened this issue Jan 22, 2023 · 43 comments

Comments

@fhteagle
Copy link

fhteagle commented Jan 22, 2023

Hi @firstof9 -

First off, thanks for this integration!

I am helping test the new GUI developed by @KipK and @jeremypoulter, which is in release candidate status now. Also starting to explore HA and whatnot as well, but still very much new to that realm. However, it looks like some of the (API?) changes that were made to enable the GUI v2 are breaking your integration. I do not have a full breakage list just yet, but some of the big ones I have noticed:

  • Manual Override works differently
  • "Max Current" variable as a safety limit vs a soft max current for manual reduction
  • Your integration puts the unit into sleep when I do not think it should, etc.
  • There is an on/off toggle for the area I assigned my OpenEVSE, which toggles manual override and sleep on in reverse of what I expect the area toggle to do?!?

I have purposefully put on blinders as to the API system and changes (trying to test from the perspective of an ignorant user), so I cannot be more accurate as to what has changed than that. But I know enough has changed that updates to your integration are needed. I will try to get more precise with the bug reports here, and help test in the future also.

Thanks!

@fhteagle fhteagle changed the title HA Integration issues with new Wifi GUI V2 HA Integration issues with new OpenEVSE GUI V2 Jan 22, 2023
@fhteagle
Copy link
Author

fhteagle commented Jan 22, 2023

@jeremypoulter
Copy link

I don't believe there are any API changes as we have tried to keep that the same for the V1 UI, what maybe different in how a top level use case is implemented.

@firstof9
Copy link
Owner

If there's no API changes, and the websocket, status, and config endpoints dictionary keys haven't changed the integration should still work as intended.

@fhteagle
Copy link
Author

fhteagle commented Jan 29, 2023

I did some digging into the HTTP API docs, and playing with command line curl POSTS, and manually monitoring http://openevse.local/ endpoints outputs, and manually adjusting HA controls. I am using core 2023.1.6 and openevse HA integration 2.1.7 installed through hacs. OpenEVSE is 7.1.3 and wifi firmware 4.1.7 nightly build from https://github.com/OpenEVSE/ESP32_WiFi_V4.x/releases/tag/v2_gui . All automations have been disabled for testing purposes. I am going to go through all my findings in pretty deep detail first to check my understanding, and if that is correct I think I have a solution to make the OpenEVSE GUI and HA integration stop fighting against each other.

If I am reading this correctly: https://openevse.stoplight.io/docs/openevse-wifi-v4/ebc578ffa7ca7-make-update-an-evse-claim , the right json property to write to ask for less amps than then unit is physically capable of is "charge_current". When I curl post setting this property, the main tab of the GUI v2 correctly updates as expected, /claims endpoint updates "charge_current" in the client:0 (priority:500) claim line, and /status endpoint "pilot" value updates.

There is an HA integration sensor.openevse_charging_current which seems to match the value of the /status endpoint property "charge_rate" instead.

However, if I manually change the value of select.openevse_max_current in the HA dashboard, /claims endpoint is not updated at all, /status endpoint "pilot" value gets updated, and /config endpoint "max_current_soft" gets changed as well. This is also a safety concern, as a computation error or bad selection by ignorant user can now set the physical unit to advertise to the EV that more amps are available than the physical wiring can safely carry. A proper install will have a physical circuit breaker to also prevent this, but I think removing one layer of safety for no benefit is not smart.

So, here are my suggestions:

  1. Change the HA integration to NOT write to max_current_soft anymore
  2. Convert select.openevse_max_current into sensor.openevse_max_current_soft
  3. Add new select.openevse_claim_charge_current that posts to the /claims/client endpoint
  4. Limit select.openevse_claim_charge_current select list to only values less than sensor.openevse_max_current_soft.

Hope this is all clear enough. If I have misunderstood something, please feel free to let me know @jeremypoulter , @KipK, or @firstof9 . Looking forward to having this more clear for end users soon.

Thanks.

@KipK
Copy link

KipK commented Jan 30, 2023

I can help to document a bit

All controls on main page use /override endpoints instead of /claims ( in sort of way it's the same as override are claims too ) but it should be simpler to parse on client side.

Clicking on the auto mode will delete any "state" property in /override if there's one.
Auto button is hidden if there's no schedule planned or other service running that can control evse state ( Can be OCPP, RFID, Divert, ... )
Clicking on Activate / Disable will either set a "state" /override.
Setting charge current slider set "charge_current" prop.
A "manual" tag will display under the slider. Setting the slider to max or clicking the delete button on the tag will delete the override property.

if a /claim is made on "charge_current" by another service , it also displays on the tag under the slider ( if there's no /override on charge_current already, or if it has higher priority ) . And the slider set itself to the related value.
If it has a priority < OpenEVSE_Client_Override , the claim can be removed from the UI ( the tag has a delete button if needed) . This is mainly used for mqtt claims that has equal priority than http ones.

Same for states on the bottom left in the status bar, it display the related service that has the priority on the state claim
To achieve that, and the tag on the slider, it use a really helpfull undocumented endpoint /claims/target
You can get quickly what ClientID is setting each property currently set ( without boilerplate code on parsing the /claims )

It's also used on the Monitoring / Commands tab to display all current targeted properties.

@fhteagle
Copy link
Author

@KipK thanks for the additional info

I just tried out the endpoints you referred to. All I can get is the "manual" tag on the GUI. I think this could be confusing for users if HomeAssistant automations are setting the charge_current value. Is there a way to set any other tag verbiage besides "manual" on the GUI? As /claims/target is undocumented, not sure if that could set a verbal tag. Setting an integer clientID is not really explanatory. Is this something that could be / should be added to OpenEVSE GUI v2 perhaps?

@KipK
Copy link

KipK commented Jan 30, 2023

ClientID are hard coded on Openevse fw. Those "integers" can be retrieved in evseman.h :

https://github.com/OpenEVSE/ESP32_WiFi_V4.x/blob/751d8374ab072df83a920295b6723dc8e904d21e/src/evse_man.h#L28

I have them defined here for the gui:
https://github.com/KipK/openevse-gui-v2/blob/master/src/lib/vars.js

They are dedicated to internal services.
If HA use /override endpoint it's then Manual clientID. If it use a claim using http api,, it can be one of those

@firstof9
Copy link
Owner

The select.openevse_max_current mimics the amperage selector on the UI. It's limit is derived from the max_current_hard value from the device.

@KipK
Copy link

KipK commented Jan 30, 2023

@firstof9 it is not the case anymore, incoming new UI use behavior explained above.

@firstof9
Copy link
Owner

Will the max_current_soft key will be no more then?

@KipK
Copy link

KipK commented Jan 30, 2023

it is but it's from configuration and shouldn't be used for setting current for charge sessions as it writes to the eeprom each time.
On a UX side , having the selector on a charge session screen would means more having to set the current for this session. This has more sense to be set as an override or claim ( you still can set auto_release prop if you want it to be kept when car unplugued ).
The only remains, is those settings don't survive reboot. You can handle that on the smart home side btw.
Also it should set charge_current, max_current is a security prop, and is used by the Current Shaper.
If you have a Settings screen for this plugin ( I don't use HA here, i don't know ) then it's the correct place to set the max_current_soft ( as you won't change it oftenly )

@firstof9
Copy link
Owner

I see, we don't want to do automation off of something that hits the EEPROM, I'll re-tool a few things in the integration to indicate that the selector is a configuration parameter and not something to be adjusted regularly.

Thanks for clearing that up.

@fhteagle
Copy link
Author

fhteagle commented Jan 30, 2023

@KipK thanks for the info about claim tag. Opened OpenEVSE/openevse_esp32_firmware#536

@firstof9 Just to clarify, are you going to add an additional selector that posts to /claims to set charge_current after a manual or automated select?

@fhteagle fhteagle changed the title HA Integration issues with new OpenEVSE GUI V2 HA Integration sets max_current instead of charge_current Jan 30, 2023
@firstof9
Copy link
Owner

are you going to add an additional selector that posts to /claims to set charge_current after a manual or automated select?

No, I'll move the selector to a configuration entity type and likely put some examples in the README on how to use the set_override service call to adjust the amperage they want to limit the charger to.

I haven't done any work with the claims endpoint at this time.

@jeremypoulter
Copy link

In an ideal world user input should use the /override endpoint and automations should use the /claim endpoint but I am not sure how you would make this distinction in Home Assistant.

@firstof9
Copy link
Owner

What would be the difference between an override vs claim to a single user?

@KipK
Copy link

KipK commented Jan 30, 2023

To be coherent with UI , as it uses override .
Actions from HA should be visible to UI then and vis versa

@firstof9
Copy link
Owner

Ok I'll see what I can sort out.

@firstof9
Copy link
Owner

My next question, doesn't the override function override the schedule?
If this is still the case, I could foresee this being an issue.

@fhteagle fhteagle changed the title HA Integration sets max_current instead of charge_current HA Integration use of max_current vs charge_current Jan 30, 2023
@fhteagle
Copy link
Author

fhteagle commented Jan 31, 2023

No, I'll move the selector to a configuration entity type and likely put some examples in the README on how to use the set_override service call to adjust the amperage they want to limit the charger to.

Well now I feel silly, as I somehow did not stumble upon the set_override service call in my initial trials with the HA integration. Working on testing that out now, think I have found some gotchas with that also that I will put into separate issue items when I can properly document them.

So far I still think a select.charge_current entity (that is limited to at or below max_current_soft) on the dashboard would still be easier to see/use/set up automations with than the set_override service call, so please keep that suggestion in mind when deciding what you are wanting to re-work.

Will be happy to help document this better to steer other future users in the right direction, instead of following the wrong path with select.max_current like I did.

@jeremypoulter
Copy link

The difference is the priority and the client, the front panel button and the UI use override as those actions are temporary changes that are cleared when the charging session finishes. This is why it is kind of helpful to distinguish between an automation which probably needs to interact with the schedule, PV divert, etc, and a direct interaction with the user that you want to always have priority.

The other case I see for override is triggering a charge (overriding the schedule) via the front button after plugging in the car, then later cancelling that via Home Assistant or the UI (but as part of a user interaction, rather than an automation)

@firstof9
Copy link
Owner

I still think a select.charge_current entity (that is limited to at or below max_current_soft) on the dashboard would still be easier

There's no way a "select.charge_current" can be done right now as the only methods result in potentially overriding a schedule. It would involve a call to the override endpoint and potentially turn on the charger when it may be scheduled to be off.

@jeremypoulter in Home Assistant pretty much anything the user can interact with can be done via automation there really isn't a separation there. I feel override is the proper way to do this, but as stated, it would disrupt the schedule.

I currently have mine set to be disabled from 12pm until 7pm (on peak hours), if I want the charger to fire up at 7pm (after the randomized delay) at 40A rather than 48A, I'd have to use the max_current_soft call, otherwise the override would turn the charger on and start a charge cycle immediately.

@fhteagle
Copy link
Author

fhteagle commented Jan 31, 2023

Good question about whether HA controls are "user" level controls or not. My intuition says that they are, HA is just doing the user controls for you programatically, instead of directly on the GUI. Therefore, sticking with the /override endpoint and tools makes logical sense to me.

I think @firstof9 raises a good point about HA automations overriding schedules that are defined on the unit itself. However, I do not see that as a reason not to make an easier to use control in the dashboard.

In my case, I do not have any schedule set on the unit itself, but I do have the scheduled sleep/disable hours defined and controlled by HA itself, using custom input_time entities and automation clauses. I plan to write them up as a recipe to share with others once I am satisfied that they are doing exactly what I want them to.

I think a note in the HA integration documentation stating that mixing HA automations with on-unit defined schedules may produce unexpected results is probably sufficient. We're probably drifting a bit from the crux of this particular issue though, so it might be good to take up brainstorming about this in the issue at OpenEVSE/openevse_esp32_firmware#461 instead.

@KipK
Copy link

KipK commented Jan 31, 2023

I still think a select.charge_current entity (that is limited to at or below max_current_soft) on the dashboard would still be easier

There's no way a "select.charge_current" can be done right now as the only methods result in potentially overriding a schedule. It would involve a call to the override endpoint and potentially turn on the charger when it may be scheduled to be off.

@jeremypoulter in Home Assistant pretty much anything the user can interact with can be done via automation there really isn't a separation there. I feel override is the proper way to do this, but as stated, it would disrupt the schedule.

I currently have mine set to be disabled from 12pm until 7pm (on peak hours), if I want the charger to fire up at 7pm (after the randomized delay) at 40A rather than 48A, I'd have to use the max_current_soft call, otherwise the override would turn the charger on and start a charge cycle immediately.

Setting an override properties don't set the charger to Active if you have not set the state property. I invite you to test the new UI to understand the new UX.
But you can totally send just a charge_current claim / override while state is disabled.
That's how it works on the new GUI

@firstof9
Copy link
Owner

Setting an override properties don't set the charger to Active if you have not set the state property. I

Ah ok so if I call http://openevse.local/override with just a charge_current set, it won't error out?
If not that'll be great and I can totally work with that.

@KipK
Copy link

KipK commented Jan 31, 2023

you will have to take care first of what properties are present on the override, and repush them with your added one or it will remove the others if there are

@firstof9
Copy link
Owner

firstof9 commented Jan 31, 2023

I can work with that.

So then I'd need to flip the override to active after the charger comes out of sleep (scheduled), correct?

Almost seems like some kind of extra key to the config endpoint that does a $SC <AMPS> N RAPI command would be easier to me.

@KipK
Copy link

KipK commented Jan 31, 2023

So then I'd need to flip the override to active after the charger comes out of sleep (scheduled), correct?
Nope you don't need to add an unecessary state prop, the scheduler service always add a claim with state Active/Disabled
Check the /target endpoint how it works, you will see multiple client controàlling different properties.

Claim engine is a priority system, where each service has a priority defined, they all can post claims over the API, the EvseManager will set each property defined by the higher priority service.
/override is in fact a special endpoint posting claims with higher priority than some other services ( but not all )

Almost seems like some kind of extra key to the config endpoint that does a $SC N RAPI command would be easier to me.

the /config endpoint doesn't need to receive the whole properties, you can send just on prop if you want. Merge is done on fw side, it's only for claims & override & probably /schedule

@firstof9
Copy link
Owner

Yup I understand that, I believe something that could mimic the $SC <AMPS> N RAPI command would be beneficial since, as I understand, using the N flag there means it doesn't write to the EEPROM.

@KipK
Copy link

KipK commented Jan 31, 2023

I've edited my post above at the same time

@KipK
Copy link

KipK commented Jan 31, 2023

Yup I understand that, I believe something that could mimic the $SC <AMPS> N RAPI command would be beneficial since, as I understand, using the N flag there means it doesn't write to the EEPROM.

That's what you can do with /override /claims, send "charge_current" is what your looking for.

The Wifi fw then send the $SC amps command to evse.
min_current_hard <= min_current_soft <= charge_current ( from claim/override) <= max_current ( from claim override ) <= max_current_soft ( from config ) <= max_current hard( from evse )

@firstof9
Copy link
Owner

That's what you can do with /override /claims, send "charge_current" is what your looking for.

So if the override isn't state = active will I need to then monitor the EVSE for when the unit wakes up and then fire the override again to make it active?

@KipK
Copy link

KipK commented Jan 31, 2023

I don't get why.
the Evse is already active, why do you want to add another claim but to prevent it to get to disabled by timers ?

@firstof9
Copy link
Owner

firstof9 commented Jan 31, 2023

Let me break out how I currently have mine setup.

I have it scheduled to sleep between 12pm - 7pm. Now if I were to send an override at like 1pm to change the charging amps for whatever reason, with the following payload: { "state": "disabled", "charge_current": 40 } at 7pm when the schedule wakes up the unit will the amperage be then limited to 40?

Then on the other end, when 12pm rolls around, if I've have an override set for { "state": "active", "charge_current": 40 }, it will roll right past the 12pm schedule right?

I am trying to avoid disrupting the scheduled sleep cycles.

@fhteagle
Copy link
Author

fhteagle commented Jan 31, 2023

@firstof9 I am having trouble thinking of a use case where it would be beneficial to have HA managing the amps but the unit managing the schedule. To me it makes more sense to have HA do scheduling as well to keep things consistent. Is there some reason you really need it set up that way?

Also, yes from my limited testing I am needing to ensure the switch.openevse_manual_override entity is turned on first in order to get what I expect from my latest draft automations. Not a big deal, just one extra line per action set to make sure that switch is on.

@firstof9
Copy link
Owner

I want my devices to be able to keep their scheduling if Home Assistant isn't running for whatever reason.

@KipK
Copy link

KipK commented Jan 31, 2023

have it scheduled to sleep between 12pm - 7pm. Now if I were to send an override at like 1pm to change the charging amps for whatever reason, with the following payload: { "state": "disabled", "charge_current": 40 } at 7pm when the schedule wakes up the unit will the amperage be then limited to 40?

Yes that's how it works. But if Evse reboot the override will be lost.

override has higher priority than Scheduler so yes you can also wake up or disable. That's what it's doing with states on current old UI too..
If you just set a charge_current props, Scheduler doesn't control charge_current, it just set max_current to max_current_soft when waking up, then Shaper will override this with its current calculated max_pwr value

@firstof9
Copy link
Owner

override has higher priority than Scheduler so yes you can wake up or disable.

Ok so if override is set "disabled" and scheduler rolls around to 7pm will the EVSE still be disabled? Because that's been my experience, it stays disabled even after the schedule has come and gone.

@KipK
Copy link

KipK commented Jan 31, 2023

yes it is, as override has bigger priority than the timer, that's the whole purpose of override.

@KipK
Copy link

KipK commented Jan 31, 2023

You don't have to set override to disabled in your use case, as timer already handle it and you want to be woke up by timer.

@KipK
Copy link

KipK commented Jan 31, 2023

When there's no state override , that's when the Robot icon ( Auto mode ) is selected on the main ui2.
Clicking on the robot delete states override.
Then Auto selected means OpenEvse handle the state change ( by Scheduler, Ocpp, Rfid, or other service )

@firstof9
Copy link
Owner

Ok, I'll try a few things and see how it goes.

@firstof9
Copy link
Owner

firstof9 commented Feb 5, 2023

I've updated the library and integration to utilize the override endpoint to adjust the charge amperes limit.
Released in integration version 2.1.8.

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

No branches or pull requests

4 participants