Energy support, NLE support #503
Conversation
* Update BNCS * feat: add centralized ventilation controller NLLF --------- Signed-off-by: Tobias Sauerwein <cgtobi@gmail.com>
…, introduces a sum energy and an helper to be used in homeassistant primarily
… mode, add some tests
Reviewer's Guide by SourceryThis pull request introduces support for energy computation using the getmeasure API with power adjustment and adds support for multiple home selections. The changes include significant modifications to the energy and history data handling, as well as updates to the module classes to support these new features. File-Level Changes
Tips
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @tmenguy - I've reviewed your changes and found some issues that need to be addressed.
Blocking issues:
- Check for potential duplicate room IDs. (link)
Here's what I looked at during the review
- 🔴 General issues: 1 blocking issue, 27 other issues
- 🟢 Security: all looks good
- 🟡 Testing: 5 issues found
- 🟡 Complexity: 2 issues found
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.
src/pyatmo/modules/module.py
Outdated
| self.sum_energy_elec_peak = 0 | ||
| self.sum_energy_elec_off_peak = 0 | ||
|
|
||
| def compute_riemann_sum(self, power_data, conservative: bool = False): |
There was a problem hiding this comment.
Can this be broken down into smaller functions?
There was a problem hiding this comment.
hum, which ones , there is not a lot of statements in those not sure the Riemann one could be broken below this in meaningful and independent pieces.
Can you point me to th one you fill is too long?
There was a problem hiding this comment.
Well, maybe let's extract that riemann function into a separate helper class as well.
| power_data = self.get_history_data( | ||
| "power", from_ts=from_ts, to_ts=to_ts | ||
| ) | ||
| if isinstance( |
There was a problem hiding this comment.
This is deeply nested and thus hard to read and test.
|
great! we made it :) thx for the reviews and time on it |
Use of the getmeasure API to compute energy (with power adjustment), support for multiple homes selection
Summary by Sourcery
This pull request introduces energy computation using the getmeasure API with power adjustment and supports multiple home selections. It includes significant refactoring of energy-related classes and mixins, enhances historical data processing, and improves error handling. New tests have been added to ensure the robustness of these features.