Fix for plan interval minutes, Fix Solcast Timezone issue - #2875
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the handling of plan_interval_minutes from using getattr() with a fallback default value to storing it as an instance variable. The changes aim to improve code maintainability by eliminating repetitive getattr() calls throughout the codebase and centralizing access to this configuration value.
Key Changes
- Moved
plan_interval_minutesinitialization earlier in thereset()method (line 337) inpredbat.py - Added
plan_interval_minutesas an instance variable in web interface classes (WebInterfaceandMCPServer) - Replaced
getattr(self.base, "plan_interval_minutes", 30)andgetattr(self, "plan_interval_minutes", 30)calls with direct access toself.plan_interval_minutesacross multiple modules
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| predbat.py | Moved plan_interval_minutes initialization and version bump to v8.27.5 |
| web.py | Added instance variable and replaced 5 getattr() calls with direct access |
| web_mcp.py | Added instance variable and replaced 1 getattr() call with direct access |
| plan.py | Removed local variables and replaced with direct access (4 locations) |
| output.py | Removed local variables and replaced with direct access (8 locations) |
| octopus.py | Added instance variable and replaced 2 getattr() calls with direct access |
| futurerate.py | Added instance variable and replaced 1 getattr() call with direct access |
| fetch.py | Replaced 6 getattr() calls with direct access |
| execute.py | Replaced 2 getattr() calls with direct access |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| self.text_plan = "Computing please wait..." | ||
| self.prediction_cache_enable = True | ||
| self.base_load = 0 | ||
| self.plan_interval_minutes = 30 |
There was a problem hiding this comment.
The configuration value for plan_interval_minutes is no longer being read from self.args. The old line 628 self.plan_interval_minutes = self.args.get(\"plan_interval_minutes\", 30) was removed, which means users can no longer configure this value via YAML. This should be changed to self.plan_interval_minutes = self.args.get(\"plan_interval_minutes\", 30) to restore the ability to configure this parameter, or the line should be moved to after the args are available if they're not available at line 337.
| self.plan_interval_minutes = 30 | |
| self.plan_interval_minutes = self.args.get("plan_interval_minutes", 30) |
springfall2008
changed the title
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| max_kwh = 9999 | ||
|
|
||
| self.now_utc = datetime.now(timezone.utc) | ||
| self.now_utc = datetime.now(self.local_tz) |
There was a problem hiding this comment.
This changes the timezone from UTC to local timezone. The variable name now_utc suggests it should remain in UTC. If this change is intentional, the variable should be renamed to reflect that it's no longer UTC, or the timezone argument should be removed to keep it as UTC.
| self.now_utc = datetime.now(self.local_tz) | |
| self.now_utc = datetime.now(timezone.utc) |
| # Update time every minute | ||
| self.now = datetime.now() | ||
| self.now_utc = datetime.now(timezone.utc).astimezone() | ||
| self.now_utc = datetime.now(self.local_tz) |
There was a problem hiding this comment.
This changes the timezone from UTC to local timezone. The variable name now_utc suggests it should contain UTC time. The original code used datetime.now(timezone.utc).astimezone() which converts to local time but the variable name indicates UTC is expected. This change may introduce bugs if other code relies on now_utc being in UTC timezone.
| self.now_utc = datetime.now(self.local_tz) | |
| self.now_utc = datetime.now(timezone.utc) |
…2008#2875) * Tidy up plan interval minutes * Fix * Solcast fixes * Fixes * Octopus TZ fix * Fix
No description provided.