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

fix logging of charge limit #432

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

dtiefnig
Copy link

There are a few improvements to the log output of update_statuses() in TWCManager.py.

You may reject the simplification of the calculation if you prefer to keep it the way you implemented it. I found it easier to read if it is simplified the way I did.

Logging looked strange the way it was while charging was ongoing, when subtractChargerLoad was set. Log line was e.g. like:
⛽ Manager 20 Limiting charging to 6.88A - 6.61A = 6.77A.
Or when nominal offer differed from actual:
⛽ Manager 20 Limiting charging to 7.04A - -0.00A = 7.04A.

When subtractChargerLoad is set, it now always has the following format:
⛽ Manager 20 Limiting charging to 7.02A - (6.59A - 6.50A) = max 6.93A.

When subtractChargerLoad is set, calculation of charging limit is
different from what has been logged so far. The new log line format
correctly represents the applied logic.
Recalculation of consumption needs to consider "subtractChargerLoad"
and add charging power if it is set.
The formula can be simplified this way to improve readability.
From my POV the check for "conwatts >= 0" could be removed, as
master.getConsumption() always returns a value >= 0, but this
might change in the future, maybe?
@ngardiner
Copy link
Owner

If we're changing it, do you think we just change it completely? I'd happily go for a format where we broke the values out clearly, in whatever way looks cleanest - key / value pairs, tabular, whatever would make it more user friendly.

I do like your update but I also think if we're fixing it, let's not limit ourselves to how it looks today, let's make it as clear as we can. I'm open to any and all suggestions.

@dtiefnig
Copy link
Author

I was playing around with log message formats a little when I implemented the fix, but couldn't find one that convinced me. Something like the following might be better to understand, but I don't really like it, do you?
⛽ Manager 20 Limiting charging to 7.02A green - (6.59A used - 6.50A charge) = max 6.93A.

@MikeBishop
Copy link
Collaborator

MikeBishop commented Apr 25, 2022

Part of the issue is that we have 2-6 different ways we can arrive at the limit. There's currently code to try to "retcon" them into one computation for display purposes, which isn't really accurate. In my ideal vision, the log line would actually explain why it's choosing that value to offer. For example:

  • Consumption exceeds production: Reducing offer to 12A (current offer) - 1.3A (excess consumption) = 10.7A
  • Production exceeds consumption: Increasing offer to 12A (current offer) + 2.8A (excess production) = 14.8A
  • Unused production has changed: [Increasing/Reducing] offer to 37.6A (current production) - 14.7A (current consumption) = 22.9A
  • Production and consumption roughly balanced: Continuing to offer 12A.

That would require preserving information from the computation stage about which method governed the outcome this cycle, but might be more user-friendly than more number spew.

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.

3 participants