Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Request for aditional atributes #3

Closed
SergioBPereira opened this issue Sep 20, 2020 · 28 comments
Closed

Request for aditional atributes #3

SergioBPereira opened this issue Sep 20, 2020 · 28 comments

Comments

@SergioBPereira
Copy link

Hi,

First thanks for the work you made available, great stuff and very helpful on my HA setup.

I do have a battery attached to the inverter and would like to have its values on the add-on. I've checked the Interface Definitions and the relevant modbus registries seem to be:

  • 56 [Energy storage unit 1] Running status

  • 57 [Energy storage unit 1] Charge and discharge power

  • 58 [Energy storage unit 1] Current-day charge capacity

  • 59 [Energy storage unit 1] Current-day discharge capacity

  • 85 to 96 seem to not be necessary as they are more about battery specifications and configuration

    Can you add those to the helper class and to the HA add-on?

Regards,

@Emilv2
Copy link
Owner

Emilv2 commented Oct 7, 2020

I'll add them later this week or next week.

@SergioBPereira
Copy link
Author

Bumping this. Thanks again.

@Emilv2
Copy link
Owner

Emilv2 commented Feb 14, 2021

I've added some battery attributes. Tell me if it works, I don't have a battery to test.

@VaR63
Copy link

VaR63 commented Feb 14, 2021

I am so happy to see that you added the battery data management. Thank you so much!
I immediately tried the revised version but unfortunately the configuration check fails (invalid option "battery: true" on huawei_solar....) and the "sensor.sun2000l_5ktl" entity disappear.
I turned on the old configuration an it run perfectly.
Have you some idea about the reason?

@Emilv2
Copy link
Owner

Emilv2 commented Feb 15, 2021

Are you sure you updated the sensor files? you will need to update the file custom_components/huawei_solar/sensor.py and custom_components/huawei_solar/manifest.json

@VaR63
Copy link

VaR63 commented Feb 15, 2021

Yes, I replaced the whole files set.

@Emilv2
Copy link
Owner

Emilv2 commented Feb 15, 2021

And did you restart HA? I don't really get that error since the option is right there:

CONF_OPTIMIZERS = "optimizers"
CONF_BATTERY = "battery"

...

PLATFORM_SCHEMA = PLATFORM_SCHEMA.extend(
    {
        vol.Required(CONF_HOST): cv.string,
        vol.Optional(CONF_OPTIMIZERS, default=False): cv.boolean,
        vol.Optional(CONF_BATTERY, default=False): cv.boolean,
    }
)

I also tried adding battery: true and I don't get that error.

@VaR63
Copy link

VaR63 commented Feb 15, 2021

I cannot retry now, I will retry soon and I will update you.
Thank you for your replies. I would be happy to offer a beer for your awesome integration, how can I do it?

@VaR63
Copy link

VaR63 commented Feb 15, 2021

I tried newly.
I deleted any previous file in config/custom_components/huawei_solar/ and I copied the unzipped files from https://github.com/Emilv2/huawei_solar
I added the battery related raw on configuration.yaml:
sensor:

  • platform: huawei_solar
    host: '192.168.1.22'
    battery: true
    I rebooted home assistant server.
    This error appears in the log:

Logger: homeassistant.components.hassio
Source: components/hassio/init.py:420
Integration: Hass.io (documentation, issues)
First occurred: 20:09:05 (1 occurrences)
Last logged: 20:09:05

Invalid config for [sensor.huawei_solar]: [battery] is an invalid option for [sensor.huawei_solar]. Check: sensor.huawei_solar->battery. (See ?, line ?).

The same error is showed if I perform the configuration check.
Yesterday the SUN2000L-5ktl disappeared, today the inverter continue to show all datas.
Even if obvious, I checked the attribute list on developer tools and no battery related attributes are listed.

@Emilv2
Copy link
Owner

Emilv2 commented Feb 15, 2021

There's something strange going on if it still shows all the data when the config is supposedly invalid. Maybe some stale information somewhere in .storage?

@Emilv2
Copy link
Owner

Emilv2 commented Feb 16, 2021

What if you make a clean development environment like explained in the docs? Do you get the same error there or does it work?

@VaR63
Copy link

VaR63 commented Feb 17, 2021

Unfortunately I am not a developer, I am not able to do what you ask in your last message.
I will try to check the . storage file first. In case of no useful news, I will try to find a developer able to help me with the clean development environment test.

@VaR63
Copy link

VaR63 commented Feb 17, 2021

I checked in the .storage dir and I do not see any filename that seems related to your integration.
Should I check something into some file?

@Emilv2
Copy link
Owner

Emilv2 commented Feb 17, 2021

It would be inside one of the files. You could delete the whole folder if you don't care about stored logins, configurations configured with configflow, and any other configuration done through the UI being removed.

You could also try to rename the custom component and see what happens. move it to /custom_compontents/huawei_solar2, renam all occurrences of huawei_solar to huawei_solar2 in manifest.json and the same in your configuration.yaml

@VaR63
Copy link

VaR63 commented Feb 17, 2021

ok, I will perform some tests as soon as I will be able. Within a couple of days, I think.

@VaR63
Copy link

VaR63 commented Feb 17, 2021

I tred to rename the custom component /custom_compontents/huawei_solar2, renamed all occurrences of huawei_solar to huawei_solar2 in manifest.json and in my configuration.yaml.
The sensor,sun2000L-5ktl disappeared.

@Emilv2
Copy link
Owner

Emilv2 commented Feb 17, 2021

:/ Any errors in the logs?

@VaR63
Copy link

VaR63 commented Feb 18, 2021

I performed a lot of checks. All run correctly, even with the last integration release, as long as the battery option is set to -false- or commented.
With - battery: true - the inverter entity become unavailable and no errors are logged.
This happens both with or without renaming huawei_solar to huawei_solar2.
(dir name to /custom_compontents/huawei_solar2 and occurrencies in manifest.json and configuration.yaml).
I suppose that the renaming into manifest.json file should be
{
"domain": "huawei_solar2",
"name": "huawei_solar2",
"documentation": "",
"dependencies": [],
"codeowners": ["Emilv2"],
"requirements": ["huawei-solar> = 0.1.0"]
}
However, since my so poor skill, i also tried to rename the last line
"requirements": ["huawei-solar2> = 0.1.0"]
in this case an error is logged and obviously the integration still does not work.

@Emilv2
Copy link
Owner

Emilv2 commented Feb 18, 2021

Ok, if there are no errors with battery: true then that means you got the new version and the issue is in my code.

I just checked and found a few issues, what happens now if you put the new sensor.py inside custom_compontents/huawei_solar2?

@VaR63
Copy link

VaR63 commented Feb 18, 2021

Same behavior, does not run with battery: true and no error logged.

@Emilv2
Copy link
Owner

Emilv2 commented Mar 2, 2021

There were some other issues with the battery part fixed over the weekend, does it work with the last version?

@VaR63
Copy link

VaR63 commented Mar 2, 2021

I replaced the file sensor.py and I uncommented the battery: true on configuration.yaml.
The sensor.sun2000L-5ktl disappeared yet.
If it is required to replace all integration files, let me know.
I am sorry for your trouble, if I could help you in some manner, let me know.

@SergioBPereira
Copy link
Author

I've tried the cchanges and am having some dificulties. From what i could test the sensor times out and HA gives up on it:
`Logger: homeassistant.components.sensor
Source: helpers/entity_platform.py:232
Integration: Sensor (documentation, issues)
First occurred: 9:58:21 AM (1 occurrences)
Last logged: 9:58:21 AM

Setup of platform huawei_solar is taking longer than 60 seconds. Startup will proceed without waiting any longer.`

I've instanciated the helper on python command line with
inverter = HuaweiSolar("my inv ip")
and managed to connect and even propreties from get(). Tried "model_name" and "storage_status" and got my expected values. But there is a delay of around 10 seconds on each call to get(). So i am assuming the timeout is coming from this delay as the sensor calls this get() several times.

Please tell me if i can help in anyway, and thanks again.

@SergioBPereira
Copy link
Author

SergioBPereira commented Mar 31, 2021

I've taken my tests a bit further and managed to make a working version by adjusting def read_register(self, register, length) on the helper class. After some test, i came to the conclusion that the helper would allways fail the second call after the first opening call, but with no clue on why. I've made some searchs on pymodbus issues related but the threads I manage to find you were actively engaged on already.

I adjusted read_register by imposing a wait of 1 (seems to be enougth on my inverter, 0 fails miserably) and i also allways close the connection before returning so a open is forced on each call, with:

        self.client.close()
        self.connected = False
        return response.encode()[1:] 

Doing this (i just copied and edited the helper code inside the sensor code file) the sensor come up alive.

I still get warnings, but it doesnt get killed by HA:
Update of sensor.sun2000l_3ktl is taking over 10 seconds

Also tested the battery option. Had to adjust the following two lines on the helper REGISTERS:
"storage_power_charging_cutoff_capacity": RegisterDefinitions("u16", "%", 10, 47081, 1),
"storage_power_discharging_cutoff_capacity": RegisterDefinitions(
"u16", "%", 10, 47082, 1
and edited the def of attributes for the batteries, you were using ":" instead of "=" to append.

Sorry if i cant help more, i hardly know python and havent code for more than a decade. Tell me if more info is needed and if i can help.

@SergioBPereira
Copy link
Author

One more adjustment on the helper class get method:

        elif reg.type == "i16" and reg.unit == "storage_status_enum":
            tmp = int.from_bytes(response, byteorder="big")
            if (tmp & 0x8000) == 0x8000:
                # result is actually negative
                tmp = -((tmp ^ 0xFFFF) + 1)
            result = STORAGE_STATUS_DEFINITIONS.get(tmp, "unknown/invalid")

from u16 to i16

@Emilv2
Copy link
Owner

Emilv2 commented Apr 1, 2021

@SergioBPereira thanks for your help! I'm a bit busy right now so I can't put much time into it but I'll take a look at your fixes.

I adjusted read_register by imposing a wait of 1 (seems to be enougth on my inverter, 0 fails miserably) and i also allways close the connection before returning so a open is forced on each call, with:

I don't remember exactly what wait time I needed, I think I was trying to be conservative with 2.

IIRC closing the connection every time makes every single request slow instead of only the first one, is your experience different?

from u16 to i16

I think it's this one that should be changed to u16:

"storage_status": RegisterDefinitions("i16", "storage_status_enum", 1, 37000, 1)

"storage_power_charging_cutoff_capacity": RegisterDefinitions("u16", "%", 10, 47081, 1),
"storage_power_discharging_cutoff_capacity": RegisterDefinitions(
"u16", "%", 10, 47082, 1
and edited the def of attributes for the batteries, you were using ":" instead of "=" to append.

Oops, too much copy-pasting. I'll fix those.

@SergioBPereira
Copy link
Author

SergioBPereira commented Apr 1, 2021

I don't remember exactly what wait time I needed, I think I was trying to be conservative with 2.

Yes, it was 2 by default.

IIRC closing the connection every time makes every single request slow instead of only the first one, is your experience different?

It would allways return 0 bytes on responding to the second call, resulting in the fallback you coded of closing and reopening. So in practive was taking even more time than just closing and reopening each time. As i could reduce the wait from 2 seconds to 1 and get away with it, i use this as a quick temporary fix to get things going. I also disabled some of the attributes i dont need so it gets a liittle faster at updating than would otherwise.
Anyway, i think there is a problem with pymodbus comunicating with the inverter, i had this working without any visible probems for months until some recent upgrade on HA.

from u16 to i16
I think it's this one that should be changed to u16:

"storage_status": RegisterDefinitions("i16", "storage_status_enum", 1, 37000, 1)

you should be probably right ofc, i dindt check the API Reference to check. It's a positive number so the conversion is :) and the attribute gets its state literal.

Again thanks for the help and awesome integration, i've learned a lot trying to help about how sensors work etc. Ill keep my costum version until you have the time to implement the fixes, happy both ways. I'll keep an eye out for when you commit any update and help to test it.

Regards,

@VaR63
Copy link

VaR63 commented Apr 11, 2021

Dear @SergioBPereira,
It would like so much to use the battery related data to improve my automation but I do not know pyton.
Could you kindly share your revised version while waiting for the @Emilv2 revision, please?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants