Separate sensors (issue #89) #91

TimSoethout · 2023-11-03T19:19:27Z

@L-four, just a quick PR with your fork.

TimSoethout · 2023-11-03T19:21:53Z

I don't have too much time to look at it yet, but at first glance it seems we can do some code deduplication. :)

…oints. Update manifest.json with iot_class and integration_type. Change version to 4.0.0

L-four · 2023-11-04T05:44:31Z

I bumped the version to 4.0.0 to reflect the fact we are removing the attributes and as such will break any templates, etc using the attributes.

f00f · 2023-11-06T09:50:48Z

custom_components/sems/sensor.py

+        if self._empty_value is not None and self._empty_value == value:
+            value = 0


I receive None values for some attributes, e.g. ['inverter', 0, 'invert_full', 'vpv3']. They cause an unhandled exception, so all updates of values failed. My quick fix is to detect None and treat it as 0. You may instead consider a try-catch around the conversion.

Suggested change

if self._empty_value is not None and self._empty_value == value:

value = 0

if self._empty_value is not None and self._empty_value == value:

value = 0

if None == value:

_LOGGER.debug(f"Value was 'None': {self._value_path}")

value = 0

f00f · 2023-11-06T10:01:17Z

When configuring the Energy dashboard, I don't see sensors for battery in the dropdown list.
Is the sensor simply missing from this PR?
Does the api not provide the data? Can I build a custom sensor from available data?

L-four · 2023-11-07T00:04:33Z

custom_components/sems/sensor.py

+        batteries = [
+                Sensor(
+                    coordinator,
+                    device_info,
+                    f"{serial_number}-vbattery1",
+                    f"Inverter {inverter['name']} Battery Voltage",
+                    path_to_inverter + ["vbattery1"],
+                    Decimal,
+                    SensorDeviceClass.VOLTAGE,
+                    ELECTRIC_POTENTIAL_VOLT,
+                    SensorStateClass.MEASUREMENT,
+                ),
+                Sensor(
+                    coordinator,
+                    device_info,
+                    f"{serial_number}-ibattery1",
+                    f"Inverter {inverter['name']} Battery Current",
+                    path_to_inverter + ["ibattery1"],
+                    Decimal,
+                    SensorDeviceClass.CURRENT,
+                    ELECTRIC_CURRENT_AMPERE,
+                    SensorStateClass.MEASUREMENT,
+                ),
+            ]


@f00f

When configuring the Energy dashboard, I don't see sensors for battery in the dropdown list. Is the sensor simply missing from this PR? Does the api not provide the data? Can I build a custom sensor from available data?

I don't have a battery so I am making some assumptions about what these values will look like, if you could provide the API response JSON you are getting from the SEMS portal we can improve this. Make sure you don't include any personal info the start of the JSON normally contains your name ect.

IsaacInsoll · 2023-11-07T04:42:13Z

This looks great to me (as somebody who just started using this integration yesterday).
Would it be possible for somebody to handle the "null homekit serial" issue in this same release?
#92 (comment)

L-four · 2023-11-07T09:18:43Z

This looks great to me (as somebody who just started using this integration yesterday). Would it be possible for somebody to handle the "null homekit serial" issue in this same release? #92 (comment)

Do you have a homekit installed?

Currently I am still using the homekit SN for backward compatibility.

f00f · 2023-11-07T11:04:01Z

Battery: here's the response I get from SEMS: https://gist.github.com/f00f/6eb432ca01b66827aa24a59c570e0d84
It's taken from the HA logs, so it's not valid JSON. Also, I replaced statistic values with 1234.

Homekit=None: I have no Homekit installed and also the sn=None issue. To me it's just cosmetic, but I haven't read #92, yet.

IsaacInsoll · 2023-11-07T11:06:15Z

Do you have a homekit installed?

I don't have a HomeKit Installed. I just gave the GM1000 smart meter which is connected to my inverter which is passing on the data. So my serial number is null.
This means when I set up the integration I got the two Incoming and Outgoing entities but didn't have the main one, due to the name being null so it didn't create a sensor. (you can see more in the linked issue)

Now that I've written the "if serial is null, make one up" i am getting the proper sensor data and everything is working correctly.

So adding those few lines solved all my problems

Account for None path values or missing paths. Add some tests.

L-four · 2023-11-09T12:54:50Z

custom_components/sems/sensor.py

+        battery_count = get_value_from_path(data, path_to_inverter + ["battery_count"])
+        if battery_count is not None:
+            for idx in range(0, battery_count):
+                path_to_battery = path_to_inverter + ["more_batterys", idx]
+                sensors += [
+                    SensorOptions(
+                        device_info,
+                        f"{serial_number}-{idx}-pbattery",
+                        f"Inverter {inverter['name']} Battery {idx} Power",
+                        path_to_battery + ["pbattery"],
+                        SensorDeviceClass.POWER,
+                        POWER_WATT,
+                        SensorStateClass.MEASUREMENT,
+                    ),
+                    SensorOptions(
+                        device_info,
+                        f"{serial_number}-{idx}-vbattery",
+                        f"Inverter {inverter['name']} Battery {idx} Voltage",
+                        path_to_battery + ["vbattery"],
+                        SensorDeviceClass.VOLTAGE,
+                        ELECTRIC_POTENTIAL_VOLT,
+                        SensorStateClass.MEASUREMENT,
+                    ),
+                    SensorOptions(
+                        device_info,
+                        f"{serial_number}-{idx}-ibattery",
+                        f"Inverter {inverter['name']} Battery {idx} Current",
+                        path_to_battery + ["ibattery"],
+                        SensorDeviceClass.CURRENT,
+                        ELECTRIC_CURRENT_AMPERE,
+                        SensorStateClass.MEASUREMENT,
+                    ),
+                    SensorOptions(
+                        device_info,
+                        f"{serial_number}-{idx}-soc",
+                        f"Inverter {inverter['name']} Battery {idx} State of Charge",
+                        path_to_battery + ["soc"],
+                        SensorDeviceClass.BATTERY,
+                        PERCENTAGE,
+                        SensorStateClass.MEASUREMENT,
+                    ),
+                    SensorOptions(
+                        device_info,
+                        f"{serial_number}-{idx}-soh",
+                        f"Inverter {inverter['name']} Battery {idx} State of Health",
+                        path_to_battery + ["soh"],
+                        SensorDeviceClass.BATTERY,
+                        PERCENTAGE,
+                        SensorStateClass.MEASUREMENT,
+                    ),
+                    SensorOptions(
+                        device_info,
+                        f"{serial_number}-{idx}-bms_temperature",
+                        f"Inverter {inverter['name']} Battery {idx} BMS Temperature",
+                        path_to_battery + ["bms_temperature"],
+                        SensorDeviceClass.TEMPERATURE,
+                        TEMP_CELSIUS,
+                        SensorStateClass.MEASUREMENT,
+                    ),
+                    SensorOptions(
+                        device_info,
+                        f"{serial_number}-{idx}-bms_discharge_i_max",
+                        f"Inverter {inverter['name']} Battery {idx} BMS Discharge Max Current",
+                        path_to_battery + ["bms_discharge_i_max"],
+                        SensorDeviceClass.CURRENT,
+                        ELECTRIC_CURRENT_AMPERE,
+                        SensorStateClass.MEASUREMENT,
+                    ),
+                    SensorOptions(
+                        device_info,
+                        f"{serial_number}-{idx}-bms_charge_i_max",
+                        f"Inverter {inverter['name']} Battery {idx} BMS Charge Max Current",
+                        path_to_battery + ["bms_charge_i_max"],
+                        SensorDeviceClass.CURRENT,
+                        ELECTRIC_CURRENT_AMPERE,
+                        SensorStateClass.MEASUREMENT,
+                    ),
+                ]


I added the values from more_batterys that I think I know what they are.

If you know what the other values are I can try and add them

#{ "pbattery": 0.0, "vbattery": 205.1, "ibattery": 0.0, "battary_work_mode": 1, "battstrings": 4.0, "bms_status": 1, "bms_temperature": 23.0, "bms_discharge_i_max": 40.0, "bms_charge_i_max": 40.0, "bms_alarm_l": 0, "bms_warning_l": 0, "soc": 5.0, "soh": 100.0, "bmsbattstr": 4, "batteryprotocol": 1234, "bms_alarm_h": 0, "bms_warning_h": 0, "bmssoftwareversion": 0.0, "batteryhardwareversion": 0.0, "batttotalcharge": 0.0, "batttotaldisCharge": 0.0, "batterysnmain": "", "bms_alarm": 0, "bms_warning": 0 }

Cool! 👏

soc is State of Charge
soh is State of Health
vbattery is probably the voltage, bms_temperature the reported temperature.
I'd assume that one value may indicate the flow direction, i.e. charging, nothing, or discharging. Maybe that's battery_work_mode.
All the other values don't look very interesting, or look like they would not be supported by my battery.

Just realized that you already had all the ones I mentioned...

@f00f did you censor "batterysnmain" i think it might stand for Battery Serial Number Main

L-four · 2023-11-09T12:56:15Z

I haven't really tested the latest changes, might get a chance on the weekend.

f00f · 2023-11-09T15:55:41Z

I haven't really tested the latest changes, might get a chance on the weekend.

I have installed the latest version. Now all entities that I had are unavailable and I have a copy of them with suffix "_2", e.g.
before the update I had an entity named "sensor.homekit_state_of_charge";
now I additionally have "sensor.homekit_state_of_charge_2";
the old one is unavailable.

This breaks all dashboards etc. If you could make your last changes in a way that I get my old sensors back, that would be much appreciated. Otherwise, I definitely can scratch everything and start over.

L-four · 2023-11-17T08:52:59Z

I made it so the powerflow sensors are not related to the homekit sn. Some people don't have a home kit but still had powerflow stats. So I have made a change to check if you already have a homekit sensor and keep the old naming method if so. This should prevent the _2 named sensors being created.

IsaacInsoll · 2023-12-06T01:40:18Z

I made it so the powerflow sensors are not related to the homekit sn. Some people don't have a home kit but still had powerflow stats. So I have made a change to check if you already have a homekit sensor and keep the old naming method if so. This should prevent the _2 named sensors being created.

Thank you, I really appreciate that!

Try and handle bug in the goodwe api where it returns zero when close to midnight. @see TimSoethout#94

IsaacInsoll · 2024-01-27T08:24:53Z

I haven't contributed any code for this release but I have the "no serial number homekit" that I hacked in #92 ...
Let me know if you would like me to do any testing.
This is my first experience helping with a Home Assistant plugin so just give me some basic instructions (IE: I presume just use HACS to pull master then see if it all works)

L-four · 2024-01-27T08:29:35Z

I use the "Studio Code Server" addon and paste my code from Pycharm then restart home assistant.

IsaacInsoll · 2024-01-27T21:52:13Z

I use the "Studio Code Server" addon and paste my code from Pycharm then restart home assistant.

Sorry I meant the best process to this this branch/build before you guys release it, not how to develop my own modifications to it 😀

is that a rough ETA for this release?

L-four · 2024-01-27T23:47:10Z

I don't know how HACS works so am not sure whats required on that side of things. The code in this pull request is working for me. The main issue I am having is #94 which is ruining my energy dashboard.

@TimSoethout whats required to release to HACS?

TimSoethout · 2024-01-28T09:17:47Z

I think git tagging a commit with a version, updating the version in the manifest json and creating a GitHub release. Is is also possible to create a beta release of a branch. Both of which require some input from me. Local testing can be done without HACS by creating a development environment using HA's instructions. Or just dropping the files in your `custom_components` directory. 28 Jan 2024 00:47:32 Kyah Rindlisbacher ***@***.***>:

…

I don't know how HACS works so am not sure whats required on that side of things. The code in this pull request is working for me. The main issue I am having is #94[#94] which is ruining my energy dashboard. @TimSoethout[https://github.com/TimSoethout] whats required to release to HACS? — Reply to this email directly, view it on GitHub[#91 (comment)], or unsubscribe[https://github.com/notifications/unsubscribe-auth/AAEQZ3CVPXCVDO4CG252KP3YQWGYVAVCNFSM6AAAAAA644A5OOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSMJTGM3TAMBWGA]. You are receiving this because you were mentioned. [Tracking image][https://github.com/notifications/beacon/AAEQZ3BIF64YTHZQC6V4KNLYQWGYVA5CNFSM6AAAAAA644A5OOWGG33NNVSW45C7OR4XAZNMJFZXG5LFINXW23LFNZ2KUY3PNVWWK3TUL5UWJTTSBO24Y.gif]

"buy" chart is also bugged around midnight.

L-four · 2024-02-07T10:07:00Z

My data is looking better with the fix ignoring data around 12pm at night. we don't lose and sell data (because no sun at night).
But we do lose some buy data...

davidchristen · 2024-02-18T18:35:36Z

I run the PR code on my Homeassistant. It works well in general, good performance in general. I have one issue: sensor.homekit.grid is always positive and does not change sign depending on if the power is imported or exported. Is there an easy way to fix?

robinhood-code · 2024-03-07T00:02:16Z

really looking forward to this PR is there further updates please?

lacona20 · 2024-03-09T18:29:52Z

sems

Brumhilde · 2024-03-15T11:04:02Z

I run the PR code on my Homeassistant. It works well in general, good performance in general. I have one issue: sensor.homekit.grid is always positive and does not change sign depending on if the power is imported or exported. Is there an easy way to fix?

GridStatus should be used to determine sign.

Brumhilde · 2024-03-15T11:14:13Z

Would it be possible to remove "HomeKit" naming from Home Assistant? With the big changes in this PR it will likely break backwards compatibility for most users anyway.

I suggest remove HomeKit device and naming from HA and add all sensors to inverter device. The HomeKit naming is confusing and I guess most users have a non HomeKit smart meter.

isHomKit from powerflow data should be used to determine if HomeKit sn should be set (if HomeKit sn is required at all?).

Brumhilde · 2024-03-15T14:37:39Z

custom_components/sems/sensor.py

+                ]
+
+    if "hasPowerflow" in data and data["hasPowerflow"] and "powerflow" in data:
+        inverter_serial_number = get_home_kit_sn(data)


Here you should add check for data["powerflow"]["isHomKit"]. If it is false there is no HomeKit sn and name.

This falls through to the next statement if you don't have a homekit and we just call it powerflow

if not has_existing_homekit_entity or inverter_serial_number is None: inverter_serial_number = "powerflow"

But that will still create the homekit device right? It is confusing.

Brumhilde · 2024-03-15T14:47:01Z

custom_components/sems/sensor.py

+                device_info,
+                f"{inverter_serial_number}-grid",
+                f"HomeKit Grid",
+                ["powerflow", "grid"],


gridStatus should be used to determine buy/sell.

Do you know what the possible values of gridStatus are and what they mean?

gridStatus seems to not work, it is always 1 for me. I have pmeter in the inverter device though and that shows same value as grid from powerflow but with sign. pmeter is negative when buying and positive when selling. Note that I don't have homekit smart meter, I have GM3000 smart meter.

Brumhilde · 2024-03-15T14:47:40Z

custom_components/sems/sensor.py

+                device_info,
+                f"{inverter_serial_number}-battery",
+                f"HomeKit Battery",
+                ["powerflow", GOODWE_SPELLING.battery],


betteryStatus should be used to determine charging/discharging

Do you know what the possible values of betteryStatus are and what they mean?

-1 = Discharging
0 = Off (100% soc and PV production for example)
1 = Charging

So we do

batteryStatus = 0 battery = 10 // kwh return battery * batteryStatus // 0

batteryStatus = 1 battery = 10 // kwh return battery * batteryStatus // 10

batteryStatus = -1 battery = 10 // kwh return battery * batteryStatus // -10

if we do the same for grid it should also work for you where it's always 1 but your grid value includes the sign
e.g.

gridStatus = 1 grid= -10 // kwh return battery * batteryStatus // -10

Signed-off-by: David Christen <[email protected]>

… home

Use new constants for units

L-four · 2024-03-26T09:10:29Z

@Brumhilde can you test changes to the sign of load, grid for you use case.

katzfiel69 · 2024-04-01T02:30:04Z

I run the PR code on my Homeassistant. It works well in general, good performance in general. I have one issue: sensor.homekit.grid is always positive and does not change sign depending on if the power is imported or exported. Is there an easy way to fix?

GridStatus should be used to determine sign.

Just started my venture into HA after getting solar and this is my first integration so apologies if this is an obvious answer.

I have my grid sensor that is also positive. I can see that grid status swings between positive and negative as per buying/selling from the grid. But how can I get a negative result to display somewhere in a sensor so I can use it for a live dashboard?

robinhood-code · 2024-05-22T10:56:05Z

any update on this PR please?

TimSoethout · 2024-05-23T06:30:58Z

any update on this PR please?

Sorry, no update from me due to lack of time.

L-four · 2024-06-01T01:59:08Z

This branch currently fulfills my use case, so is done from my perspective.

TimSoethout · 2024-06-20T05:41:59Z

Great. Unfortunately I don't have time to review the branch properly.
I can however create a beta release if that is handy for you. Then it should be downloadable in HACS with the beta toggle.

robinhood-code · 2024-08-20T09:26:32Z

any news for the beta release please?

TimSoethout · 2024-08-27T19:07:23Z

any news for the beta release please?

It seems I can't create a release since the branch is not on my repo. And I currently don't have time/energy to look into it more.

You can always manually deploy the files in your custom_components folder similar to the manual install in the readme.

L-four added 5 commits October 18, 2023 20:59

A bunch of updates.

16b1a6c

move everything to sensors.

2d72695

Formatting

9fd794a

Compat

a7ae2fb

Fix typo in Grid ac voltage. sensor name.

fdf54fe

TimSoethout changed the title ~~Separate sensors~~ Separate sensors (issue #89) Nov 3, 2023

L-four added 4 commits November 4, 2023 16:09

Use v3 of the GetMonitorDetailByPowerstationId api for more decimal p…

e41941e

…oints. Update manifest.json with iot_class and integration_type. Change version to 4.0.0

Refactor sensor instantiation

6c748ef

Fix manifest.json conflict

9add1d2

Merge branch 'master' into master

037203a

The json import is required. My IDE lied to me.

300d1b3

f00f reviewed Nov 6, 2023

View reviewed changes

f00f mentioned this pull request Nov 6, 2023

Feedback on moving from extra_state_attributes to sensors. #89

Open

TimSoethout mentioned this pull request Nov 6, 2023

Expose "powerflow" metrics: real time power import / export in watts #92

Closed

L-four reviewed Nov 7, 2023

View reviewed changes

L-four added 2 commits November 9, 2023 23:19

Refactor computing sensors to facilitate testing.

571df4b

Account for None path values or missing paths. Add some tests.

Add more_batterys

4bb29f4

L-four reviewed Nov 9, 2023

View reviewed changes

Check for existing homekit, for backwards compatibility.

332e2e2

Fix bug with detecting existing homekit_entity

1471caf

Try and handle bug in the goodwe api where it returns zero when close to midnight. @see TimSoethout#94

datetime.now() is 24hour time.

a624c1b

"buy" chart is also bugged around midnight.

Brumhilde reviewed Mar 15, 2024

View reviewed changes

davidchristen and others added 4 commits March 20, 2024 10:32

Use new constants for units

766e0ba

Signed-off-by: David Christen <[email protected]>

Added additional sensor to detect direction of power between grid and…

81a134c

… home

Add handler to multiply load, grid, battery by the relevant xStatus

7d4e79a

Merge pull request #1 from davidchristen/main

9961d9e

Use new constants for units

TimSoethout mentioned this pull request Mar 29, 2024

Version 5.0.0 is making my GoodWe inverter offline #106

Open

		if self._empty_value is not None and self._empty_value == value:
		value = 0

Separate sensors (issue #89) #91

Are you sure you want to change the base?

Separate sensors (issue #89) #91

Conversation

TimSoethout commented Nov 3, 2023

TimSoethout commented Nov 3, 2023

L-four commented Nov 4, 2023

Choose a reason for hiding this comment

f00f commented Nov 6, 2023

Choose a reason for hiding this comment

IsaacInsoll commented Nov 7, 2023

L-four commented Nov 7, 2023

f00f commented Nov 7, 2023

IsaacInsoll commented Nov 7, 2023

Choose a reason for hiding this comment

f00f Nov 9, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

L-four commented Nov 9, 2023

f00f commented Nov 9, 2023

L-four commented Nov 17, 2023

IsaacInsoll commented Dec 6, 2023

IsaacInsoll commented Jan 27, 2024

L-four commented Jan 27, 2024

IsaacInsoll commented Jan 27, 2024

L-four commented Jan 27, 2024

TimSoethout commented Jan 28, 2024 via email

L-four commented Feb 7, 2024

davidchristen commented Feb 18, 2024

robinhood-code commented Mar 7, 2024

lacona20 commented Mar 9, 2024

Brumhilde commented Mar 15, 2024

Brumhilde commented Mar 15, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

L-four commented Mar 26, 2024

katzfiel69 commented Apr 1, 2024

robinhood-code commented May 22, 2024

TimSoethout commented May 23, 2024

L-four commented Jun 1, 2024

TimSoethout commented Jun 20, 2024

robinhood-code commented Aug 20, 2024

TimSoethout commented Aug 27, 2024 • edited Loading

f00f Nov 9, 2023 •

edited

Loading

TimSoethout commented Aug 27, 2024 •

edited

Loading