tdorssers / TeslaPy

A Python module to use the Tesla Motors Owner API
MIT License
374 stars 84 forks source link

Added tariff setting method to battery #82

Closed 696GrocuttT closed 2 years ago

tdorssers commented 2 years ago

Thanks for this addition. This will break Python 2.7 and early 3.x support, since you are using dataclass. Can you rewrite it so it supports earlier versions?

696GrocuttT commented 2 years ago

Even getting python2.7 installed was a bit of a mission, but I think its all sorted now.

tdorssers commented 2 years ago

@696GrocuttT I have refactored the code, can you please validate that it is still doing what it is supposed to do?

The code could run into an infinite loop, because the background_time list was appended within a for loop. I made a copy of background_time by slicing and enumerated that to prevent an infinite loop.

I made costs a defaultdict(list) so these lines can be omitted:

if not period.cost in costs:
    costs[period.cost] = []

The BatteryTariffPeriodCost and BatteryTariffPeriod classes are now namedtuples. This is convenient when sorting a list of costs (no key function needed). But since tuples are immutable, you need a new tuple if a change is needed: period = period._replace(end=midnight_start_time)

I changed these 3 lines:

            sortedCosts     = sorted(costs, key=lambda x: (x.buy, x.sell))
            for (index, cost) in enumerate(reversed(sortedCosts)):
                name                  = period_names[index]

To a single line:

        for (name, cost) in zip(period_names, sorted(costs, reverse=True)):

All variables are now camel case and white spaces are removed. Finally the function returns a JsonDict, so it prints nicely.

696GrocuttT commented 2 years ago

@tdorssers I've had a quick look and I can't see any problems with the new code. My powerwall is still accepting the generated tariff data, so all good from my side.