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

Abstract out TWC interface code #483

Draft
wants to merge 62 commits into
base: main
Choose a base branch
from
Draft

Abstract out TWC interface code #483

wants to merge 62 commits into from

Conversation

MikeBishop
Copy link
Collaborator

@MikeBishop MikeBishop commented Nov 6, 2022

This is the first step toward making Gen2 TWCs just another module and supporting other charge controllers alongside them. At this point, it's pure code rearrangement -- functionality should be identical. Testing to ensure I haven't broken anything -- particularly modes I don't use, like fakeMaster: 0 -- would be appreciated.

Before merging, I want to:

  • Move the TWC code into a module, so that the core TWCManager doesn't interact with the serial interface at all
  • Abstract the TWCMaster code's relationship with that module so it doesn't make assumptions about what's behind a given charge controller; this likely requires moving a lot of its code into the charge controller
  • Move as much charging logic as possible out of the TWC code and into the main program loop
  • Add support for Tesla API as an alternative charge controller (i.e. vehicles on mobile connector)
  • Deduplicate EVSE instances which are addressing the same vehicle (e.g. vehicle plugged into a TWC)

Long term, I'd also like to support read-only charge controllers, such as TWCs connected to a second TWCManager instance. Their data can be considered when making charge policy decisions, even if it can't be affected locally.

@MikeBishop
Copy link
Collaborator Author

An interesting ramification of these changes is that different types of EVSEs may have different voltages -- the canonical example being a Gen2 TWC on 240V 80A and a UMC on 120V 15A.

With a diversity of voltages in play, I think it may make more sense to convert all the core calculations to working in watts and convert to amps just before sending them into the calls that require amperage as the input. That's going to flow back through various other parts of the system (and probably break stuff).

@MikeBishop
Copy link
Collaborator Author

MikeBishop commented Nov 10, 2022

Feature-complete but bug-ridden. Here are the things it does and doesn't do, and the known bugs:

  • TWCs (when VINs are reported) and vehicles are paired up. This should have the side-effect of fixing the single-phase car on three-phase issue, since the number of phases reported by the car is preferred over what the TWC reports. I've made no attempt so far to make this sane on TWCs which cannot report the connected VIN.
  • Directing which method to use for start/stop is kind of fishy right now; it might be the case, at least in the short-term, that this will always use both methods -- API commands and TWC commands -- in parallel.
  • The HTTPControl module makes lots of assumptions about the interior of a TWC, reaching inside to pull out information. This currently throws exceptions, since the API object doesn't have the same internal structure. We might want to change the API return format to match the interface I've used for the EVSE type instead; it seems impractical to duplicate the entire internal structure.
  • If there's not enough power to run all EVSEs at their lowest level, EVSEs are kicked out of the target group until there are few enough left to manage. There's currently no prioritization in this order, so it's somewhat random which one will get blessed with power.
  • Because we might have EVSEs with different possible amounts of power, power is allocated in order of increasing max power. The EVSE with the lowest max power is given a fair share of the available pool; if it can't take that much, the remaining EVs split the excess and so on.
  • The maximum power consumption settings currently apply only to Gen2 TWCs; the virtual EVSEs representing cars (which may be on the Mobile Connector or a Gen3 TWC) share the power available from the policy but make no attempt to keep their collective draw under any threshold. (It's assumed that either appropriate circuit sizing or Gen3 TWC power sharing will take care of that.)
  • The Power History API depends on the periodic current measurements recorded by receiving the TWC heartbeat. The API-based EVSEs have no such periodic reporting; at best we know when it changes (Teslamate) and at worst we find out occasionally when we have reason to poll the API again. I'm sure with some judicious use of timestamps we could reconstruct power usage, though without Teslamate I'm dubious it will be very accurate.
  • I'm not totally clear on whether Tesla APIs are being called at an appropriate frequency -- it might be too often, it might not be often enough. There are some new paths through the code, and some short-circuits might be getting skipped.

Overall, I think I'd like to see us drop a 1.2.7 release with any other features we'd like to ship first, then make this the base of a 1.3 or 2.0 release. This is going to need some testing in a variety of setups before it's ready for broad use.

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.

2 participants