home-assistant-omnik-inverter icon indicating copy to clipboard operation
home-assistant-omnik-inverter copied to clipboard

Cannot connect to Trannergy invertor (old addon works)

Open rob-on-git opened this issue 1 year ago • 22 comments

Detailed description

When configuring within the GUI, i cannot connect to the TCP stream to my trannergy SGN4000TL (while i do see traffic)

With serial 0: a short burst of traffic, and timing out
With serial of device: Short burst of traffic and saying 'cannot connect'

Can someone point me out in how to debug this properly? (i do have some scripting/network background, but not that much within HA)

Context

Currently i use the https://github.com/hultenvp/home_assistant_omnik_solar integration, which works fine (adapted the code to allow my 2nd string to be read)

Diagnostics

When adding to through the GUI in TCP mode, it says: 'Cannot connect' almost rightaway.

rob-on-git avatar Jul 16 '22 12:07 rob-on-git

There hasn't been any activity on this issue recently, so we clean up some of the older and inactive issues. Please make sure to update to the latest version and check if that solves the issue. Let us know if that works for you by leaving a comment 👍 This issue has now been marked as stale and will be closed if no further activity occurs. Thanks!

github-actions[bot] avatar Aug 06 '22 08:08 github-actions[bot]

Are you sure you are entering the correct serial? This can be confusing. You need to have the one from the (wifi) device information.

Screenshot 2022-06-15 at 13 43 49

robbinjanssen avatar Aug 08 '22 07:08 robbinjanssen

I know I used the proper serial, as the older addon also works (and i copied that serial). Also, i use ethernet instead of wifi, but that works similar (with serial etc).

I took a tcpdump, and i do see communication with the inverter when trying to configure, but the GUI still says it cannot connect. (one part of the packet says "DATA SEND IS OK" from the inverter back to HA) To be honest, i don't know how to debug this part.

rob-on-git avatar Aug 08 '22 07:08 rob-on-git

Can you possibly share this dump? I've been refactoring the code somewhat to better cope with this second DATA SEND IS OK message as none of the other implementations deal with it. Is there anything in the logs that indicates what could be going wrong, before this issue is inevitably rewrapped as a generic/useless "cannot connect" error?

MarijnS95 avatar Aug 08 '22 13:08 MarijnS95

I didn't find anything in the logs. Also tried to enable more debugging on logs, but nothing seems related to the omnik.

trannergy-omnik.dump.gz

rob-on-git avatar Aug 08 '22 14:08 rob-on-git

@rob-on-git thanks, I'll throw this through the parser when home and see what comes out of it. We probably do need some better logging at some point, not sure where the mostly-specific error types are disappearing.

MarijnS95 avatar Aug 08 '22 14:08 MarijnS95

@rob-on-git After a bit of puzzling I managed to extract valid data from the dump you provided:

DEBUG:omnikinverter:Handling message `bytearray(b'YQ\xb0\xedV\xfd%\xedV\xfd%\x81\x01\x16PVL4000N20505016\x00\xd4\x05\xf6\x05\xdb\x00\x00\x00\t\x00\x07\x00\x00\x00\n\x00\x00\x00\x00\t!\x00\x00\x00\x00\x13\x8c\x00\xcb\x00\x00\x00\x00\x00\x00\x00\x00\x00/\x00\x00}\xdc\x00\x00 5\x00\x01\x00\x00\x00\x00\xff\xff\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x8c')`
DEBUG:omnikinverter:Message type 0xb0, length 89, checksum 0x8c
WARNING:omnikinverter:Unexpected padding0 `b'\x81\x01\x16'`
DEBUG:omnikinverter:Handling message `bytearray(b'\x11Q\xf0\xedV\xfd%\xedV\xfd%DATA SEND IS OK\r\n\r')`
DEBUG:omnikinverter:Message type 0xf0, length 17, checksum 0xd
WARNING:omnikinverter:Omnik sent text message `DATA SEND IS OK`
{'serial_number': 'PVL4000N20505016', 'temperature': 21.200000000000003, 'dc_input_voltage': [152.6, 149.9, 0.0], 'dc_input_current': [0.9, 0.7000000000000001, 0.0], 'ac_output_current': [1.0, 0.0, 0.0], 'ac_output_voltage': [233.70000000000002, 0.0, 0.0], 'ac_output_frequency': [50.04, 0.0, 0.0], 'ac_output_power': [203, 0, 0], 'solar_energy_today': 0.47000000000000003, 'solar_energy_total': 3222.0, 'solar_hours_total': 8245, 'inverter_active': True}

You can pick the WIP changes from https://github.com/MarijnS95/python-omnikinverter/commit/8de16303e535b6b56ec695db19b9f0169e342f9c; I'll tinker around with the Unexpected padding0 `b'\x81\x01\x16'` a bit, before coming up with a solution that supports your inverter by allowing the different constants and using it to detect whether to parse the firmware fields or not.

Most problems were caused by Wireshark giving back bad data when saving the reconstructed TCP stream to a file, who in their right mind wants to save the literal ASCII text dump with all invalid characters replaced with ., instead of getting the raw bytes?!?!

MarijnS95 avatar Aug 08 '22 23:08 MarijnS95

Tried with your changes, but i get an 'Unknown error occurred' at this moment.

And in the logs:

Logger: omnikinverter
Source: /usr/local/lib/python3.10/site-packages/omnikinverter/tcp.py:237
First occurred: 10:27:55 (2 occurrences)
Last logged: 10:28:02
Unexpected padding0 `b'\x81\x01\x16'
Logger: aiohttp.server
Source: custom_components/omnik_inverter/config_flow.py:160
Integration: Omnik Inverter ([documentation](https://github.com/robbinjanssen/home-assistant-omnik-inverter), [issues](https://github.com/robbinjanssen/home-assistant-omnik-inverter/issues))
First occurred: 10:27:55 (2 occurrences)
Last logged: 10:28:02
Error handling request

Traceback (most recent call last):
  File "/usr/local/lib/python3.10/site-packages/aiohttp/web_protocol.py", line 435, in _handle_request
    resp = await request_handler(request)
  File "/usr/local/lib/python3.10/site-packages/aiohttp/web_app.py", line 504, in _handle
    resp = await handler(request)
  File "/usr/local/lib/python3.10/site-packages/aiohttp/web_middlewares.py", line 117, in impl
    return await handler(request)
  File "/usr/src/homeassistant/homeassistant/components/http/security_filter.py", line 60, in security_filter_middleware
    return await handler(request)
  File "/usr/src/homeassistant/homeassistant/components/http/forwarded.py", line 222, in forwarded_middleware
    return await handler(request)
  File "/usr/src/homeassistant/homeassistant/components/http/request_context.py", line 28, in request_context_middleware
    return await handler(request)
  File "/usr/src/homeassistant/homeassistant/components/http/ban.py", line 79, in ban_middleware
    return await handler(request)
  File "/usr/src/homeassistant/homeassistant/components/http/auth.py", line 236, in auth_middleware
    return await handler(request)
  File "/usr/src/homeassistant/homeassistant/components/http/view.py", line 136, in handle
    result = await result
  File "/usr/src/homeassistant/homeassistant/components/config/config_entries.py", line 177, in post
    return await super().post(request, flow_id)
  File "/usr/src/homeassistant/homeassistant/components/http/data_validator.py", line 62, in wrapper
    result = await method(view, request, *args, **kwargs)
  File "/usr/src/homeassistant/homeassistant/helpers/data_entry_flow.py", line 109, in post
    result = await self._flow_mgr.async_configure(flow_id, data)
  File "/usr/src/homeassistant/homeassistant/data_entry_flow.py", line 277, in async_configure
    result = await self._async_handle_step(
  File "/usr/src/homeassistant/homeassistant/data_entry_flow.py", line 359, in _async_handle_step
    result: FlowResult = await getattr(flow, method)(user_input)
  File "/config/custom_components/omnik_inverter/config_flow.py", line 160, in async_step_setup_tcp
    await client.inverter()
  File "/usr/local/lib/python3.10/site-packages/omnikinverter/omnikinverter.py", line 187, in inverter
    return Inverter.from_tcp(fields)
  File "/usr/local/lib/python3.10/site-packages/omnikinverter/models.py", line 194, in from_tcp
    return Inverter(
TypeError: Inverter.__init__() missing 2 required positional arguments: 'firmware' and 'firmware_slave'

rob-on-git avatar Aug 09 '22 08:08 rob-on-git

TypeError: Inverter.__init__() missing 2 required positional arguments: 'firmware' and 'firmware_slave'

Alright, I've have only applied the necessary changes to the tcp module while testing, the Inverter object requires the firmware and firmware_slave field. A force-pushed commit has been pushed to https://github.com/MarijnS95/python-omnikinverter/tree/trannergy that works around this issue.

If I may ask, how'd you apply this patch so quickly? I've been struggling to find the best way to develop on both a local development homeassistant instance as well as my own "production" instance to test-drive these contributions. Writing code is easy, replacing and live-editing packages is hard :grimacing:

MarijnS95 avatar Aug 09 '22 08:08 MarijnS95

If I may ask, how'd you apply this patch so quickly? I've been struggling to find the best way to develop on both a local development homeassistant instance as well as my own "production" instance to test-drive these contributions. Writing code is easy, replacing and live-editing packages is hard 😬

I run the homeassistant on a supervised docker on Debian.

Just went into docker shell and did the edit by hand... after that restarted the docker container. (in other words, quick and dirty...)

rob-on-git avatar Aug 09 '22 09:08 rob-on-git

Look a lot better now! Only missing the serial of the inverter (but not really needed). The rest of the prod-items are templated items. solar

Still the logfile entry about the padding and

Logger: omnikinverter
Source: /usr/local/lib/python3.10/site-packages/omnikinverter/tcp.py:213
First occurred: 11:18:35 (2 occurrences)
Last logged: 11:28:34
Omnik sent text message `DATA SEND IS OK`

rob-on-git avatar Aug 09 '22 10:08 rob-on-git

Fwiw this is where we're loosing most of the error context, I think this should at least have a LOGGER.exception() here to print the exception (current context) to the log:

https://github.com/robbinjanssen/home-assistant-omnik-inverter/blob/c08ab6a99479b58b0114c6219a96c22dcfed0b02/custom_components/omnik_inverter/config_flow.py#L161-L162

Look a lot better now!

Awesome! :tada:

Only missing the serial of the inverter (but not really needed).

I think these are intentionally omitted; if you feel like more fields should be added (can be disabled by default so that they're invisible) they can be added at:

https://github.com/robbinjanssen/home-assistant-omnik-inverter/blob/c08ab6a99479b58b0114c6219a96c22dcfed0b02/custom_components/omnik_inverter/sensor.py#L42-L164

(The fields are made available by python-omnikinverter, just not converted to HASS sensors/entities here).

Still the logfile entry about the padding

Can you possibly send a few more dumps (extracted TCP stream - saved as RAW(!) - is fine) collected over time, so that we can understand what data these fields are connected to? Or is it always \x81\x01\x16 (in which case I don't need more dumps)?

Omnik sent text message DATA SEND IS OK

I think we'll have to demote this to DEBUG, it's not relaly a WARNING. My intent was to refactor the TCP parsing so that it always requires/expects this text as the second message, but I haven't yet managed to finish that branch and open a PR for it. That approach will also reuse the TCP connection :tada:

MarijnS95 avatar Aug 09 '22 10:08 MarijnS95

I'll try to do some dumps and let you know.

Another weird thing is, that it seems the plugin stopped pulling data after 24 hours (this morning, it started to get data... and suddenly stopped after a few hours, while the old integration still kept on running. Might be an additional issue... After a restart, it started to work again. I'll monitor this behaviour the next days.

rob-on-git avatar Aug 10 '22 10:08 rob-on-git

I had to insert socket.settimeout() when I was still using the plugin from Hein Oldenhuis, something like https://github.com/PierreLevres/home_assistant_omnik_solar/commit/4cf5401aac4a16aaa0c930707bdbfa7c554a8250 but I thought it also landed on the fork from hultenvp?

I think I forgot to port this timeout to the TCP backend.

MarijnS95 avatar Aug 10 '22 10:08 MarijnS95

I did end up covering the timeout in https://github.com/klaasnicolaas/python-omnikinverter/pull/207 as well, which @rob-on-git already seems to have found and given a thumbs-up :grin:

MarijnS95 avatar Aug 10 '22 11:08 MarijnS95

Still the logfile entry about the padding

Can you possibly send a few more dumps (extracted TCP stream - saved as RAW(!) - is fine) collected over time, so that we can understand what data these fields are connected to? Or is it always \x81\x01\x16 (in which case I don't need more dumps)?

Omnik sent text message DATA SEND IS OK

I think we'll have to demote this to DEBUG, it's not relaly a WARNING. My intent was to refactor the TCP parsing so that it always requires/expects this text as the second message, but I haven't yet managed to finish that branch and open a PR for it. That approach will also reuse the TCP connection tada

I did a longer dump (only forgot to post the analysis), but seems like the same padding is always there.... No other padding seen (approx 1 hour of traffic) Hopefully, this makes it a bit easier.

rob-on-git avatar Aug 18 '22 12:08 rob-on-git

In light of the recent request for dealing with the alert field (https://github.com/robbinjanssen/home-assistant-omnik-inverter/issues/126, https://github.com/klaasnicolaas/python-omnikinverter/pull/247), could one of these bits possibly be representing that? Can you check if there's an alert value on your inverter reported by any of the connection mechanisms (JS/HTML/JSON)?

Otherwise, can you check in JS/HTML/JSON if there's any value matching 0x16 = 22 or 0x116 = 278? For the Omnik inverters that'd mean we need to have something for 0x201 (= 513) or 0x1, perhaps this is the "sub-manufacturer" ID or something?

MarijnS95 avatar Sep 07 '22 07:09 MarijnS95

In light of the recent request for dealing with the alert field (#126, klaasnicolaas/python-omnikinverter#247), could one of these bits possibly be representing that? Can you check if there's an alert value on your inverter reported by any of the connection mechanisms (JS/HTML/JSON)?

Otherwise, can you check in JS/HTML/JSON if there's any value matching 0x16 = 22 or 0x116 = 278? For the Omnik inverters that'd mean we need to have something for 0x201 (= 513) or 0x1, perhaps this is the "sub-manufacturer" ID or something?

Hi Marijn,

I do not see any alerts in the webinterface. The only thing what might be related (?) is the firmware version i can see, which is : H4.01.51Y4.0.02W1.0.57(2017-07-261-D)

It also says 'Connected inverter number 1 , type trannergy'

rob-on-git avatar Sep 22 '22 15:09 rob-on-git

g the timeout in klaasnicolaas/python-omnikinverter#207 as well, which @rob-on-git already seems to have found and give

Can i assist in taking this forward ? See that your pull request is not complete yet for the TCP timeout. I believe that is a minor update is needed to get it in, I would assume.

rob-on-git avatar Sep 28 '22 09:09 rob-on-git

@rob-on-git If I remember correctly I think @klaasnicolaas requested me to not "refactor" the system by pulling the timout up higher - which then spans multiple async calls instead of purely the network request - so I'd have to move it down again and check again which calls inside the TCP backend exactly are responsible for blocking on the request to complete.

@klaasnicolaas If I misremembered/misunderstood, or just for general clarity, please request this as change on the PR so that I can track it :)

MarijnS95 avatar Sep 28 '22 12:09 MarijnS95

There hasn't been any activity on this issue recently, so we clean up some of the older and inactive issues. Please make sure to update to the latest version and check if that solves the issue. Let us know if that works for you by leaving a comment 👍 This issue has now been marked as stale and will be closed if no further activity occurs. Thanks!

github-actions[bot] avatar Mar 16 '23 08:03 github-actions[bot]

Hi @MarijnS95, is there already any progress on this. Didn't see much activity.

Still using the old plugin though, which still works.

rob-on-git avatar Mar 19 '23 08:03 rob-on-git