Roborock timer: add find_next_schedule to replace next_schedule
This is a follow up for: https://github.com/rytilahti/python-miio/pull/1520#discussion_r970052935
In PR #1520 we discussed, if next_schedule should return the next schedule relative to (A) the time of access or (B) the time of object creation. While alternative B has the advantage of "fresh" data, even if a reference to a Timer is kept for an extended period, it may be surprising to a user that a property of an object can change over time (without any external influence). In other words: I would be surprised, if in a single threaded program the following assertion could ever fail assert timer.next_schedule == timer.next_schedule.
If an API behaves in a surprising way, it may lead to bugs as developers assume a certain behavior while using the API (see principle of least surprise).
As an alternative I suggested deprecating the next_schedule property and replace it with a method find_next_schedule. A method returning a different value based on the current time is far less surprising (IMHO).
To be honest: I'm not sure if it's worth the change. Maybe programs just should not keep the Timer objects in memory anyhow, as they might get changed by the user or another program at any point in time.
Codecov Report
Merging #1528 (ed7daf0) into master (818b891) will not change coverage. The diff coverage is
100.00%.
@@ Coverage Diff @@
## master #1528 +/- ##
=========================================
Coverage 82.19% 82.19%
=========================================
Files 145 145
Lines 14173 14173
Branches 3418 1607 -1811
=========================================
Hits 11650 11650
Misses 2298 2298
Partials 225 225
| Impacted Files | Coverage Δ | |
|---|---|---|
| ...o/integrations/vacuum/roborock/vacuumcontainers.py | 80.71% <100.00%> (ø) |
:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more
So, I don't think downstream users should cache the timer objects due to the reasons you already mentioned. The saved I/O is unlikely worth it and the use cases for it are rather uncommon, and if someone really needs that it is not so difficult to implement this on your own.