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

Total yield in two registers? #76

Open
rupertnash opened this issue Jul 17, 2022 · 9 comments
Open

Total yield in two registers? #76

rupertnash opened this issue Jul 17, 2022 · 9 comments

Comments

@rupertnash
Copy link
Contributor

While looking at #75, I realised that if the data we're getting from the inverter is basically the same as the MODBUS data, then each datum is a 16 bit register value. Since the energy yield is in units of 0.1 kWh, that means the max yield it can respond with is 6,553.5 kWh - for my system that's probably about 1 year's generation

So - my question is: is the yield actually split over 2 data items? I see that the next item in the data array is currently zero for me. Similarly for the total feed-in energy and total consumption.

I just looked in the code of the Solax MODBUS thing (https://github.com/wills106/homeassistant-solax-modbus/blob/784153e6a0039402335026281ddca9aadc532a67/custom_components/solax_modbus/__init__.py#L802) and they clearly believe this to be the case.

If this is the case, then we will need to alter the response -> data code to cope with values that come from multiple registers.

rupertnash added a commit to rupertnash/solax that referenced this issue Jul 17, 2022
@VadimKraus
Copy link
Contributor

I think it's already covered using this post processing method

def resetting_counter(value, mapped_sensor_data, key, adjust,

@rupertnash
Copy link
Contributor Author

Ah - sorry I hadn't seen that - thanks for flagging for me. The choice of name ("reset") doesn't really reflect the way I was thinking about it (just encoding a 32 bit val over 2 16bit words). Those functions are only used in inverters I haven't just for my cribbing...

I implemented a solution already yesterday (rupertnash@21a2d19)

If project leads want to keep using the existing code, that's cool otherwise I could also adapt qvolt_hyb_g3_3p.py and x3_v34.py to use my proposed code.

@rupertnash
Copy link
Contributor Author

I think the resetting_counter has an off-by-one error.

If there has been 1 reset/overflow of the least significant word, and the sensor LSW is zero, then the result should be 65,536. (Unless the inverter is doing something weird with its encoding of data, which is always possible...)

rupertnash added a commit to rupertnash/solax that referenced this issue Jul 18, 2022
rupertnash added a commit to rupertnash/solax that referenced this issue Jul 18, 2022
@rupertnash
Copy link
Contributor Author

Can anyone with an inverter that's been running long enough please confirm this by checking against the SolaX cloud view?

@VadimKraus
Copy link
Contributor

You are totally right. I have one with 8 "resets" there is an offset of 8 between cloud and this lib!

I vote in favor of your solution.

@rupertnash
Copy link
Contributor Author

Great- thanks for checking. I’ve made a PR (#77) which includes this feature. Feel free to separate that out and merge separately (if you have the permission lol)

@VadimKraus
Copy link
Contributor

btw. to_signed is probably also one off

> import struct
> print(struct.unpack("h",struct.pack("<H",40000)))
(-25536,)
> print(to_signed(40000))
-25535

@rupertnash
Copy link
Contributor Author

OK - updated #77 with a fix and unit test

rupertnash added a commit to rupertnash/solax that referenced this issue Aug 1, 2022
rupertnash added a commit to rupertnash/solax that referenced this issue Aug 1, 2022
squishykid added a commit that referenced this issue Sep 11, 2022
* Per issue #76, combine multiple register values

* #76: replace resets with proposed combiner and fix tests for off-by-one error

* fix off-by-one in to_signed and add test for it

* Add missing requirement async-timeout and silence warning in it's use

* Add timeout for connections since on most recent pocket wifi post hangs

* add support for X1 Hybrid G4

* ensure inverters raise if they get an error response from endpoint

* add testing for X1 Hybrid G4

* Satisfy cov for new inverter code

* remove (unused) bare index from ResponseDecoderType and assoicated code

* transpose ResponseDecoderType

* Update type annotations for multi-register indices

* pin async_timeout version

* protocol to define bit packer

* appease linter

* remove timeout() from this PR

* fix ha interface via sensor_map

Co-authored-by: Robin Wohlers-Reichel <robin.wohlersreichel@gmail.com>
@VadimKraus
Copy link
Contributor

This was fixed in #77, right?

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

2 participants