MQTT Discovery, filtering, custom component #13

neilbacon · 2023-08-29T08:14:27Z

Not sure how much of this you might want:

support MQTT Discovery
filtering of attributes
custom component to run it in homeassistant

kubaceg · 2023-09-11T07:32:37Z

Lovely, I'll try to review this soon :)

…e at night

kubaceg · 2023-09-20T12:42:05Z

I really like those changes but my original assumption was to create a standalone data collector for SOFAR inverters. It's not written to be used only with Home Assistant, Your changes change this approach.
What do you think about creating Your own repo with HA integration? You can include this repo e.g. as submodule and handle the whole building stuff there.
Of course, we link Your repo here in README. WDYT?

Also, check build there are some problems detected by linter with error check :)

neilbacon · 2023-09-21T01:07:51Z

Agreed adding MQTT Discovery makes it HA specific.
"What do you think about creating Your own repo with HA integration?" "You can include this repo e.g. as submodule" I have had to make small changes all over the place, so I don't think I can do that. E.g. to adapters/devices/sofar/sofar_protocol.go
The linter error is about function interfaces in ports (don't know the right terminology for go) having an error return but the implementations never actually do so (they log errors and always return success), so I thought it was misleading to have the caller check for the never occurring error. So should I remove the error return from the function interfaces in ports?

If you just want to close this, it's fine by me. More changes I want to do in my fork:

fix bug resulting in "short reply" errors. In lsw.go readRegisterRange(): n, err := connPort.Read(buf) needs to be in a loop appending bytes until enough have been read.
write log messages to MQTT instead of stderr (have something in HA write the messages to its logs). This change would involve tidying up error handling (returning errors from lower levels rather than logging them).
I sometimes get mostly valid data but with some values erroneously 0. HA history graph shows a smooth curve broken by spikes to 0. I wonder if the cause is 1 above.

kubaceg · 2023-09-28T12:32:25Z

Sorry for long time to respond :)

By separating changes I mean only home assistant plugin changes so whole custom_components directory, hacs.json, info.md etc. Other changes including code changes could stay in this repo as long as they don't spoil other integrations. My intention is to keep this repo as a clean standalone application :)

Of course, I think we should link repo with Your HA plugin here in README :)

Hope it's more clear right now, resuming:

code changes for HA without breaking other integrations - 👍🏻
HA configs with description how run this script - 👎🏻

If You agree with that please clean this PR from unwanted changes, than I'l go with CR :)

PS. I have idea for integrating HA with this tool using HA add-on https://www.home-assistant.io/addons/. Then whole integration (with build binaries) will be updated using HA interface, no extra config, file coping or compiling is needed. I'm using such an approach and it's working brilliant, but whole config is drafted and I never finish it for publishing :(
In such a scenario updating such add-on will be provided by HA, binaries are built automatically on release https://github.com/kubaceg/sofar_g3_lsw3_logger_reader/releases/tag/1.3.0 so with new addon version only what is needed is update binary version, and config/example config when needed. If You're interested we can continue to discuss about addon here #11.

neilbacon · 2023-09-29T01:11:55Z

happy to remove the custom component and adopt your addon. Addon's have their own logs don't they, so that removes the issue of the logs not being integrated. But I think addons are more intended for a whole new type of function (e.g. MQTT, Samba, SSH) and just implementing sensors is more typically done in a component.
which changes do you regard as "breaking other integrations"? Working well with HA is my top priority , so I might prefer to continue with forked code (but I won't needlessly break things)

kubaceg · 2023-10-05T21:12:37Z

Yes, add-ons have their own logs. Hmmm You have right with sensors and components... But on the other hand, this project could be executed on another machine not on machine with HA and everything will work too. Maybe we should separate collector (this project) with the component. The component will rely on MQTT topics delivered by the collector, and the component will configure sensors in HA instance. As I said before my vision for this project is to keep it as a standalone not related to HA tool (that's why it have also prometheus/grafana exporter).
I'm not saying that there are already BC changes. I want You to be aware :)

neilbacon · 2023-10-22T04:14:17Z

Hi Jakub,
I've addressed the linter complaint, removed custom_components/ and replaced the README.md with your one. Please take anything you want and lets get this closed.

I'm still interested in having the inverter data in HA, but it's become less of an issue for me after installing a Shelly EM:

it has current transformer clamps on grid import/export and consumption (less need for inverter data)
it keeps working at night whereas the inverter shuts down and provides no data
it provides faster real time updates (I've been running the sofar program with readInterval: 10)
no spikes!
provides data in the right form for use in the Energy dashboard
provides a relay I use so that HA can control my hot water system (one of the biggest loads in the house)

kubaceg

I leave some comments. If You don't want to fix it let me know I'll fix it myself :)

I was wondering about the external energy meter too because I want to monitor energy consumption for the whole house. BTW how accurate are readings from the inverter vs external energy monitor?

Big thanks for Your contribution once again

kubaceg · 2023-10-27T14:22:09Z

Makefile

 build-arm:
-	env GOOS=linux GOARCH=arm GOARM=5 go build -o sofar-arm
+	env GOOS=linux GOARCH=arm GOARM=5 go build -o custom_components/sofar_g3_lsw3_logger_reader/sofar-arm


Suggested change

env GOOS=linux GOARCH=arm GOARM=5 go build -o custom_components/sofar_g3_lsw3_logger_reader/sofar-arm

env GOOS=linux GOARCH=arm GOARM=5 go build -o bin/sofar-arm

kubaceg · 2023-10-27T14:22:27Z

Makefile

+build-x86:
+	go build -o custom_components/sofar_g3_lsw3_logger_reader/sofar-x86


Suggested change

go build -o custom_components/sofar_g3_lsw3_logger_reader/sofar-x86

go build -o bin/sofar-x86

kubaceg · 2023-10-27T14:22:47Z

.gitignore

+.vscode
+.idea


Suggested change

.idea

.idea

bin

kubaceg · 2023-10-27T14:41:00Z

adapters/devices/sofar/device.go

+	if len(s.attrWhiteList) > 0 {
+		_, ok := s.attrWhiteList[k]
+		return ok
+	} else {
+		for _, re := range s.attrBlackList {
+			if re.MatchString(k) {
+				return false
+			}
+		}
 	}
+	return true


Here's bug. if there is no whitelist elements but there is blacklist element it should ignore elements from blacklist and pass others:

Suggested change

if len(s.attrWhiteList) > 0 {

_, ok := s.attrWhiteList[k]

return ok

} else {

for _, re := range s.attrBlackList {

if re.MatchString(k) {

return false

}

}

}

return true

if len(s.attrWhiteList) > 0 {

if _, ok := s.attrWhiteList[k]; ok {

return true

}

}

for _, re := range s.attrBlackList {

if re.MatchString(k) {

return false

}

}

return true

Here's unit test for it (fully generated by chat gpt :D )

func TestNameFilter(t *testing.T) { // Create a Logger instance with the desired attributes logger := &Logger{ attrWhiteList: map[string]struct{}{ "whitelisted": {}, }, attrBlackList: []*regexp.Regexp{ regexp.MustCompile("^blacklisted"), }, } // Test case 1: Key in the white list result := logger.nameFilter("whitelisted") if result != true { t.Errorf("Expected: true, Got: %v", result) } // Test case 2: Key not in the white list, but not matching any black list regex result = logger.nameFilter("notblacklisted") if result != true { t.Errorf("Expected: true, Got: %v", result) } // Test case 3: Key in the black list result = logger.nameFilter("blacklisted-key") if result != false { t.Errorf("Expected: false, Got: %v", result) } // Test case 4: Key not in the white list and matches a black list regex result = logger.nameFilter("blacklisted") if result != false { t.Errorf("Expected: false, Got: %v", result) } // Test case 5: No white or black list logger = &Logger{} // Reset the logger result = logger.nameFilter("anykey") if result != true { t.Errorf("Expected: true, Got: %v", result) } }

kubaceg · 2023-10-27T14:46:42Z

hacs.json

@@ -0,0 +1,4 @@
+{
+  "name": "Sofar LSW3",


Can we remove this file as this repo doesn't support hacs?

kubaceg · 2023-10-27T14:58:27Z

adapters/export/mosquitto/mqtt.go

+}
+
+// MQTT Discovery: https://www.home-assistant.io/integrations/mqtt/#mqtt-discovery
+func (conn *Connection) InsertDiscoveryRecord(discovery string, state string, expireAfter int, fields []ports.DiscoveryField) error {


expireAfter is unused

kubaceg · 2023-10-27T15:03:07Z

info.md

@@ -0,0 +1,29 @@
+# Integration for Sofar Solar PV Inverters with LSW-3 WiFi data logger with SN 23XXXXXXX


We should remove this info as we removed custom_components

kubaceg · 2023-10-27T15:09:58Z

main.go


-		measurements, err := device.Query()
+	if hasMQTT {


it should not be triggered if discovery is empty

kubaceg · 2023-10-27T17:17:58Z

adapters/export/mosquitto/mqtt.go

+	User      string `yaml:"user"`
+	Password  string `yaml:"password"`
+	Discovery string `yaml:"discovery"`
+	State     string `yaml:"state"`


I'd prefer to stay with a prefix for the sake of backward compatibility

kubaceg · 2023-10-27T17:31:04Z

config-example.yaml

+  url: 1.2.3.4:1883       # MQTT broker URL (e.g. 1.2.3.4:1883)
+  user:                   # MQTT username (leave empty when not needed)
+  password:               # MQTT password (leave empty when not needed)
+  discovery: homeassistant/sensor   # topic prefix for MQTT Discovery


because it's home assistant-specific setting I would rename it to ha_discovery_prefix or smth

neilbacon · 2023-11-01T05:17:40Z

Yes, please take/change whatever you want from this (doing it yourself will be quicker than chatting to me about it).

My thinking on the white/black list (filtering of attributes) was:

new user runs with neither and sees heaps of attributes with value always zero or otherwise unwanted
they set up blacklist entries to get rid of most of them e.g. if single phase to get rid of phase 2 and 3
they figure out what they really want and put that in the whitelist, when there is a non-empty whitelist the blacklist is not used, it's just one or the other (or neither)
once they are happy with the whitelist they run sh/cleanmqtt.sh (on a machine/container with MQTT installed) to get rid of superfluous retained MQTT Discovery messages

But feel free to make it work some other way if you prefer.

The Shelly EM provides 1W resolution, whereas the sofar datalogger provides 10W resolution.

My Shelly EM has a 100A current transformer clamp on the grid and a 50A clamp on all household loads. At night these should read the same (assuming there are no other loads - like maybe the inverter uses a little power?), but mine differ by 5W. So I guess the accuracy is something around that, but it could be worse, I don't really know.

kubaceg · 2023-11-03T14:01:18Z

Ok I'll finish it on my branch :) Thanks for contributing once again

neilbacon added 3 commits August 20, 2023 15:23

add attribute white list

39e7187

discovery

d25f93d

custom component

735a892

neilbacon added 5 commits September 17, 2023 15:55

hacs

711d09c

handle additional device_classes and Unavailable when inverter offlin…

7ab7810

…e at night

sofar binaries

997f554

doc

0788947

config

a2f98cb

kubaceg self-requested a review September 20, 2023 12:03

neilbacon added 5 commits September 21, 2023 15:20

doc

fd12004

Merge branch 'hacs' of github.com:neilbacon/sofar_g3_lsw3_logger_reader

6620146

doc

7639284

buffering was superfluous

9560424

read until enough bytes for a valid modbus reply

f167f09

neilbacon added 5 commits October 17, 2023 13:06

availability on/offline

fdb59ac

Merge remote-tracking branch 'upstream/master'

199c69c

address linter warning

b27d247

delete custom_component & updated README

d164673

config-example.yaml

8eb66a3

kubaceg requested changes Oct 27, 2023

View reviewed changes

kubaceg changed the base branch from master to filters November 3, 2023 14:01

kubaceg merged commit e61f6f8 into kubaceg:filters Nov 3, 2023
1 check passed

kubaceg added a commit that referenced this pull request Dec 11, 2023

MQTT Discovery, filtering, custom component (#13) (#14)

5e43112

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

MQTT Discovery, filtering, custom component #13

MQTT Discovery, filtering, custom component #13

neilbacon commented Aug 29, 2023

kubaceg commented Sep 11, 2023

kubaceg commented Sep 20, 2023

neilbacon commented Sep 21, 2023 •

edited

Loading

kubaceg commented Sep 28, 2023

neilbacon commented Sep 29, 2023 •

edited

Loading

kubaceg commented Oct 5, 2023

neilbacon commented Oct 22, 2023 •

edited

Loading

kubaceg left a comment •

edited

Loading

kubaceg Oct 27, 2023

kubaceg Oct 27, 2023

kubaceg Oct 27, 2023

kubaceg Oct 27, 2023

kubaceg Oct 27, 2023

kubaceg Oct 27, 2023

kubaceg Oct 27, 2023

kubaceg Oct 27, 2023

kubaceg Oct 27, 2023

kubaceg Oct 27, 2023

neilbacon commented Nov 1, 2023

kubaceg commented Nov 3, 2023

	env GOOS=linux GOARCH=arm GOARM=5 go build -o custom_components/sofar_g3_lsw3_logger_reader/sofar-arm
	env GOOS=linux GOARCH=arm GOARM=5 go build -o bin/sofar-arm

		build-x86:
		go build -o custom_components/sofar_g3_lsw3_logger_reader/sofar-x86

	go build -o custom_components/sofar_g3_lsw3_logger_reader/sofar-x86
	go build -o bin/sofar-x86

		@@ -0,0 +1,29 @@
		# Integration for Sofar Solar PV Inverters with LSW-3 WiFi data logger with SN 23XXXXXXX

		.vscode
		.idea

MQTT Discovery, filtering, custom component #13

MQTT Discovery, filtering, custom component #13

Conversation

neilbacon commented Aug 29, 2023

kubaceg commented Sep 11, 2023

kubaceg commented Sep 20, 2023

neilbacon commented Sep 21, 2023 • edited Loading

kubaceg commented Sep 28, 2023

neilbacon commented Sep 29, 2023 • edited Loading

kubaceg commented Oct 5, 2023

neilbacon commented Oct 22, 2023 • edited Loading

kubaceg left a comment • edited Loading

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

Choose a reason for hiding this comment

neilbacon commented Nov 1, 2023

kubaceg commented Nov 3, 2023

neilbacon commented Sep 21, 2023 •

edited

Loading

neilbacon commented Sep 29, 2023 •

edited

Loading

neilbacon commented Oct 22, 2023 •

edited

Loading

kubaceg left a comment •

edited

Loading