ngardiner / TWCManager

Control power delivered by a Tesla Wall Charger using two wires screwed into its RS-485 terminals.
The Unlicense
130 stars 55 forks source link

polling of /api/getStatus isn't updating values while charging #211

Open hopfi2k opened 3 years ago

hopfi2k commented 3 years ago

I wonder why polling /api/getStatus doesn't seem to provide actual values for consumption/generation etc. when a car is charging?

Calling this URL calls the function master.getStatus() which is in TWCMaster.py. What I don't understand why values are obviously not updated while charging?

MikeBishop commented 3 years ago

Consumption/generation in TWCMaster is only updated while a Green policy is active. So if the car is charging because there's solar available, they'll be accurate. If the car is charging because there's a Scheduled period, the generation/consumption numbers will be stale.

hopfi2k commented 3 years ago

Thanks Mike for the clarification. Is there any reason why these values aren't updated on other charging policies?

ngardiner commented 3 years ago

The green energy values aren't fetched when they aren't being used, because we don't need them in those cases. Some installations may not track green energy at all - there's a few different use cases including people who want to control power draw on shared circuits with additional visualization or controls.

There's a parameter that you can use (isGreenPolicy) as part of the getStatus function which indicates if a policy is tracking green energy (and as a result if generation is actively being updated). I'd be inclined to suggest that if this is set to No, hiding the green energy generation details might be the best approach, as it would hide those unnecessary details for non-green energy users as well. How dependent is the UI on showing solar generation/consumption, would it handle use cases where there isn't any?

hopfi2k commented 3 years ago

Thanks for insight @ngardiner !

Limiting displaying of these values to an isGreenPolicy = true scenario is something I'd suggest to change. If I understand things correctly, I don't see any reason why functions like getConsumption() should be called when NOT using a green policy. Think of a scenario, where someone charges his car on a cloudy day using a non green policy (e.g. charge now). I think it would make a lot of sense to still update and show values for the current solar production (it maybe be like 45% of the actual consumption) and draw from grid. Therefore I do strongly suggest to call getConsumption() and getGeneration() even on none green policies. It is confusing (in standard and new UI) to see that these values are not updated. Is there some reason I may be overlooking that is preventing this value from always being updated? (independent from the current policy)

ngardiner commented 3 years ago

There are some considerations - for example, if any cloud service is used with a quota of API calls per day (there's a handful of cloud services such as SolarEdge, Enphase, SmartMe that we support), continuing to query those API calls will exceed the quota per day, meaning we'd have to spread that quota over a full 24 hour period rather than the ~8 hours of green energy per day, which would result in much longer intervals between queries and making the integration less useful (as we want to react as fast as we can to changes in consumption/generation).

This isn't an issue currently, as we only fetch them when we need them (during the relevant green energy hours, when the policy is active).

That said, I can't see why the background function that updates the consumption/generation values couldn't be added to the charge now/scheduled charging policies if it were necessary so that updates are visible in the UI (with some potential impact to the cloud API users if they make significant use of scheduled charging, something that we'd want to note in the documentation).

You can modify the policies in your config.json to make it do what you want and test if it meets your requirements, just uncomment all of the commented out policies in config.json in the Override section, and add the bolded lines below to the Charge Now and Scheduled Charging policies, and it should do what you are asking:

"override": [ { "name": "Charge Now", "match": [ "settings.chargeNowAmps", "settings.chargeNowTimeEnd", "settings.chargeNowTimeEnd" ], "condition": [ "gt", "gt""gt" ], "value": [ 0, 0, "now" ], "background_task": "checkGreenEnergy", "charge_amps": "settings.chargeNowAmps", "charge_limit": "config.chargeNowLimit" },

{ "name": "Scheduled Charging", "match": [ "checkScheduledCharging()" ], "condition": [ "eq" ], "value": [ 1 ], "background_task": "checkGreenEnergy", "charge_amps": "settings.scheduledAmpsMax", "charge_limit": "config.scheduledLimit" },

MikeBishop commented 3 years ago

Careful with that.... IIRC, that background task actually sets the charge_amps (you'll note that Track Green Energy doesn't supply a charge_amps value). So just adding the task to the policy will cause it to oscillate between the applied limit and zero at a high frequency.

We also can't condition that behavior on checking for a green policy, because the isGreenPolicy code... checks whether the background task is checkGreenEnergy. 🤔

MikeBishop commented 3 years ago

I suspect if we want green energy polling all the time, we either need to split checkGreenEnergy up into two pieces; one that fetches the data and one that applies the limit; or be able to pass a flag to checkGreenEnergy to indicate whether to apply the limit.

Maybe as simple as having it check whether the currently-active policy specifies a charge_amps and not overriding it if it does.

hopfi2k commented 3 years ago

Very interesting insights - thank you so much! Concerning the API rate restrictions that @ngardiner mentioned: this is a very important point (although to the best of my knowledge this only affects SolarEdge devices at the moment?). I do really need to find the time to write an universal ModBus EMS so we don't have to rely on only Cloud API access (which, as the hardware is usually on premise, doesn't make sense at all...). The other thing, that @MikeBishop pointed out is the matter of the charge_limit parameter. Shouldn't charge_limit be a fixed value variable (user/config defined)? Why are we setting it in the background task?

MikeBishop commented 3 years ago

Because when you're doing a green policy, the charge limit is set based on the generation/consumption numbers.

hopfi2k commented 3 years ago

Ah, sorry, my bad. I was mixing SOC with charge_limit. Sorry for that.

MikeBishop commented 3 years ago

Actually, no -- I was. This is charge_amps, not charge_limit that's being overridden.

hopfi2k commented 3 years ago

:) Wouldn't the easiest solution be to check in the background task wether isGreenPolicy() is True and otherwise don't update charge_amps? Maybe I have to dig into this code too...

MikeBishop commented 3 years ago

No, because the indicator that something is a green policy is the presence of that background task.

def policyIsGreen(self):
    if self.getPolicyByName(self.active_policy):
        return (
            self.getPolicyByName(self.active_policy).get("background_task", "")
            == "checkGreenEnergy"
        )
    return 0

I think we'd want to check for the presence of charge_amps in the policy and only override if it's not specified.

hopfi2k commented 3 years ago

Yes, that makes much more sense! Would you be willing to make the necessary changes, Mike?

ngardiner commented 3 years ago

I've merged @MikeBishop's PR in #212 which ensures that the charge_amps value isn't overwritten when calling checkGreenEnergy. This would allow us to add checkGreenEnergy to Charge Now and Scheduled Charging policies. I think we need to be running it for a little while first though to make sure it's not going to break anything.

I'm running it up in my install. So far, it's working perfectly - on a Charge Now policy I'm getting the selected amperage for the period of time set, but with the Green Energy values updating throughout.

image

MikeBishop commented 3 years ago

I'd think at that point it makes more sense to just always run the background task, maybe with an option to turn it off for non-green policies if you're API-limited.