Add several new sensors for SolisCloud: batteryStateOfHealth, gridTotalEnergyPurchased, batteryCurrent, batteryVoltage

[jmason]

Also, reduce some duplicated code by extracting a method for the unit-conversion logic.

Includes a fix for https://github.com/hultenvp/solis-sensor/issues/158 .

[viking2010]

@jmason you beat me to adding in this pull request! I have attached the .py files I've edited in the zip file here. solis_integration.zip

If you want to look over the files and add them to yours, please feel free. In the zip file there is also a change log of the things I'd altered. I've not had any issues with any of the changes I've made.

I am aware however that having moved the Battery Power to the Station Detail from Inverter Detail, this causes issues for those who have multiple plants. @hultenvp has already commented on this in other posts and was looking at InverterDay as a possible alternative. I'm not sure how that can be used as it exports all entries every 5 mins for a 24 hr period, but then my Python skills are not as great as I'm sure yours are! :-)

For me the changes work, and I'm happy that they've not caused any issues elsewhere.

Cheers

[jmason]

@viking2010 yeah sorry-- I wound up needing some of the new fields in a hurry :)

I will take a look at the zip and try to merge the two. I think the batteryPower seems to work correctly in this current PR despite coming from the Inverter Detail API.

[viking2010]

No worries! The battery power does work, but the inverter detail API figure for power, for those with the newer data logger, shows as a positive figure regardless of whether its charging or discharging. When using station detail, it shows as a negative number when discharging and positive when charging. From that I have then created a template sensor to determine the charge status. If you have the old data logger (the one that doesn't have the 3 lights on the outside of the casing) the inverter detail shows both a pos and neg signed number. I think it's something to do with the fact the old logger data goes via solarman before hitting the Solis servers, whereas the new logger goes direct to solis.

[jmason]

Ah OK I get it now! I have one of the new data logger sticks, and my battery power is always positive -- so I was incorrect in stating that it was working correctly. It's batteryCurrent which is doing the right thing and going negative when discharging.

Maybe a fix would be to invert batteryPower when batteryCurrent is negative? that'd maintain the pos/neg behaviour described in https://github.com/hultenvp/solis-sensor/issues/158 without having to use stationDetail, so it'd work correctly when there are multiple plants.

[jmason]

I've updated to incorporate many of your changes; the only difference is to keep the battery state of health as batstateofhealth rather than batteryHealthSoh, as I think that's more consistent with the other existing battery attributes. I've also kept gridtotalenergypurchased rather than gridPurchasedTotalEnergy as again I think it's more consistent with the existing energypurchased sensors.

I'm not 100% on the motivation for changing gridtotalongridenergy from gWh to mWh -- is that necessary?

Other than that, I've incorporated these changes:

• Battery Power changed unit to kW rather than W

• added "Force Charge SOC" and "Force Discharge SOC"

• Turned off "correct_daily_on_grid_energy_enabled" (as that seems to be accurate for my system too)

[viking2010]

Hi

Happy with those changes. Thanks for keeping things consistent! I hadn't really had time to do more of a tidy up, so things were left working but not necessarily in keeping with existing code.

I'm not sure about the gridtotalongridenergy but I think that's an existing bit of code that changes the units used depending on the output from gWh to mWh.

With regards to the Battery Current, can you clarify something for me.....I thought that in the Soliscloud_api.py, the section in the square brackets in quotes needed to match the output from the API query. So I currently have this in my API InverterDetail output:

"bstteryCurrent": 1.3,
"bstteryCurrentStr": "A",

So in the soliscloud_api.py, I entered BAT_CURRENT: ['bstteryCurrent', float, 1], and yes, that's not a typo on my part but on Solis' part, and I therefore added it in with the typo to match what was in the api output.

....but in your soliscloud_api.py, you have it listed as BAT_CURRENT: ['storageBatteryCurrent', float, 2],

Does this mean it doesn't have to match the output from the api query? More than happy to stand corrected! 😄 After all, I'm only just getting to grips with all this Python coding!!

Great work and thank you!

[viking2010]

Actually, one other thing I did change and didn't let you know about was the timing of data refreshes. In service.py I changed the following:

# REFRESH CONSTANTS
SCHEDULE_OK = 1 #Changed from 2 to 1
SCHEDULE_NOK = 1

I have done this as I have 1 minute refresh intervals on my data from Solis.

[jmason]

Oh yeah -- if the source data has 1-minute resolution we should totally poll at that rate, sounds sensible! @hultenvp does that make sense to you?

[viking2010]

Oh yeah -- if the source data has 1-minute resolution we should totally poll at that rate, sounds sensible! @hultenvp does that make sense to you?

Thing is by default, it's 5 mins, but you can ask Solis very nicely if they'll reduce it. They might say no but I was recently testing their remote inverter control from the app/portal, so I mentioned about changing the update interval and they altered it. You could always poll the api for the datalogger to get the refresh rate which I think is listed in seconds to determine the integration refresh time. I say that, but I've no idea how to write that in anything other than pseudo code!

Also, thinking more about the battery power negative sign not being there for new data loggers in the InverterDetail API output, if you or @hultenvp have the Python skills to convert the positive value to a negative one based on the values coming from the battery current, then that would resolve that issue and we could continue using the InverterDetail rather than moving it to StationDetail or InverterDay. I'm not sure how to achieve that in Python, but I'm sure you folks have the skills for that!

[jmason]

Oh right, I see. I'm definitely seeing 5-minute data resolution in my output :( One thing however is that while the source data is being produced on a 5 minute schedule, polling every 1 minute means that I get the latest data point as soon as possible after it is made available on the backend, instead of waiting potentially up to 2 minutes, so that is still nice to have.

Regarding converting the positive value to negative based on the battery-current value -- yes, that's what I'm suggesting :) I can take care of that!

[viking2010]

@jmason one issue I have just spotted, is that my stats for gridtotalenergypurchased have just ticked over to 1 mWh, and as they've done so, it's now pooched the graphs. So I think it needs another conversion setting up like some of the others do, to change the unit depending on the value returned.

[jmason]

that's weird, it should handle that ok! what does the soliscloud_test script show for that value and its "str" value?

[jmason]

You could always poll the api for the datalogger to get the refresh rate which I think is listed in seconds to determine the integration refresh time.

looks like this is feasible -- the API docs for /v1/api/collectorDetail contains this field:

dataUpload Cycle | Int | Data Upload interval

[viking2010]

that's weird, it should handle that ok! what does the soliscloud_test script show for that value and its "str" value?

Lol, I restarted HA and thought it was working correctly and detecting it as MWh but I was looking at the wrong value Doh!!

So the API output is:

"gridPurchasedTotalEnergy": 1.018,
"gridPurchasedTotalEnergyStr": "MWh",

....so it is picking it up in the API correctly, but not translating that in to HA

I'll try and get some time this weekend to take a look at it and see if I can figure it out. In the meantime, if you or anyone else comes up with a fix for that, I'm happy to test it.

[jmason]

all I can think of is I might have made some basic python slip-up in https://github.com/hultenvp/solis-sensor/pull/227/files#diff-4a3e7a36513596fee6d969be8cee71c790d2fd9b4d62010cace0ed2f8d61d20eR401-R403 ? 😅

[hultenvp]

Hey there, bit busy at work, so had little time for hobby ;-)

Thanks for the PR, I'll review it in a minute.

Let me answer some of the questions floating in the discussion:

Oh yeah -- if the source data has 1-minute resolution we should totally poll at that rate, sounds sensible! @hultenvp does that make sense to you?

It does, but some people see a lot of 502 errors, looking like a load balancer tripping. I'm still investigating, but until that time I'd rather not increase the load on their systems if not required.

Maybe a fix would be to invert batteryPower when batteryCurrent is negative? that'd maintain the pos/neg behaviour described in https://github.com/hultenvp/solis-sensor/issues/158 without having to use stationDetail, so it'd work correctly when there are multiple plants.

That's actually very easy to fix implement. I'd prefer this as the solution. Inverterday is a nogo; too much data and stationDetail is only a workaround if you have a single inverter.

....but in your soliscloud_api.py, you have it listed as BAT_CURRENT: ['storageBatteryCurrent', float, 2], Does this mean it doesn't have to match the output from the api query? More than happy to stand corrected! 😄 After all, I'm only just getting to grips with all this Python coding!!

It should match the output, but perhaps both values are present in the output? I haven't checked myself, but best to follow the API 1.2 spec as you can find in the docs folder of this repo.

[hultenvp]

I did not see any changes regarding the batteryPower. I can make that fix in a separate PR

[jmason]

I did not see any changes regarding the batteryPower. I can make that fix in a separate PR

yes, good idea :)

[jmason]

I did not see any changes regarding the batteryPower. I can make that fix in a separate PR

yes, good idea :)

actually, I've gone ahead and implemented this now.

[jmason]

@hultenvp this should be safe enough for potential merging if you are happy with it :) I've pulled out the change to the workaround into a separate PR, https://github.com/hultenvp/solis-sensor/pull/231 .

update: actually, I forgot about https://github.com/hultenvp/solis-sensor/pull/227#issuecomment-1344866463 . that's still an open question. any news on that @viking2010 ?

[jmason]

It should match the output, but perhaps both values are present in the output? I haven't checked myself, but best to follow the API 1.2 spec as you can find in the docs folder of this repo.

oh, this is one more question I hadn't answered yet.

bstteryCurrent (with the typo) is not accurate for my account -- it doesn't seem to correctly match what the SolisCloud app or website reports for the battery activity -- it's way off (I wonder if there's some internal battery in the inverter that this reflects). storageBatteryCurrent however, does match up correctly. It's not ideal that storageBatteryCurrent appears to be undocumented in the 1.2 API docs though!

[viking2010]

all I can think of is I might have made some basic python slip-up in https://github.com/hultenvp/solis-sensor/pull/227/files#diff-4a3e7a36513596fee6d969be8cee71c790d2fd9b4d62010cace0ed2f8d61d20eR401-R403 ? 😅

Sorry I've not replied sooner but I came down with flu so not had the energy to even respond until now. With regards to the kWh/mWh I don't think this was from your code as I'm still using the original, so it's a bit weird. Looks like I've got some more messages to go through here too!!

[viking2010]

Hi,

Just with regards to some bits you answered earlier this week:

Let me answer some of the questions floating in the discussion:

Oh yeah -- if the source data has 1-minute resolution we should totally poll at that rate, sounds sensible! @hultenvp does that make sense to you?

It does, but some people see a lot of 502 errors, looking like a load balancer tripping. I'm still investigating, but until that time I'd rather not increase the load on their systems if not required.

I've actually reverted mine back to the 2 min duration now as I'm also getting timeouts. It works for a bit then as you say, it trips out, often ending up with a delay of 15 mins or so before refreshing the data. I find this a bit odd though as in the API 1.2 documentation, it says the "API interface calling frequency is limited to three times every five seconds for the same IP".

But, perhaps their systems aren't coping with all the calls being made?

Maybe a fix would be to invert batteryPower when batteryCurrent is negative? that'd maintain the pos/neg behaviour described in #158 without having to use stationDetail, so it'd work correctly when there are multiple plants.

That's actually very easy to fix implement. I'd prefer this as the solution. Inverterday is a nogo; too much data and stationDetail is only a workaround if you have a single inverter.

Sounds like a plan!!

....but in your soliscloud_api.py, you have it listed as BAT_CURRENT: ['storageBatteryCurrent', float, 2], Does this mean it doesn't have to match the output from the api query? More than happy to stand corrected! 😄 After all, I'm only just getting to grips with all this Python coding!!

It should match the output, but perhaps both values are present in the output? I haven't checked myself, but best to follow the API 1.2 spec as you can find in the docs folder of this repo.

In the API doc, it shows under InverterDetail:

There doesn't appear to be any other spelling format for this which is weird. It would seem to imply that the setting in the soliscloud_api.py file should be the misspelt version.

[viking2010]

ok....so I should have read all the messages then replied in one! lol :-D

That's very interesting then. As you say, StorageBatteryCurrent isn't in the documentation at all! In the API output, I'd only looked under the Battery section and hadn't seen the StorageBattery section, or if I did, I paid no attention to it lol

I therefore agree that the storage one is the correct one.

[hultenvp]

Merging into rel 3.2.0

[hultenvp]

Thanls @jmason and @viking2010: Fixes are now in. I made one change to harden the sign check on the bat power: if self._data[BAT_CURRENT] < 0 and self._data[BAT_POWER] > 0: This ensures we do not revert sign if it was already negative.