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

Cannot connect to Trannergy invertor (old addon works) #121

Open
rob-on-git opened this issue Jul 16, 2022 · 22 comments
Open

Cannot connect to Trannergy invertor (old addon works) #121

rob-on-git opened this issue Jul 16, 2022 · 22 comments
Labels
no-stale This issue or PR is exempted from the stable bot.

Comments

@rob-on-git
Copy link

rob-on-git commented Jul 16, 2022

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.

@github-actions
Copy link

github-actions bot commented Aug 6, 2022

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!

@robbinjanssen
Copy link
Owner

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

@rob-on-git
Copy link
Author

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.

@MarijnS95
Copy link
Collaborator

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?

@rob-on-git
Copy link
Author

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

@MarijnS95
Copy link
Collaborator

@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
Copy link
Collaborator

@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 MarijnS95/python-omnikinverter@8de1630; 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?!?!

@rob-on-git
Copy link
Author

rob-on-git commented Aug 9, 2022

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'

@MarijnS95
Copy link
Collaborator

MarijnS95 commented Aug 9, 2022

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 😬

@rob-on-git
Copy link
Author

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
Copy link
Author

rob-on-git commented Aug 9, 2022

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`

@MarijnS95
Copy link
Collaborator

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:

except OmnikInverterError:
errors["base"] = "cannot_connect"

Look a lot better now!

Awesome! 🎉

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:

SENSORS: dict[Literal["inverter", "device"], tuple[SensorEntityDescription, ...]] = {
SERVICE_INVERTER: (
SensorEntityDescription(
key="solar_current_power",
name="Current Power Production",
icon="mdi:weather-sunny",
native_unit_of_measurement=POWER_WATT,
device_class=SensorDeviceClass.POWER,
state_class=SensorStateClass.MEASUREMENT,
),
SensorEntityDescription(
key="solar_energy_today",
name="Solar Production - Today",
native_unit_of_measurement=ENERGY_KILO_WATT_HOUR,
device_class=SensorDeviceClass.ENERGY,
state_class=SensorStateClass.TOTAL_INCREASING,
),
SensorEntityDescription(
key="solar_energy_total",
name="Solar Production - Total",
icon="mdi:chart-line",
native_unit_of_measurement=ENERGY_KILO_WATT_HOUR,
device_class=SensorDeviceClass.ENERGY,
state_class=SensorStateClass.TOTAL_INCREASING,
),
SensorEntityDescription(
key="solar_hours_total",
name="Solar Production - Uptime",
icon="mdi:clock",
native_unit_of_measurement=TIME_HOURS,
state_class=SensorStateClass.TOTAL_INCREASING,
),
SensorEntityDescription(
key="temperature",
name="Inverter temperature",
icon="mdi:thermometer",
native_unit_of_measurement=TEMP_CELSIUS,
device_class=SensorDeviceClass.TEMPERATURE,
state_class=SensorStateClass.MEASUREMENT,
),
ArraySensorEntityDescription(
key="dc_input_{}_voltage",
data_key="dc_input_voltage",
range=range(3),
name="DC Input {} - Voltage",
entity_registry_enabled_default=False,
icon="mdi:lightning-bolt",
native_unit_of_measurement=ELECTRIC_POTENTIAL_VOLT,
device_class=SensorDeviceClass.VOLTAGE,
state_class=SensorStateClass.MEASUREMENT,
),
ArraySensorEntityDescription(
key="dc_input_{}_current",
data_key="dc_input_current",
range=range(3),
name="DC Input {} - Current",
entity_registry_enabled_default=False,
icon="mdi:current-dc",
native_unit_of_measurement=ELECTRIC_CURRENT_AMPERE,
device_class=SensorDeviceClass.CURRENT,
state_class=SensorStateClass.MEASUREMENT,
),
ArraySensorEntityDescription(
key="ac_output_{}_voltage",
data_key="ac_output_voltage",
range=range(3),
name="AC Output {} - Voltage",
entity_registry_enabled_default=False,
icon="mdi:lightning-bolt",
native_unit_of_measurement=ELECTRIC_POTENTIAL_VOLT,
device_class=SensorDeviceClass.VOLTAGE,
state_class=SensorStateClass.MEASUREMENT,
),
ArraySensorEntityDescription(
key="ac_output_{}_current",
data_key="ac_output_current",
range=range(3),
name="AC Output {} - Current",
entity_registry_enabled_default=False,
icon="mdi:current-ac",
native_unit_of_measurement=ELECTRIC_CURRENT_AMPERE,
device_class=SensorDeviceClass.CURRENT,
state_class=SensorStateClass.MEASUREMENT,
),
ArraySensorEntityDescription(
key="ac_output_{}_power",
data_key="ac_output_power",
range=range(3),
name="AC Output {} - Power",
entity_registry_enabled_default=False,
icon="mdi:lightning-bolt",
native_unit_of_measurement=POWER_WATT,
device_class=SensorDeviceClass.POWER,
state_class=SensorStateClass.MEASUREMENT,
),
ArraySensorEntityDescription(
key="ac_output_{}_frequency",
data_key="ac_output_frequency",
range=range(3),
name="AC Output {} - Frequency",
entity_registry_enabled_default=False,
icon="mdi:sine-wave",
native_unit_of_measurement=FREQUENCY_HERTZ,
device_class=SensorDeviceClass.FREQUENCY,
state_class=SensorStateClass.MEASUREMENT,
),
),
SERVICE_DEVICE: (
SensorEntityDescription(
key="signal_quality",
name="Signal Quality",
icon="mdi:wifi",
native_unit_of_measurement=PERCENTAGE,
entity_category=EntityCategory.DIAGNOSTIC,
),
SensorEntityDescription(
key="ip_address",
name="IP Address",
icon="mdi:network",
entity_category=EntityCategory.DIAGNOSTIC,
),
),
}

(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 🎉

@rob-on-git
Copy link
Author

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.

@MarijnS95
Copy link
Collaborator

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

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

@MarijnS95
Copy link
Collaborator

I did end up covering the timeout in klaasnicolaas/python-omnikinverter#207 as well, which @rob-on-git already seems to have found and given a thumbs-up 😁

@rob-on-git
Copy link
Author

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.

@MarijnS95
Copy link
Collaborator

MarijnS95 commented Sep 7, 2022

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?

@rob-on-git
Copy link
Author

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
Copy link
Author

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.

@MarijnS95
Copy link
Collaborator

@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 :)

@github-actions
Copy link

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 github-actions bot added the stale There has not been activity on this issue or PR for quite some time. label Mar 16, 2023
@robbinjanssen robbinjanssen added no-stale This issue or PR is exempted from the stable bot. and removed stale There has not been activity on this issue or PR for quite some time. labels Mar 16, 2023
@rob-on-git
Copy link
Author

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

Still using the old plugin though, which still works.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no-stale This issue or PR is exempted from the stable bot.
Projects
None yet
Development

No branches or pull requests

3 participants