Repository navigation
Conversation
Contributor
Author
|
@sourcery-ai review |
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="packages/helpermodules/timecheck.py" line_range="246" />
<code_context>
BUFFER_START_EARLIER = 600 # 10 Min vor dem geplanten Start kann begonnen werden
def scheduled_charging(self) -> Tuple[float, Optional[str], Chargemode]:
- plan_data, remaining_time, duration = self._find_recent_plan(self.data.usage.scheduled_charging.plans)
</code_context>
<issue_to_address>
**DST deadline shifts by an hour**
When a charging deadline falls in the repeated local hour during DST fallback, `end - now` uses naive wall-clock arithmetic, while `end.timestamp()` resolves an ambiguous local time independently; during the repeated hour at DST fallback, the returned deadline can differ from the deadline implied by `remaining_time`, so price-slot selection uses the wrong target time.
Build the deadline using timezone-aware datetimes, or derive its timestamp consistently from the same occurrence used to calculate `remaining_time`.
</issue_to_address>There was a problem hiding this comment.
Hey - I've found 2 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="packages/control/ev/charge_template.py" line_range="130" />
<code_context>
missing_amount: float = 0
phases: int = 1
plan: Optional[ScheduledChargingPlan] = None
+ deadline: Optional[float] = None
</code_context>
<issue_to_address>
**Missing deadlines abort charging**
When a plan without a deadline reaches price-based scheduling while dynamic pricing is enabled, `scheduled_charging_calc_current` raises an uncaught `ValueError` when it reaches price-based scheduling without a deadline, aborting the calculation instead of returning a charging state.
Represent the deadline as required on plans that can enter price-based scheduling, or handle a missing deadline as a controlled scheduling error rather than raising from the calculation path.
Also at `packages/control/consumer/consumer.py:254`.
</issue_to_address>
### Comment 2
<location path="packages/control/consumer/consumer.py" line_range="200" />
<code_context>
try:
plans_diff_end_date.append(
- {p.id: timecheck.check_end_time(p, self.BUFFER_AFTER_END_TIME)})
+ {p.id: timecheck.check_end_time_with_deadline(p, self.BUFFER_AFTER_END_TIME)})
log.debug(f"Verbleibende Zeit bis zum Zieltermin [s]: {plans_diff_end_date}")
except Exception:
</code_context>
<issue_to_address>
**Updated plans use stale deadlines**
When a configuration update replaces or modifies a scheduled plan after its deadline is read but before the selector looks up that plan again, the selector later looks up the plan by ID and calculates its duration from the updated plan, but keeps the deadline captured from the earlier plan state. The charging calculation can therefore choose price slots against a target that no longer belongs to the selected plan.
Select and retain the plan object together with its deadline in one consistent snapshot, or synchronize plan updates with selection.
Also at `packages/control/ev/charge_template.py:370`.
</issue_to_address>
Contributor
Author
|
@sourcery-ai review |
There was a problem hiding this comment.
Hey - I've found 2 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="packages/control/consumer/consumer.py" line_range="217" />
<code_context>
+ return None, 0, 0, None
plan_id = list(plan_dict.keys())[0]
- diff_end_date = list(plan_dict.values())[0]
+ diff_end_date, deadline = list(plan_dict.values())[0]
for p in plans:
</code_context>
<issue_to_address>
**Duplicate IDs mix plan deadlines**
When a stored scheduled-plan list contains two plans with the same ID and the earlier deadline entry is selected, the selected entry carries the deadline calculated for one plan, but the subsequent lookup by ID binds a different plan with the same ID. The remaining-time calculation and price schedule then combine one plan’s settings with another plan’s deadline, so charging follows the wrong target.
Resolve and carry the selected plan object alongside its deadline instead of looking it up again by ID, or reject duplicate IDs when loading plans.
Also at `packages/control/ev/charge_template.py:386`.
</issue_to_address>
### Comment 2
<location path="packages/helpermodules/timecheck_test.py" line_range="76" />
<code_context>
+ try:
+ now_timestamp = datetime.datetime.fromisoformat(now_utc).timestamp()
+ deadline_timestamp = datetime.datetime.fromisoformat(expected_deadline_utc).timestamp()
+ datetime.datetime.today.return_value = datetime.datetime.fromtimestamp(now_timestamp)
+ plan = Mock(spec=ScheduledChargingPlan, time=plan_time, frequency=Mock(
+ spec=FrequencyDate, selected=frequency, once=now_utc[:10],
</code_context>
<issue_to_address>
**DST tests fail during setup**
The test assigns `return_value` to the built-in `datetime.datetime.today` method, which does not support that attribute. Every parameterized DST case raises `AttributeError` before exercising the deadline calculation, so the test suite fails.
Use `monkeypatch.setattr` to replace `datetime.datetime.today` with a callable that returns the desired local time.
</issue_to_address>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
#4214 (comment)
Summary by Sourcery
Correct scheduled charging deadline handling so price-based charging uses the intended final slot boundary.
Bug Fixes:
Enhancements:
Tests: