nmakel / solaredge_modbus

SolarEdge Modbus data collection library
MIT License
146 stars 36 forks source link

Parent Inverter Modbus TCP not working #64

Open rmnsrrs opened 1 year ago

rmnsrrs commented 1 year ago

I am trying to query a system with 2 inverters. If I run the example script with each unit (1 & 2) then it is working. However the following code does not work and query twice the inverter 1. I notice that if I change the order in the list I then get twice the inverter 2. In other word I get the 1st inverter in the list twice and never the 2nd one. If that normal behavior ? How should I use parent parameter ? Thanks

#!/usr/bin/env python3
import argparse
import json

import solaredge_modbus

class solarEdge_extractor(object):
    def __init__(self, host, port):
        self.inverters = [solaredge_modbus.Inverter(
            host=host,
            port=port,
            unit=1
        )]
        self.inverters.append(solaredge_modbus.Inverter(
            parent=self.inverters[0],
            unit=2
        ))

    def get_data(self):
        values = {}
        values["inverters"] = {}
        values["meters"] = {}
        values["batteries"] = {}
        for i,c_i in enumerate(self.inverters):
            print(c_i.read("current"))
            values["inverters"][i] = c_i.read_all()

            meters = c_i.meters()
            batteries = c_i.batteries()
            values["meters"][i] = {}
            for meter, params in meters.items():
                meter_values = params.read_all()
                values["meters"][i][meter] = meter_values

            values["batteries"][i] = {}
            for battery, params in batteries.items():
                battery_values = params.read_all()
                values["batteries"][i][battery] = battery_values

        return json.dumps(values, indent=4)

if __name__ == "__main__":
    argparser = argparse.ArgumentParser()
    argparser.add_argument("host", type=str, help="Modbus TCP address")
    argparser.add_argument("port", type=int, help="Modbus TCP port")
    args = argparser.parse_args()
    se = solarEdge_extractor(args.host, args.port)
    print(se.get_data())
nmakel commented 1 year ago

Looks like this should work. Have you grabbed the latest version from github? A commit from 10 days ago, which isn't in the pypi release yet, should have fixed this.

Edit: this is the commit in question.

rmnsrrs commented 1 year ago

You were correct, It did fix the issue. However the meter is now empty. While it worked perfectly before. It detects "Meter1" but return an empty dict. Any chance you would know why ?

nmakel commented 1 year ago

Would you mind trying it explicitly, through the intepreter for example, to rule out any funny business with your looping and dict assignments?

rmnsrrs commented 1 year ago

Yes sorry, it was not clear. But I am always testing with the example given on the git. When calling unit 2, I can see the meter is "Meter1" but meter_values is an empty dict {} (cf below) Also I reverted back to the pipy version and the meter is working fine again. It seems that there is bug introduced in between the pipy version and the latest github

    for meter, params in meters.items():
        meter_values = params.read_all()
        values["meters"][meter] = meter_values
        print(meter_values)
nmakel commented 1 year ago

I understand. But what I'm asking you is to run through all of the steps one by one. This way we can see where and when things go wrong. Perhaps somewhere a unit is not being passed correctly, but by only looking at the code I'm not seeing it. Debug output, printing results from every function, printing every object (inverter, meter), would be useful.

rmnsrrs commented 1 year ago

I can confirm after several tests that this is only due do adding these lines:

         if unit:
            self.unit = unit
        else:
            self.unit = parent.unit

Problem: Meter is not read correctly. and return empty dict. Thus by looking in places where unit is used for meter, we should find out the problem.

Note : In my case the Meter is on unit=2

Regarding the previous steps:

jgyates commented 1 year ago

I am having a similar issue. the problem is that in the above lines:

     if unit:
        self.unit = unit
    else:
        self.unit = parent.unit

If unit is anything other than 1 then it is false. Here is the output form the python console:

    >>> import solaredge_modbus
    >>> inv1 = solaredge_modbus.Inverter(host = "192.168.1.51", port=1502, unit =1, timeout=3)
    >>> print(inv1)
    Inverter(192.168.1.51:1502, connectionType.TCP: timeout=3, retries=3, unit=0x1)
    >>> inv2 = solaredge_modbus.Inverter(parent=inv1, unit = 2)
    >>> print(inv2)
    Inverter(192.168.1.51:1502, connectionType.TCP: timeout=3, retries=3, unit=0x1)
    >>> inv2.unit == True
    True
    >>> 2 == True
    False
    >>> 1 == True
    True

This makes the 2nd inverter inaccessible if trying to access two inverters (via a parent) in the same program. It works with example.py if you pass a unit of 2 as there is no parent in that case.

jgyates commented 1 year ago

If you change the lines:

 if unit:
        self.unit = unit
    else:
        self.unit = parent.unit

to

 if unit != parent.unit:
        self.unit = unit
    else:
        self.unit = parent.unit

it corrects the problem.