Initialize and cache descriptors only once

[starkillerOG]

Add caching to the descriptors to prevent unnessesary IO.

Also needs to happen for action descriptors which are added in this PR: #1588 (which PR gets merged first does not matter).

[codecov-commenter]

Codecov Report

Merging #1592 (14933c4) into master (b5a2940) will increase coverage by 0.00%.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #1592   +/-   ##
=======================================
  Coverage   80.88%   80.88%           
=======================================
  Files         156      156           
  Lines       15254    15256    +2     
  Branches     3283     3283           
=======================================
+ Hits        12338    12340    +2     
  Misses       2667     2667           
  Partials      249      249           

Impacted Files	Coverage Δ
miio/devicestatus.py	90.54% <0.00%> (+0.26%)	⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[starkillerOG]

@rytilahti something is wrong with the linting check, something about a username and gitlab???
That is unrelated to this PR I think.

[starkillerOG]

@rytilahti ready to be reviewd/merged

[starkillerOG]

@rytilahti thanks for the review!
I processed all feedback :)

[starkillerOG]

@rytilahti thanks for the feedback!
Ready for another round of comments.

[rytilahti]

Considering this is the main device class, please add some tests for the expected functionalities and this is then ready to be merged.

[rytilahti]

Needs to be rebased on top of the current master to fix the CI, see #1598

[rytilahti]

Oops, I wrote some comments earlier but forgot to submit them, sorry for the delay.

[starkillerOG]

@rytilahti I finnally got all tests working and fixed the looping issues with the info() call.
Could you have another look/merge?

[rytilahti]

This is wrong, the _fetch_info() is device-class internals and should not be accessed by any other class.

[starkillerOG]

Well how do you then suggest to prevent the loop: status --> info --> _initialize_descriptors--> status --> etc.
I can just remove the info call, but I am not sure what will then break downstream.

[rytilahti]

I'm not sure what's the best approach right off the bat (especially as genericmiot constructs status containers dynamically), but for statically defined containers it would be enough to access the status container class without really initializing one, right?

In [5]: from miio.integrations.vacuum.roborock import VacuumStatus

In [6]: vars(VacuumStatus)
Out[6]: 
mappingproxy({'__module__': 'miio.integrations.vacuum.roborock.vacuumcontainers',
<snip>
              '_sensors': {'state_code': SensorDescriptor(id='VacuumStatus.state_code', type=<class 'int'>, name='State code', property='state_code', unit=None),
               'state': SensorDescriptor(id='VacuumStatus.state', type=<class 'str'>, name='State', property='state', unit=None),
<snip>
              '_settings': {'fanspeed': NumberSettingDescriptor(id='VacuumStatus.fanspeed', name='Fanspeed', property='fanspeed', unit='%', min_value=0, max_value=100, step=1, range_attribute=None, type=<SettingType.Number: 2>),
<snip>

Probably the simplest way to access the status container would be using the return type annotation like this:

_status_container = self_.status.__annotations__["return"]

Do you think that could work for the time being until we figure out a better solution for that?

[starkillerOG]

I just tried it, but that does not include the embeded sensors:

from miio import RoborockVacuum

r = RoborockVacuum(ip = "192.168.1.IP", token = "TokenTokenToken")
status = r.status.__annotations__["return"]
print(status.sensors(status))

That is missing the embeded sensors/settings....

[rytilahti]

Ouch, that's indeed annoying... The real solution would be doing the "embedding" of status container classes outside of the status query (maybe inside __init__), and performing the update on the container prior to returning it in status(). This way:

1. we can introspect the available information without performing any I/O.
2. if some of the method calls required to embedded containers fail, we can remove those queries from future update cycles.

The question is how/where to do this sanely, I don't have an answer to that right now.

[starkillerOG]

I do see what you mean, and think it is a good path forward,
but I think at this point it gets a bit to complicated for me.

I think it would be more efficient if you would implement those changes.
Than we also do not need to have long wait times in between us working on it.

You could do one of the following:

• make the changes and push them to this branch.
• copy this code as a start, make your own branch with a new PR and close this one
• First merge this as a starting point and then make the changes later to get rid of the _fetch_info calls in device classes (can be added as a TO DO)

[starkillerOG]

I will make some additional tests once the actual code is approved and we decided how to handle the info call.

[starkillerOG]

Closing due to merge conflicts, follow up PR is here: #1701