zabuldon / teslajsonpy

Apache License 2.0
57 stars 62 forks source link

Energy site name not being set correctly #338

Closed shred86 closed 2 years ago

shred86 commented 2 years ago

Currently self._site_name in power.py is being set using data.get("site_name", f"{self._energy_site_id}"). However, looking at the JSON response data, site_name appears to be defined in the site_info endpoint (aka SITE_CONFIG). It doesn't look like that endpoint is every being used, only live_status (aka SITE_DATA) and products (aka PRODUCT_LIST), meaning it will always be set to the self._energy_site_id.

I think the best solution might be to simply pull the JSON data from site_info when the integration initializes and append that to the energysite dictionary. The issue this introduces though is it would be a breaking change since right now we're setting _uniq_name to be self._name, so all the power sensors would be recreated and the old ones would become unavailable.

I'm not sure if the major rewrite is still underway but perhaps we can just introduce all of these changes then so we just have one big change with breaking changes.

Of note, I noticed when I first looked at the JSON response of site_info, it did not have a key for site_name. I posted that response here. When I looked at it again today, I noticed it contained site_name as well as more values for my home address. I did try messing around with changing my site name in the Tesla app when I was working on this previously, and I ended up reverting the name back to My Home, so I'm wondering if once you set the name in the app, it shows up in the JSON response. Here's the full JSON response from today:

{
    "id": "redacted",
    "site_name": "My Home",
    "site_number": "redacted",
    "installation_date": "2022-04-04T15:56:35-07:00",
    "user_settings": {
        "storm_mode_enabled": null,
        "powerwall_onboarding_settings_set": null,
        "sync_grid_alert_enabled": false,
        "breaker_alert_enabled": false
    },
    "components": {
        "solar": true,
        "solar_type": "pv_panel",
        "battery": false,
        "grid": true,
        "backup": false,
        "gateway": "gateway_type_none",
        "load_meter": true,
        "tou_capable": false,
        "storm_mode_capable": false,
        "flex_energy_request_capable": false,
        "car_charging_data_supported": false,
        "off_grid_vehicle_charging_reserve_supported": false,
        "vehicle_charging_performance_view_enabled": false,
        "vehicle_charging_solar_offset_view_enabled": false,
        "battery_solar_offset_view_enabled": false,
        "energy_service_self_scheduling_enabled": true,
        "rate_plan_manager_supported": true,
        "configurable": false,
        "grid_services_enabled": false
    },
    "installation_time_zone": "America/Los_Angeles",
    "time_zone_offset": -420,
    "geolocation": {
        "latitude": redacted
        "longitude": redacted
    },
    "address": {
        "address_line1": "redacted",
        "city": "redacted",
        "state": "redacted",
        "zip": "redacted",
        "country": "US"
    }
}
alandtse commented 2 years ago

I wouldn't wait for the rewrite. Just decide if this makes sense on it's own and do the breaking change then. Hopefully it includes some specific benefits on the solar side to encourage people to update.

shred86 commented 2 years ago

Sounds good! Just made a PR and it won't introduce a breaking change.