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

Possible to merge changes from CanisUrsa/hass-sunpower to include Battery storage? #61

Closed
dlp688 opened this issue Apr 10, 2024 · 16 comments

Comments

@dlp688
Copy link

dlp688 commented Apr 10, 2024

Would be great to get the ESS work done by CanisUrsa merged into the main branch if possible.

@krbaker
Copy link
Owner

krbaker commented Apr 10, 2024

CanisUrsa fork <-- Just getting the link myself.

@krbaker
Copy link
Owner

krbaker commented Apr 10, 2024

Oh fun, this is conflict city ... Looking at the overall changes I think they are reasonable to bring in but needs some work ( @CanisUrsa thoughts? ). Overall

  • I think the flag to enable ESS makes sense.
  • I do think were pushing a bit much into the fetch routine (mapping all of the fields for ESS, though I was already doing a bit more than I wanted with mine).
  • I need to look over making the Entities more generic.. I think its cleaner, just need to make sure it doesn't break anything else.
  • It does appear to re-key entities which I've really tried to avoid (I think if I pulled it the way it is it might loose historical data, dashboards etc)

@krbaker
Copy link
Owner

krbaker commented Apr 17, 2024

@dlp688 please read the notes and try out https://github.com/krbaker/hass-sunpower/releases/tag/2024.4.2 (or let me know why not here). I've merged the changes and cut a pre-release (note you have to have HACS set for betas to get it). I don't have an ESS and the testing on this project isn't great just yet. THANKS! My goal is to cut a full release by end of the week.

@dlp688
Copy link
Author

dlp688 commented Apr 17, 2024

Hey @krbaker I just tried to enable the new beta release and it does not appear to be pulling any entities at all when the 'use ESS' checkbox is enabled. once I uncheck the box, it is able to grab the entities immediately

@krbaker
Copy link
Owner

krbaker commented Apr 17, 2024

Can you search for 'sunpower' in your logs and send any errors?

Can you confirm which entities its fetching without ESS? is it fetching everything including ESS?

I'll take a read over the code and see if I can figure it out too.

@dlp688
Copy link
Author

dlp688 commented Apr 17, 2024

The full logs are ~400MB so here is a snippet of what is in there, it seems to be mostly these, or very similar to them

2024-04-17 08:41:53.692 ERROR (MainThread) [custom_components.sunpower] Unexpected error fetching SunPower PVS data: 'ESS BMS'
Traceback (most recent call last):
File "/usr/src/homeassistant/homeassistant/helpers/update_coordinator.py", line 315, in _async_refresh
self.data = await self._async_update_data()
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/usr/src/homeassistant/homeassistant/helpers/update_coordinator.py", line 271, in _async_update_data
return await self.update_method()
^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/config/custom_components/sunpower/init.py", line 286, in async_update_data
return await hass.async_add_executor_job(
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/usr/local/lib/python3.12/concurrent/futures/thread.py", line 58, in run
result = self.fn(*self.args, **self.kwargs)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/config/custom_components/sunpower/init.py", line 243, in sunpower_fetch
data.update(convert_ess_data(ess_data, pvs_serial))
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/config/custom_components/sunpower/init.py", line 75, in convert_ess_data
data[BATTERY_DEVICE_TYPE][device["serial_number"]]["battery_amperage"] = device[
~~~~^^^^^^^^^^^^^^^^^^^^^
KeyError: 'ESS BMS'
2024-04-17 08:41:53.778 ERROR (MainThread) [custom_components.sunpower] Unexpected error fetching SunPower PVS data: 'ess_report'
Traceback (most recent call last):
File "/usr/src/homeassistant/homeassistant/helpers/update_coordinator.py", line 315, in _async_refresh
self.data = await self._async_update_data()
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/usr/src/homeassistant/homeassistant/helpers/update_coordinator.py", line 271, in _async_update_data
return await self.update_method()
^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/config/custom_components/sunpower/init.py", line 286, in async_update_data
return await hass.async_add_executor_job(
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/usr/local/lib/python3.12/concurrent/futures/thread.py", line 58, in run
result = self.fn(*self.args, **self.kwargs)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/config/custom_components/sunpower/init.py", line 243, in sunpower_fetch
data.update(convert_ess_data(ess_data, pvs_serial))
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/config/custom_components/sunpower/init.py", line 74, in convert_ess_data
for device in ess_data["ess_report"]["battery_status"]:
~~~~~~~~^^^^^^^^^^^^^^

@krbaker
Copy link
Owner

krbaker commented Apr 17, 2024

Ahhh... I didn't realize the way the ESS conversion was working. The PVS section of my code actually creates some of the dict entries that are then used by the ESS code. Working on a patch.

@krbaker
Copy link
Owner

krbaker commented Apr 17, 2024

if you can grab the raw output of your ESS data and PVS data that would be awesome (feel free to change the serial numbers, just keep them unique and consistent between files). If you can make a PR putting them in the samples directory I can at least start testing against something :-)

@krbaker
Copy link
Owner

krbaker commented Apr 17, 2024

https://github.com/krbaker/hass-sunpower/releases/tag/2024.4.3 <-- Probably fixes that problem (I'll start testing when I have data to test against)

@dlp688
Copy link
Author

dlp688 commented Apr 17, 2024

@krbaker
Copy link
Owner

krbaker commented Apr 17, 2024

Have you happened to give it a test?

@dlp688
Copy link
Author

dlp688 commented Apr 17, 2024

2024.4.3 seems to have resolved the issue - I can collect data now!

I have no idea how to setup the reinman integrations to make that part work however.

I am also noticing that the customer state of charge here shows 66% where the sunpower app shows 59% not sure if there is some conversion going on that's not working?
image

@krbaker
Copy link
Owner

krbaker commented Apr 17, 2024

The Virtual entry is basically taking all of the individual battery measurements, adding them up and dividing by the number of batteries. I could see if maybe the batteries are different sizes or something it gets this wrong? I'm actually wondering if the refresh rate of sunpower is just slow (7% is quote a lot though). I've seen 15 min lag in Sunpower which is about 1250kwh at that (5kw).. Do you have ~11KW of capacity or less?

I'm a little surpised to see the error state... but now that I look at your example data the errors are in there too.

This looks like its working... If you want to try and validate the SOCs you can grab an ESS dump and sum the data then divide by the number of batteries.
"customer_state_of_charge": {
"unit": "%",
"value": 55.000000000000007
},

@dlp688
Copy link
Author

dlp688 commented Apr 17, 2024

I have a first gen ESS, so its a single BMS with an extra battery enclosure. It contains 4 separate battery modules, but as far as I can tell, the BMS just sees one big pack. Its a 26kwh system.

@krbaker
Copy link
Owner

krbaker commented Apr 18, 2024

In the PVS output I see.. 4 batteries (though little data about each one, no current charge etc)
{
"ISDETAIL": true,
"SERIAL": "M111111111111",
"TYPE": "BATTERY",
...
}, {
"ISDETAIL": true,
"SERIAL": "M111111111112",
"TYPE": "BATTERY",
...
}, {
"ISDETAIL": true,
"SERIAL": "M111111111113",
"TYPE": "BATTERY",
...
}, {
"ISDETAIL": true,
"SERIAL": "M111111111114",
"TYPE": "BATTERY",
...
},

And the ESS (with the overall capacity)
{
...
"DEVICE_TYPE": "Energy Storage System",
"hw_version": "0",
"interface": "none",
"operational_ac_kW": 6,
"operational_ac_kWh": 21.96,
"rated_ac_kW": 6.8,
"rated_ac_kWh": 26,
...
},

Then I see only ESS data in the ess dump.. and that gives the live % full but only for the whole system (but interesting that it is returning one item in a list)

	"battery_status":	[{
			"battery_amperage":	{
				"unit":	"A",
				"value":	8.4
			},
			"battery_voltage":	{
				"unit":	"V",
				"value":	53.2
			},
			"customer_state_of_charge":	{
				"unit":	"%",
				"value":	55.000000000000007
			},
			"last_updated":	"2024-04-17 18:01:13",
			"serial_number":	"BC1111111111111111111",
			"system_state_of_charge":	{
				"unit":	"%",
				"value":	62
			},
			"temperature":	{
				"unit":	"C",
				"value":	29.8
			}
		}],

One question: can you look at your actual data? the 'Serial Number' in the ESS dump battery list is not something I see in the PVS dump. Did you keep the obfuscated serial numbers consistent? I would expect the ESS code to fail on this (the serial number for each battery has to be in the PVS output if I'm reading the code correctly). Besides that issue I would expect the 'Sunvault' reported value to be 55% customer / 62% system soc at the time this dump was made.

@krbaker
Copy link
Owner

krbaker commented May 20, 2024

closing the overall integration as done, feel free to open an follow up if there are data issues (this follows the requested branch behavior)

@krbaker krbaker closed this as completed May 20, 2024
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

When branches are created from issues, their pull requests are automatically linked.

2 participants