tdorssers / TeslaPy

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

Extra information being returned for solar panel systems #96

Open shred86 opened 2 years ago

shred86 commented 2 years ago

I noticed when I get a JSON response from the SITE_DATA endpoint in the CLI using the --site argument, the data from the PRODUCT_LIST endpoint is also being included.

This is what the SITE_DATA endpoint return should look like:

{
    "solar_power": 5713.47998046875,
    "energy_left": 0,
    "total_pack_energy": 1,
    "percentage_charged": 0,
    "battery_power": 0,
    "load_power": 0,
    "grid_status": "Unknown",
    "grid_services_active": false,
    "grid_power": -5713.47998046875,
    "grid_services_power": 0,
    "generator_power": 0,
    "island_status": "island_status_unknown",
    "storm_mode_active": false,
    "timestamp": "2022-08-15T17:29:31Z",
    "wall_connectors": null
}

This is what's showing up when using --site from the CLI:

{
    "energy_site_id": redacted,
    "resource_type": "solar",
    "id": "redacted",
    "asset_site_id": "redacted",
    "solar_power": 11892.01953125,
    "solar_type": "pv_panel",
    "storm_mode_enabled": null,
    "powerwall_onboarding_settings_set": null,
    "sync_grid_alert_enabled": false,
    "breaker_alert_enabled": false,
    "components": {
        "battery": false,
        "solar": true,
        "solar_type": "pv_panel",
        "grid": true,
        "load_meter": true,
        "market_type": "residential"
    },
    "energy_left": 0,
    "total_pack_energy": 1,
    "percentage_charged": 0,
    "battery_power": 0,
    "load_power": 0,
    "grid_status": "Unknown",
    "grid_services_active": false,
    "grid_power": -11892.01953125,
    "grid_services_power": 0,
    "generator_power": 0,
    "island_status": "island_status_unknown",
    "storm_mode_active": false,
    "timestamp": "2022-08-15T17:12:26Z",
    "wall_connectors": null
}

You can see the additional information is coming from what's contained in PRODUCT_LIST, which is:

{
        "energy_site_id": redacted,
        "resource_type": "solar",
        "id": "redacted",
        "asset_site_id": "redacted",
        "solar_power": 2530,
        "solar_type": "pv_panel",
        "storm_mode_enabled": None,
        "powerwall_onboarding_settings_set": None,
        "sync_grid_alert_enabled": False,
        "breaker_alert_enabled": False,
        "components": {
            "battery": False,
            "solar": True,
            "solar_type": "pv_panel",
            "grid": True,
            "load_meter": True,
            "market_type": "residential",
        }

I noticed when get_site_data() is being called, it's returning self, which if I print out self prior to get_site_data() being called, it is the JSON response to PRODUCT_LIST. So it just appears it's combining the two JSON responses.

If I change the get_site_data() method to simply return self.api('SITE_DATA')['response'], it fixes the issue. However, I'm sure there's a reason self.update was used and return self, but I can't figure it out. 😄

tdorssers commented 2 years ago

The basic idea is that when you call the VEHICLE_SUMMARY API you get the same basic information as returned by PRODUCT_LIST and when you call VEHICLE_DATA, you get detailed info. I decided to put this in a single dict, because you probably need the basic and the detailed information and then its available in a single dict. I don't want to create a method for each API call, so the code is easier to maintain. Some time later I improved the vehicle dict to automatically call the VEHICLE_DATA API, but I didn't do that for other products. I wanted to keep the powerwall and solar panel interface similar to the vehicle interface and that's why both JSON responses are combined.

shred86 commented 2 years ago

Ahh, I see. That definitely threw me for a loop as I thought I was going crazy. 😄 At a minimum, that may be worth documenting just in a sentence somewhere. Personally, I'd prefer to see the responses broken out into two separate dictionaries so it's clear which response came from which endpoint, but I can understand that you're trying to simplify it. I also kind of like just having only the response for the endpoint you're hitting display since it's easy to combine multiple CLI arguments, but I suppose it depends on the use case. For me, I'm trying to have folks with different Tesla solar system setups provide their JSON response which this tool you created to be the easiest to do that, but obviously having responses combined makes that a bit more difficult. It's easy enough to just fork and make some minor changes though. I appreciate you creating and maintaining this tool as it's been very helpful.