albertogeniola / MerossIot

Async Python library for controlling Meross devices
https://albertogeniola.github.io/MerossIot/
MIT License
478 stars 88 forks source link

[suggestion] LAN_HTTP_FIRST : why not have a DeviceRegistry stored on disk ? #255

Closed MichelRabozee closed 1 year ago

MichelRabozee commented 1 year ago

Hi,

First, thank you for your incredible work !

Today, I ran into the "You have issued too many tokens without logging out." problem. I suspect this was because one of the MSS310 was offline, leading to some manager.close and http_api_client.async_logout not being performed, so I added a try/catch (as seen elsewhere) to correctly close and logout.

Anyway, this made me think about the "LAN_HTTP_FIRST" feature. However, this one requires anyway to log onto the cloud to retrieve the devices and build the DeviceRegistry, each time the script runs.

It is possible to have that DeviceRegistry written to disk and read from it for the stored data to be used instead of getting them each time from the cloud ?

My setup is 6 MSS310 to read the power consumption every minute, so every means to reduce the probability of a temporary account lock (for 5 hours!) is welcome!

Michel

albertogeniola commented 1 year ago

Hi @MichelRabozee ,

that's actually one of the feature I wanted to add in the near future. One of the ways I can think of it is to add two methods on the manager/device registry: export and import (both as json). Then it's up to the library user to handle storage on the disk and to load them when needed.

Feel free to open a PR for that, I'll try to have a look at that!

MichelRabozee commented 1 year ago

Hello,

sadly, I am not so savvy with JSON to create such a pull request with the relevant code (if that is what you mean by PR). The way you mention it seems easy, but it need to have a good knowledge of the data needed to be exported and imported and at which point in your code; I fear it is out of my league, though I wanted to help :-(

Anyway, I guess I will first take a look locally, I guess the best is to create the methods export_registry and import_registry using json.dump and json.load.

Due to the involved objects complexity, I think the best is to use the "jsonpickle" module. But it implies to need to install that module. However, even with jsonpickle, the objects hierarchy is really complex and does not output fully in order to be easily recreated.

--edit-- I tried, but I run into some not easily solvable issues when reimporting the jsonpickle exported DeviceRegistry object.

I guess this operation requires more deep knowledge of the contents of that array, and a more "item by item" way of exporting/importing the data. Just staying at the DeviceRegistry object won't work.

joethemonk commented 1 year ago

HI @albertogeniola,

I REALLY would like you to make it so I can dump the devices to a json and load them from there rather than running async_device_discovery() every time, as @MichelRabozee says. (Bump! 🎸) ) Running that too frequently is a real risk of our accounts being shut off, and as it is I need to do that more or less every time I want to turn a plug on or off.

Your library kicks ass, so I really want to use it, and here's my plug for implementing the above! The next best thing I could do to flip a meross device on or off from python would be to install Home Assistant on a VM I keep running all the time, or get an odroid or raspberry pi and set it up there. TON of work.

So I'm going to be sitting here eagerly waiting! 😁 🙌

I can't wait :)

albertogeniola commented 1 year ago

Hi! The latest version (0.4.5.3) of the library adds the basic support for this feature.

You can dump the device list with the following command:

manager.dump_device_registry("test.dump")

At the same time, you can reload that dump with this:

manager.load_devices_from_dump("test.dump")

Have a look at the dump.py script inside the examples directory. Would you mind test it and report back if that addresses your needs?

MichelRabozee commented 1 year ago

Hello, I am currently testing the dump functionality.

I have a question: in the dump.py example, the second time a manager is created, there is no "manager.close()" nor "await http_api_client.async_logout()". Is it normal ? Will this not risk to exhaust the number of allowed tokens from Meross because the second manager also use "http_api_client = await MerossHttpClient.async_from_user_password(email=EMAIL, password=PASSWORD)" hence it is unclear if a "login" is really made or not. At least, I call "await http_api_client.async_logout()" and I am not calling "manager.close()" otherwise, the following error occurs:

/usr/local/Cellar/python@3.9/3.9.14/Frameworks/Python.framework/Versions/3.9/lib/python3.9/asyncio/base_events.py:1890: RuntimeWarning: coroutine 'MerossManager._update_and_send_push' was never awaited
  handle = self._ready.popleft()
RuntimeWarning: Enable tracemalloc to get the object allocation traceback

Also, I found a typo in the dump.py example:

    print("Discovered %d devices:", len(devices))

should be:

    print(f"Discovered {len(devices)} devices:")

Regarding the dump system, it seems this works very well and in combination with "manager.default_transport_mode = TransportMode.LAN_HTTP_FIRST", we do gain the "discovery" process.

I think to have the discovery then dumping done every morning , just in case, and use the dumped file for the recurrent calls to get the energy readings.

albertogeniola commented 1 year ago

Hi @MichelRabozee , you are absolutely right about suggestions: the manager must be closed in order to avoid the Meross ban. I've updated the example as you advised.

Thanks! I'm closing the issue as you reported it's working as intended.

MichelRabozee commented 1 year ago

The thing is, if "manager.close()" is called, you always get an error:

/usr/local/Cellar/python@3.9/3.9.14/Frameworks/Python.framework/Versions/3.9/lib/python3.9/asyncio/base_events.py:1890: RuntimeWarning: coroutine 'MerossManager._update_and_send_push' was never awaited
  handle = self._ready.popleft()
RuntimeWarning: Enable tracemalloc to get the object allocation traceback

Calling "await http_api_client.async_logout()" is fine though in the second instance as well.

albertogeniola commented 1 year ago

Have you tried invoking loop.stop() rather than loop.close() in your main()?

MichelRabozee commented 1 year ago

Yes, I tried, to no avail. As soon as "manager.close()" is called, the above error appears. Looks like when no access to the Meross cloud is needed, something is not created/initialised in the manager, or rather, something is tried to be closed but was never started as no access to the Meross cloud was needed.

albertogeniola commented 1 year ago

I see. It looks like there still was a call to the discovery method, triggered as soon as the mqtt broker is connected. To avoid that, I've added a property, auto_discovery_on_connection that you can set to False in order to disable that behavior. You can also instantiate the MerossManager and set the auto_discovery_on_connection to False, before starting it.

Would you mind testing the 0.4.5.4rc1 which adds that features to the library and let me know if setting that flag to false solves the issue?

MichelRabozee commented 1 year ago

Hello, sorry, I did not see you answered :-(

I tested the 0.4.5.4rc1 in full "local" mode, as it seems a problem appeared in "async_device_discovery()" (see #274).

There is no more error calling "manager.close()" is "manager.auto_discovery_on_connection = False" in the case of a full local situation !

Thank you !

albertogeniola commented 1 year ago

274 has been solved: please upgrade to 0.4.5.4, as it includes the features from 0.4.5.4rc1 and the hotfix for #274 .