tolwi / hassio-ecoflow-cloud

EcoFlow Cloud Integration for Home Assistant
406 stars 74 forks source link

Question/Enhancement/Concept: mqtt client ids (public API) | Part 2 #351

Open giovanne123 opened 1 month ago

giovanne123 commented 1 month ago

@tolwi, (already asked the question here https://github.com/tolwi/hassio-ecoflow-cloud/issues/306 but because I was not able to reopen the issue I create a new one, maybe you only look to open issues/tickets)

can you please describe a bit how the creation of client_id, mqtt connection is working in current version v1.2.0?

I encountered still a problem sometimes when e.g. updating HA and restarting therefore. Maybe because I have 4 devices in Ecoflow Cloud Integration (Edit: with each device has it's own group 🫣) and a separate python script (using one client_id) running and reaching the client_id limit ... Recreate Key/Secret everytime solves the problem! Maybe this is also the reason why often the people are raising issues... Are you using only one mqtt connection per account(key/value)? Edit: per group

custom_components.ecoflow_cloud.api.EcoflowException: (('signature is wrong',), {})
2024-09-26 14:13:53.713 ERROR (MainThread) [homeassistant] Error doing job: Task exception was never retrieved (None)
Traceback (most recent call last):
  File "/config/custom_components/ecoflow_cloud/api/public_api.py", line 80, in quota_all
    raw = await self.call_api("/device/quota/all", {"sn": sn})
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/config/custom_components/ecoflow_cloud/api/public_api.py", line 100, in call_api
    return await self._get_json_response(resp)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/config/custom_components/ecoflow_cloud/api/__init__.py", line 80, in _get_json_response
    raise EcoflowException(f"{response_message}")
custom_components.ecoflow_cloud.api.EcoflowException: (('signature is wrong',), {})
2024-09-26 14:13:55.030 ERROR (Thread-7 (_thread_main)) [custom_components.ecoflow_cloud.api.ecoflow_mqtt] MQTT connect: The connection was refused. (Hassio-open-exxxxxxxxxxxxxxxxxxxxxxxeec5-Powerstream-2-20240926) - None
2024-09-26 14:13:55.031 ERROR (Thread-7 (_thread_main)) [custom_components.ecoflow_cloud.api.ecoflow_mqtt] Unexpected MQTT Socket disconnection : <ssl.SSLSocket fd=76, family=2, type=1, proto=6, laddr=('192.xxx.0.111', 55459), raddr=('18.xxx.239.78', 8883)>
2024-09-26 14:13:55.044 ERROR (Thread-6 (_thread_main)) [custom_components.ecoflow_cloud.api.ecoflow_mqtt] MQTT connect: The connection was refused. (Hassio-open-exxxxxxxxxxxxxxxxxxxxxxxeec5-Delta-2-20240926) - None
2024-09-26 14:13:55.045 ERROR (Thread-6 (_thread_main)) [custom_components.ecoflow_cloud.api.ecoflow_mqtt] Unexpected MQTT Socket disconnection : <ssl.SSLSocket fd=79, family=2, type=1, proto=6, laddr=('192.xxx.0.111', 48077), raddr=('18.xxx.239.78', 8883)>
2024-09-26 14:15:55.144 ERROR (Thread-7 (_thread_main)) [custom_components.ecoflow_cloud.api.ecoflow_mqtt] MQTT connect: The connection was refused. (Hassio-open-exxxxxxxxxxxxxxxxxxxxxxxeec5-Powerstream-2-20240926) - None
2024-09-26 14:15:55.145 ERROR (Thread-7 (_thread_main)) [custom_components.ecoflow_cloud.api.ecoflow_mqtt] Unexpected MQTT Socket disconnection : <ssl.SSLSocket fd=78, family=2, type=1, proto=6, laddr=('192.xxx.0.111', 42147), raddr=('18.xxx.239.78', 8883)>

Why are you using the "group" in the client_id? Because the "name"/certificateAccount (open-xxxx) makes it already unique? Makes it sense to remove it from client_id? What is the intention of the group entry/option at all? Also the "datetime" part maybe can be removed to have more security to not reach the client_id limits? (for my python script e.g. I am only using one fix client_id for all devices per key/secret (certificateAccount/-Password) over all the time)

image

So far I have used the integration, group this way (because I insert my key/secret) and do not need the group: my group name is identical to device name: (maybe I do not use your integration the right way?) image

Would be fine if you answer to get a bit insights. Final question, is your planned one mqtt connection (https://github.com/tolwi/hassio-ecoflow-cloud/issues/306#issuecomment-2321209246) already in v1.2.0?

Edit: I think it is one mqtt connection per group πŸ€” and I am using group wrong. At the moment I have one device in one separate group. So for my 4 devices results in 4 mqtt connections... Maybe need to put all in one group πŸ€”... Can Someone post a screenshot for a group with some devices?

Edit 2: is it better to have not one connection per group, but per key/secret (certificateAccount/-Password)? Because in current Implementation you can add different devices/accouts (Key/Secret) into a group.

@tolwi , and many many thanks to you for this integration! Beside this client_id topic it is working absolutely stable for me :-)

walderston commented 1 month ago

Every connection with MQTT needs a unique client_id.

Currently the way this integration works is 1 device per group (and each group has a unique client_id generated upon creation/restart).

As EF imposes a limit of 10 ids, it quickly fills up although its meant to reset however I've never noticed mine reset

giovanne123 commented 1 month ago

beside the questions for general concept for the mqtt connection per certificateAccount/-password(Key/Secret) insead of group mentined in above initial posted issue --> would be fine if @tolwi will have a look to above questions if the time is there.

as a first workaround I have removed the date in the client_id to not reach the client_id limit so far: line 40: hassio-ecoflow-cloud/custom_components/ecoflow_cloud/api/public_api.py self.mqtt_info.client_id = f"Hassio-{self.mqtt_info.username}-{self.group.replace(' ', '-')}"

For the devices I have still one device per group (like in picture from initial post above) which means 4 mqtt connections/client_ids in my situation (+ 1 connection/client_id for my standalone python listener). = 5 client_ids/mqtt connections This now has worked without issue reaching client_id limit/connection refused, ... for a Home Assistant (to 2024.10.1) update and system updates on the Pi 4 which includes docker updates and therefore HA/Ecoflow Cloud restarts.

For the topic reaching the client_id limits per day (how this is implemented in Ecoflow mqtt broker is still not confirmed by Ecoflow) we must keep in mind that we do not know when for us (mqtt-e/EU broker) the new days starts. Additional in my case I have a restricted internet disconnect from internet provider once per day (e.g. at 2 o'clock) which can/will result in first created new client_ids ...

--> so for me it makes sense to first remove the date from the client_ids like done above to prevent this issues (for my standalone python listener I use the same client_id now for nearly 3/4 year) will continue to monitor this issue...

florismetzner commented 1 month ago

Thx for sharing your insights @giovanne123 Yesterday I ran into the limit again due to multiple reboots within short time. I'll try your suggested workaround and share my insights here

florismetzner commented 1 month ago

Update: in hassio-ecoflow-cloud/custom_components/ecoflow_cloud/api/public_api.py I just changed the line line 40 into this:

self.mqtt_info.client_id = f"Hassio-{self.mqtt_info.username}-{self.group.replace(' ', '-')}"

After several reboots it looks good...will need to test it a bit. I have 2 devices plus py script for regulation:

image

giovanne123 commented 1 month ago

Thanks for sharing your feedback. Nevertheless I see this (remove date from client id) as workaround.

I think it would be better to have not one mqtt connection per integration group, it should be one connection per key/secret (certificateAccount/-Password) because we add devices to that/our Ecoflow account ...

But I have not investigated how to replace group and use account instead...

Naumsede commented 3 weeks ago

Still working with your workaround!! Thanks a lot!

giovanne123 commented 3 weeks ago

Yeah, for me too. Had some Home Assistant updates and rpi system updates in the meantime and still working without any issue. But plan will be still for future to have one connection per key/secret (certificateAccount/-Password) instead of integration's group: https://github.com/tolwi/hassio-ecoflow-cloud/issues/351#issuecomment-2401591323 ... when time will come πŸ˜‰

giovanne123 commented 5 days ago

Still fine with the workaround (remove date from client id): https://github.com/tolwi/hassio-ecoflow-cloud/issues/351#issuecomment-2399489673), today again sevaral HA updates, restarts and rpi system updates without an issue :-)

But must keep in mind to redo the change/workaround if there will be a HA Ecoflow Cloud Integration Update coming (without this handlig already included)

Nevertheless plan will be still for future to have one connection per key/secret (certificateAccount/-Password) instead of integration's group: https://github.com/tolwi/hassio-ecoflow-cloud/issues/351#issuecomment-2401591323 ... when time will come πŸ˜‰

walderston commented 5 days ago

I've put in a PR, lets see if its approved by @tolwi

giovanne123 commented 4 days ago

I've put in a PR, lets see if its approved by @tolwi

Thanks for the PR

walderston commented 2 days ago

my PR has been merged. So on next release, it should be there.

giovanne123 commented 2 days ago

Saw it thanks.

Will let this issue/question open and see after next release and further testing if it will be still interesting to have one connection per key/secret (certificateAccount/-Password) instead of integration's group. In general I think that it fits better to have client id per key/secret because we have no group on Ecoflow's end. But will see...