firstof9 / openevse

OpenEVSE Integration for Home Assistant
MIT License
66 stars 11 forks source link

HA Integration use of max_current vs charge_current #120

Closed fhteagle closed 1 year ago

fhteagle commented 1 year ago

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:

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 commented 1 year ago

https://github.com/KipK/openevse-gui-v2/

https://github.com/OpenEVSE/ESP32_WiFi_V4.x/releases/tag/v2_gui

jeremypoulter commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

@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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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

firstof9 commented 1 year ago

Will the max_current_soft key will be no more then?

KipK commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

@KipK thanks for the info about claim tag. Opened https://github.com/OpenEVSE/ESP32_WiFi_V4.x/issues/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?

firstof9 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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

KipK commented 1 year ago

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

firstof9 commented 1 year ago

Ok I'll see what I can sort out.

firstof9 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 https://github.com/OpenEVSE/ESP32_WiFi_V4.x/issues/461 instead.

KipK commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

I've edited my post above at the same time

KipK commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

@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 commented 1 year ago

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

KipK commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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

KipK commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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

firstof9 commented 1 year ago

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.