Skip to content

fix: ZielLaden selects wrong last slot - #4309

Open
tpd-opitz wants to merge 2 commits into
openWB:masterfrom
tpd-opitz:bugfix/zielladen-time-slot-selection
Open

tpd-opitz wants to merge 2 commits into
openWB:masterfrom
tpd-opitz:bugfix/zielladen-time-slot-selection

Conversation

@tpd-opitz

@tpd-opitz tpd-opitz commented Nov 12, 2026 •

Copy link
Copy Markdown
Contributor

#4214 (comment)

  • avoid timeslot starting at target time being selected as charge time

Summary by Sourcery

Correct scheduled charging deadline handling so price-based charging uses the intended final slot boundary.

Bug Fixes:

  • Prevent scheduled charging from selecting a price slot that starts exactly at the target deadline.

Enhancements:

  • Preserve and propagate absolute charging deadlines through scheduled-charging plan selection and price-based charging calculations.
  • Handle missing deadlines safely by stopping charging and reporting a clear status message.
  • Improve deadline timestamp handling across daylight-saving transitions.

Tests:

  • Add coverage for deadline-aware plan selection, exclusion of slots starting at the deadline, missing deadlines, and daylight-saving transitions.

@tpd-opitz

Copy link
Copy Markdown
Contributor Author

@sourcery-ai review

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

Comment thread packages/helpermodules/timecheck.py Resolved

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

Comment thread packages/control/ev/charge_template.py Resolved
Comment thread packages/control/consumer/consumer.py Resolved
@tpd-opitz

Copy link
Copy Markdown
Contributor Author

@sourcery-ai review

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

Comment thread packages/control/consumer/consumer.py Resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant