Simplify the V1 methods by using single re-usable base methods

[GraemeDBlue]

Summary

• Simplify the V1 methods by using single re-usable base methods
• Add Mix/SPH to V1 and while retaining legacy Shinephone backwards compatibility
• Improve some code style

Checklist

• I've made sure the PR does small incremental changes. (new code additions are dificult to review when e.g. the entire repository got improved codestyle in the same PR.)
• I've added/updated the relevant docs for code changes i've made.

[johanzander]

Thanks for adding mix support! However, I have a concern about the API design:

Passing inverter_type as a parameter to every method call creates unnecessary coupling and repetition. As I wrote in previous feedback (even though AI helped me phrase it): Inverter type is a property of the inverter, not a parameter of each operation. It should be determined once and reused internally, not passed around repeatedly.

Here is how you could do it:

  class OpenApiV1(GrowattApi):
      def __init__(self, token):
          super().__init__(agent_identifier=self._create_user_agent())
          self.api_url = f"{self.server_url}v1/"
          self.session.headers.update({"token": token})
          self._device_type_cache = {}

      def device_list(self, plant_id):
          """Get devices and cache their types"""
          response = self.session.get(
              url=self._get_url("device/list"),
              params={"plant_id": plant_id, "page": "", "perpage": ""}
          )
          data = self._process_response(response.json(), "getting device list")

          # Cache device types (type field from API)
          for device in data.get('devices', []):
              self._device_type_cache[device['device_sn']] = device['type']

          return data

      def _get_device_type(self, device_sn):
          """Get cached device type"""
          if device_sn not in self._device_type_cache:
              raise GrowattParameterError(
                  f"Device type unknown for {device_sn}. Call device_list() first."
              )
          return self._device_type_cache[device_sn]

      def _map_type_to_enum(self, type_code):
          """Map numeric type to DeviceType enum"""
          # Based on device_list documentation (line 203-213)
          if type_code == 7:
              return DeviceType.MIN_TLX
          elif type_code in [2, 5, 6]:  # storage, sph, spa
              return DeviceType.MIX_SPH
          # ... other mappings
          else:
              raise GrowattParameterError(f"Unsupported device type: {type_code}")

      # PUBLIC API - user never passes device_type
      def device_detail(self, device_sn):
          """Get device details - automatically routes to correct endpoint"""
          type_code = self._get_device_type(device_sn)
          device_type = self._map_type_to_enum(type_code)
          return self._device_details(device_sn, device_type)

      def device_energy(self, device_sn):
          """Get device energy - automatically routes to correct endpoint"""
          type_code = self._get_device_type(device_sn)
          device_type = self._map_type_to_enum(type_code)
          return self._device_energy(device_sn, device_type)

      # INTERNAL methods (with underscore prefix) - these take device_type
      def _device_details(self, device_sn, device_type):
          """Internal: Get detailed data for a device"""
          if not isinstance(device_type, DeviceType):
              raise GrowattParameterError(f"Invalid device type: {device_type}")

          response = self.session.get(
              self._get_device_url(device_type, ApiDataType.BASIC_INFO),
              params={'device_sn': device_sn}
          )
          return self._process_response(response.json(), f"getting {device_type.name} 
  details")

      # BACKWARDS COMPATIBILITY - keep old method names
      def min_detail(self, device_sn):
          """Backwards compatible - prefer device_detail()"""
          return self._device_details(device_sn, DeviceType.MIN_TLX)

      def mix_detail(self, device_sn, plant_id=None):
          """Backwards compatible - prefer device_detail()"""
          return self._device_details(device_sn, DeviceType.MIX_SPH)

  Clean Usage

  # Get all devices (caches types automatically)
  devices = api.device_list(plant_id)

  # Clean, consistent API - works for ALL device types
  for device in devices['devices']:
      sn = device['device_sn']

      # User never thinks about type - just uses device_sn
      details = api.device_detail(sn)
      energy = api.device_energy(sn)
      settings = api.device_settings(sn)

  Key points:
  - Public methods (device_detail, device_energy) = no device_type parameter
  - Internal methods (_device_details, _device_energy) = have device_type parameter
  - Backwards compatibility kept with min_detail, mix_detail wrappers
  - Type determined once from device_list(), cached, and reused automatically

[GraemeDBlue]

Thanks for adding mix support! However, I have a concern about the API design:

Passing inverter_type as a parameter to every method call creates unnecessary coupling and repetition. As I wrote in previous feedback (even though AI helped me phrase it): Inverter type is a property of the inverter, not a parameter of each operation. It should be determined once and reused internally, not passed around repeatedly.

Here is how you could do it:

  class OpenApiV1(GrowattApi):
      def __init__(self, token):
          super().__init__(agent_identifier=self._create_user_agent())
          self.api_url = f"{self.server_url}v1/"
          self.session.headers.update({"token": token})
          self._device_type_cache = {}

      def device_list(self, plant_id):
          """Get devices and cache their types"""
          response = self.session.get(
              url=self._get_url("device/list"),
              params={"plant_id": plant_id, "page": "", "perpage": ""}
          )
          data = self._process_response(response.json(), "getting device list")

          # Cache device types (type field from API)
          for device in data.get('devices', []):
              self._device_type_cache[device['device_sn']] = device['type']

          return data

      def _get_device_type(self, device_sn):
          """Get cached device type"""
          if device_sn not in self._device_type_cache:
              raise GrowattParameterError(
                  f"Device type unknown for {device_sn}. Call device_list() first."
              )
          return self._device_type_cache[device_sn]

      def _map_type_to_enum(self, type_code):
          """Map numeric type to DeviceType enum"""
          # Based on device_list documentation (line 203-213)
          if type_code == 7:
              return DeviceType.MIN_TLX
          elif type_code in [2, 5, 6]:  # storage, sph, spa
              return DeviceType.MIX_SPH
          # ... other mappings
          else:
              raise GrowattParameterError(f"Unsupported device type: {type_code}")

      # PUBLIC API - user never passes device_type
      def device_detail(self, device_sn):
          """Get device details - automatically routes to correct endpoint"""
          type_code = self._get_device_type(device_sn)
          device_type = self._map_type_to_enum(type_code)
          return self._device_details(device_sn, device_type)

      def device_energy(self, device_sn):
          """Get device energy - automatically routes to correct endpoint"""
          type_code = self._get_device_type(device_sn)
          device_type = self._map_type_to_enum(type_code)
          return self._device_energy(device_sn, device_type)

      # INTERNAL methods (with underscore prefix) - these take device_type
      def _device_details(self, device_sn, device_type):
          """Internal: Get detailed data for a device"""
          if not isinstance(device_type, DeviceType):
              raise GrowattParameterError(f"Invalid device type: {device_type}")

          response = self.session.get(
              self._get_device_url(device_type, ApiDataType.BASIC_INFO),
              params={'device_sn': device_sn}
          )
          return self._process_response(response.json(), f"getting {device_type.name} 
  details")

      # BACKWARDS COMPATIBILITY - keep old method names
      def min_detail(self, device_sn):
          """Backwards compatible - prefer device_detail()"""
          return self._device_details(device_sn, DeviceType.MIN_TLX)

      def mix_detail(self, device_sn, plant_id=None):
          """Backwards compatible - prefer device_detail()"""
          return self._device_details(device_sn, DeviceType.MIX_SPH)

  Clean Usage

  # Get all devices (caches types automatically)
  devices = api.device_list(plant_id)

  # Clean, consistent API - works for ALL device types
  for device in devices['devices']:
      sn = device['device_sn']

      # User never thinks about type - just uses device_sn
      details = api.device_detail(sn)
      energy = api.device_energy(sn)
      settings = api.device_settings(sn)

  Key points:
  - Public methods (device_detail, device_energy) = no device_type parameter
  - Internal methods (_device_details, _device_energy) = have device_type parameter
  - Backwards compatibility kept with min_detail, mix_detail wrappers
  - Type determined once from device_list(), cached, and reused automatically

@johanzander thanks for your comments, but there is no need to pass the type at all did you see

   plants = api.plant_list()
    print(f"Plants: Found {plants['count']} plants")
    plant_id = plants['plants'][0]['plant_id']
    today = datetime.date.today()
    devices = api.get_devices(plant_id)
    for device in devices:
        # Works automatically for MIN, MIX, or any future device type!
        details = device.details()
        energy = device.energy()
        settings = device.settings()
        history = device.energy_history(start_date=today)

[johanzander]

great to add this to the library. But I have up to 4 individual PVs, so please use something more dynamic, taking into account the sum of up to 4 if present.

[GraemeDBlue]

@johanzander are they all of the same type or do you have a mix? that might be a interesting scenario

[GraemeDBlue]

added a sum

[johanzander]

but you have modified min_example.py to take device type parameter. is this a mistake?

[johanzander]

passing device_type?

[GraemeDBlue]

let me check , but I believe I have 3 different ways to call the same methods , device_details() is the base method with sn and device type https://github.com/indykoning/PyPi_GrowattServer/pull/125/files#diff-12865f9986e5f28194397aae0efed2b7abc9137d18c0a09197682e6c7e65cd01R484-R511, the details() https://github.com/indykoning/PyPi_GrowattServer/pull/125/files#diff-12865f9986e5f28194397aae0efed2b7abc9137d18c0a09197682e6c7e65cd01R1225-R1227 where the sn and device type are set on the object, so not needed as params , I will roll back my changes in examples/min_example.py to use a backwards compatible min_detail(sn) which will just call device_details(sn, device_type=7 )

[johanzander]

Amazing!
Maybe name it v1_example.py instead of mix_v1_example.py since its a generic example?

[johanzander]

Backward compatibility with MIN inverters using V1 seem to work...
Will test the new harmonized V1 APIs later...

[johanzander]

Please remove debug code from prod version.
Im testing the library and I see these:
Saving getting_MIN_TLX_details_data.json
Saving getting_MIN_TLX_settings_data.json
Saving getting_MIN_TLX_energy_data_data.json

[johanzander]

For consistency, should you call the enum

MiX_SPH for SPH_MIX instead (<DEVICE_TYPE>)

consider adding all types in the enum:
INVERTER = 1
STORAGE = 2
OTHER = 3
MAX = 4
SPH_MIX = 5
SPA = 6
MIN_TLX = 7
PCS = 8
HPS = 9
PBD = 10

https://www.showdoc.com.cn/262556420217021/6117958613377445

[GraemeDBlue]

The only thing consistent with their naming convention, is the inconsistency

[GraemeDBlue]

Added Read commands

[GraemeDBlue]

@indykoning what it the process to get a review here?

[GraemeDBlue]

Is this ever going to get anywhere?